Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: Upgrade typescript #30354

Merged
merged 17 commits into from
Jan 30, 2025
Merged

Build: Upgrade typescript #30354

merged 17 commits into from
Jan 30, 2025

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 23, 2025

What I did

I want to use module mocking via the pacjage.json imports field.
However to do that I need a higher version of typescript from what we current use.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.9 MB 77.9 MB 519 B 0.97 0%
initSize 131 MB 131 MB 72.7 kB 3.57 0.1%
diffSize 53 MB 53 MB 72.2 kB 4.96 0.1%
buildSize 7.17 MB 7.17 MB 784 B -0.7 0%
buildSbAddonsSize 1.85 MB 1.85 MB 0 B 1.4 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB -12 B -0.82 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.91 MB 3.91 MB -12 B -0.74 0%
buildPreviewSize 3.26 MB 3.27 MB 796 B -0.68 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 15.2s 18.5s 3.3s 0.2 17.9%
generateTime 17.9s 18.7s 801ms -0.99 4.3%
initTime 11.3s 12.3s 918ms -0.82 7.5%
buildTime 8.4s 8.8s 315ms -0.62 3.6%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 4.9s 5.6s 738ms 0.12 13.1%
devManagerResponsive 3.6s 4.2s 598ms 0.27 14.1%
devManagerHeaderVisible 699ms 849ms 150ms 1.16 17.7%
devManagerIndexVisible 709ms 877ms 168ms 0.99 19.2%
devStoryVisibleUncached 3.9s 3.9s 89ms 1.51 2.2%
devStoryVisible 730ms 878ms 148ms 1.01 16.9%
devAutodocsVisible 644ms 831ms 187ms 1.35 🔺22.5%
devMDXVisible 619ms 662ms 43ms 0.44 6.5%
buildManagerHeaderVisible 677ms 774ms 97ms 0.22 12.5%
buildManagerIndexVisible 768ms 893ms 125ms 0.28 14%
buildStoryVisible 666ms 756ms 90ms 0.22 11.9%
buildAutodocsVisible 683ms 687ms 4ms -0.09 0.6%
buildMDXVisible 534ms 539ms 5ms -0.02 0.9%

Greptile Summary

Upgraded TypeScript across the Storybook monorepo from 5.3.2 to 5.7.3 to enable module mocking via package.json imports field, with notable performance impacts in development mode.

  • Added verbatimModuleSyntax compiler option in code/renderers/svelte/tsconfig.json to support module mocking
  • Modified type imports in code/renderers/svelte/src/mount.ts from { type X } to type { X } syntax
  • Changed type assertions in test files from toEqualTypeOf to toMatchTypeOf for more flexible type checking
  • Performance regression in dev mode with devAutodocsVisible showing 22.5% slowdown
  • Added explicit any type annotations in several files to satisfy stricter type checking requirements

@ndelangen ndelangen changed the title Buikd: Upgrade typescript Build: Upgrade typescript Jan 23, 2025
@ndelangen ndelangen changed the title Build: Upgrade typescript Build: Upgrade typescript Jan 23, 2025
@ndelangen ndelangen self-assigned this Jan 23, 2025
@ndelangen ndelangen added build Internal-facing build tooling & test updates ci:normal labels Jan 23, 2025
Copy link

nx-cloud bot commented Jan 23, 2025

View your CI Pipeline Execution ↗ for commit c605c85.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 48s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-30 19:06:14 UTC

@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Jan 23, 2025

Package Benchmarks

Commit: c605c85, ran on 30 January 2025 at 19:09:39 UTC

The following packages have significant changes to their size or dependencies:

@storybook/addon-docs

Before After Difference
Dependency count 17 17 0
Self size 2.20 MB 2.26 MB 🚨 +61 KB 🚨
Dependency size 7.92 MB 7.92 MB 0 B
Bundle Size Analyzer Link Link

@storybook/addon-essentials

Before After Difference
Dependency count 36 36 0
Self size 12 KB 12 KB 0 B
Dependency size 13.87 MB 13.93 MB 🚨 +61 KB 🚨
Bundle Size Analyzer Link Link

@storybook/addon-storysource

Before After Difference
Dependency count 7 7 0
Self size 1.89 MB 1.89 MB 0 B
Dependency size 10.74 MB 10.80 MB 🚨 +63 KB 🚨
Bundle Size Analyzer Link Link

@storybook/builder-webpack5

Before After Difference
Dependency count 237 237 0
Self size 80 KB 80 KB 🚨 +5 B 🚨
Dependency size 31.15 MB 31.14 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/angular

Before After Difference
Dependency count 266 266 0
Self size 366 KB 367 KB 🚨 +904 B 🚨
Dependency size 33.73 MB 33.71 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/ember

Before After Difference
Dependency count 256 256 0
Self size 22 KB 22 KB 🎉 -33 B 🎉
Dependency size 31.18 MB 31.17 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/html-webpack5

Before After Difference
Dependency count 247 247 0
Self size 6 KB 6 KB 0 B
Dependency size 31.70 MB 31.68 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/preact-webpack5

Before After Difference
Dependency count 245 245 0
Self size 6 KB 6 KB 0 B
Dependency size 31.27 MB 31.26 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/react-webpack5

Before After Difference
Dependency count 323 323 0
Self size 6 KB 6 KB 0 B
Dependency size 42.48 MB 42.47 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/server-webpack5

Before After Difference
Dependency count 255 255 0
Self size 14 KB 14 KB 0 B
Dependency size 32.68 MB 32.66 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/svelte-webpack5

Before After Difference
Dependency count 310 310 0
Self size 6 KB 6 KB 0 B
Dependency size 39.27 MB 39.25 MB 🎉 -13 KB 🎉
Bundle Size Analyzer Link Link

@storybook/vue3-webpack5

Before After Difference
Dependency count 495 495 0
Self size 6 KB 6 KB 0 B
Dependency size 55.68 MB 55.67 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/web-components-webpack5

Before After Difference
Dependency count 245 245 0
Self size 5 KB 5 KB 0 B
Dependency size 31.32 MB 31.31 MB 🎉 -12 KB 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 388 388 0
Self size 503 KB 503 KB 🎉 -310 B 🎉
Dependency size 75.28 MB 75.35 MB 🚨 +75 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 277 277 0
Self size 612 KB 617 KB 🚨 +6 KB 🚨
Dependency size 65.36 MB 65.43 MB 🚨 +70 KB 🚨
Bundle Size Analyzer Link Link

@storybook/source-loader

Before After Difference
Dependency count 5 5 0
Self size 41 KB 41 KB 0 B
Dependency size 10.68 MB 10.75 MB 🚨 +63 KB 🚨
Bundle Size Analyzer Link Link

@ndelangen ndelangen marked this pull request as ready for review January 30, 2025 20:02
@ndelangen ndelangen merged commit 3f0a7c5 into next Jan 30, 2025
58 checks passed
@ndelangen ndelangen deleted the norbert/typescript-upgrade branch January 30, 2025 20:02
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

83 file(s) reviewed, 17 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 57 to +58
"@types/webpack-env": "^1.16.0",
"typescript": "^5.3.2"
"typescript": "^5.7.3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider adding a ~ instead of ^ to prevent auto-updating to TypeScript 5.8+ when it releases, which could introduce breaking changes

@@ -141,7 +141,7 @@ export const getInteractions = (finalStatus: CallStates) =>
.filter((call) => call.interceptable)
.map((call) => ({
...call,
childCallIds: [],
childCallIds: [] as any[],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: consider using a more specific type than any[] for childCallIds, such as Call['id'][] to maintain type safety

Suggested change
childCallIds: [] as any[],
childCallIds: [] as Call['id'][],

Comment on lines 105 to 109
"devDependencies": {
"@types/node": "^22.0.0",
"next": "^15.0.3",
"typescript": "^5.3.2"
"typescript": "^5.7.3"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider adding an explicit minimum TypeScript version requirement in peerDependencies since this upgrade enables new functionality that may not work with older versions

@@ -57,10 +57,10 @@
"typescript": "^4.9.4 || ^5.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: dependency version range for typescript should be updated to match devDependency version ^5.7.3 for consistency

Comment on lines +59 to 67
"typescript": "^5.7.3",
"vue-component-meta": "^2.0.0",
"vue-docgen-api": "^4.75.1"
},
"devDependencies": {
"@types/find-package-json": "^1.2.6",
"@types/node": "^22.0.0",
"typescript": "^5.3.2",
"typescript": "^5.7.3",
"vite": "^4.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: TypeScript is listed in both dependencies and devDependencies. Consider removing it from one location to avoid potential version conflicts and duplicate installations.

Suggested change
"typescript": "^5.7.3",
"vue-component-meta": "^2.0.0",
"vue-docgen-api": "^4.75.1"
},
"devDependencies": {
"@types/find-package-json": "^1.2.6",
"@types/node": "^22.0.0",
"typescript": "^5.3.2",
"typescript": "^5.7.3",
"vite": "^4.0.0"
"magic-string": "^0.30.0",
"vue-component-meta": "^2.0.0",
"vue-docgen-api": "^4.75.1"
},
"devDependencies": {
"@types/find-package-json": "^1.2.6",
"@types/node": "^22.0.0",
"typescript": "^5.7.3",
"vite": "^4.0.0"

@@ -89,7 +89,7 @@ const isCurrentVersionPublished = async ({
`Unexpected status code when checking the current version on npm: ${response.status}`
);
}
const data = await response.json();
const data: any = await response.json();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider creating an interface for the npm registry response instead of using 'any'. This would provide better type safety.

@@ -10,11 +10,11 @@ const getTheLastCommitHashThatUpdatedTheSandboxRepo = async (branch: string) =>
const repo = 'sandboxes';

try {
const branchData = await (
const branchData: any = await (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Using 'any' type here bypasses type safety. Consider creating an interface for the GitHub API branch response structure.

await fetch(`https://api.github.com/repos/${owner}/${repo}/branches/${branch}`)
).json();
const latestCommitSha = branchData.commit.sha;
const commitData = await (
const commitData: any = await (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Using 'any' type here bypasses type safety. Consider creating an interface for the GitHub API commit response structure.

"target": "es2022",
"module": "ES2022",
"target": "ESNext",
"module": "Preserve",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Setting module to 'Preserve' may affect how modules are processed. Ensure this doesn't break existing module resolution behavior.

Suggested change
"module": "Preserve",
"module": "ESNext",

@@ -13,7 +13,7 @@ export const githubClient = (apiKey: string) => {
}),
});

const result = await res.json();
const result: any = await res.json();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider defining a proper type for the GraphQL response instead of using 'any'. Could use a generic type parameter based on the expected query result.

@github-actions github-actions bot mentioned this pull request Feb 1, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Internal-facing build tooling & test updates ci:normal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants