Type check production files #31555

mrginglymus · 2025-05-23T09:45:49Z

What I did

As recommended by tsup, this runs type checking over generated .d.ts files to ensure tsup has not generated invalid production typescript.

Unfortunately, whilst the script works fine when invoked manually, I've no idea how to get nx to actually run it - there's a bit too much implicit magic going on for my to understand what's needed.

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>

Greptile Summary

This PR enhances TypeScript type checking by validating both source and production files, particularly the generated .d.ts files from tsup.

Added production mode to check.ts script that disables path aliases and skipLibCheck for dist files
Modified TypeScript config handling to support async file reading with better error logging
Added new check task configuration in nx.json with production mode option
Ensures generated TypeScript declaration files are valid during build process

greptile-apps

_{2 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

scripts/prepare/check.ts

code/nx.json

nx-cloud · 2025-05-23T09:52:32Z

View your CI Pipeline Execution ↗ for commit bcffd61.

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

☁️ Nx Cloud last updated this comment at 2025-06-12 12:06:23 UTC

mrginglymus · 2025-06-08T18:10:12Z

So what we're stuck on now is that tsup appears to be stripping the @ts-expect-error comments that are attached to types that form part of storybook's API. We could remove the production type-checks from those packages (nextjs and addon-vitest) as at least we'd then catch regressions on all the others.

mrginglymus · 2025-06-08T18:11:39Z

The addon-vitest one is curious, looks like tsup can't handle nodenext module resolutions?

ndelangen · 2025-06-11T14:24:49Z

code/addons/a11y/package.json

-    "check": "jiti ../../../scripts/prepare/check.ts",
-    "prep": "jiti ../../../scripts/prepare/addon-bundle.ts"


can you explain why these scripts need to be renamed?

I think I can. I find nx a bit impenetrable, but it seems that if a user runs nx check, and nx finds a script called check in package.json, it'll run that script rather than its own, ignoring all nx config.

So the options were either rename all the check scripts in all package.json, which is churny but not user visible, or rename the top-level nx command, which would potentially confuse users.

ndelangen · 2025-06-11T14:43:51Z

scripts/prepare/check.ts

+async function getTSFilesAndConfig(tsconfigPath: string, production = false) {
+  const content = await fs.readJson(tsconfigPath);
+  if (production) {
+    content.compilerOptions.skipLibCheck = false;


I see, so the current check is running a check on the src and this new "production" is type-checking the dist, which you can only do, if you have this flag as false, correct?

Yes - skipLibCheck needs to be false to check .d.ts files. I see that storybook's custom reporter is already eliminating errors that come from outside the sources root, so it could probably be safely set to true globally.

ndelangen · 2025-06-11T15:01:18Z

code/nx.json

+      "executor": "nx:run-commands",
+      "options": {
+        "cwd": "{projectRoot}",
+        "command": "yarn typecheck"


Could we keep this name check, to reduce the amount of changes needed?

Suggested change

"command": "yarn typecheck"

"command": "yarn check"

See above, as far as I can tell nx will prefer package.json definitions to its own if the names match. In the likely event I'm wrong it can be changed, but it took me five times as long to get nx working as to get the type checking working.

ndelangen · 2025-06-11T15:03:33Z

scripts/tasks/check.ts


 export const check: Task = {
  description: 'Typecheck the source code of the monorepo',
+  dependsOn: ['compile'],


I think it's preferable if we can still typecheck our src without compiling first; this would make that impossible?

All of the nx check tasks already depend on the build task (which is, confusingly, what the compile task runs...), so this makes minimal difference; at the very least it should include install to support running type checking from a cold-start

I suppose it does somewhat reduce the degree of parallelisation available as all build tasks need to be completed before check tasks can start. The stacked task runners do make this hard to follow sometimes.

storybook-app-bot · 2025-06-12T12:11:37Z

Package Benchmarks

^{Commit: bcffd61, ran on 12 June 2025 at 12:11:31 UTC}

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

`storybook`

	Before	After	Difference
Dependency count	49	49	0
Self size	31.87 MB	31.85 MB	🎉 -16 KB 🎉
Dependency size	17.41 MB	17.41 MB	0 B
Bundle Size Analyzer	Link	Link

`sb`

	Before	After	Difference
Dependency count	50	50	0
Self size	1 KB	1 KB	0 B
Dependency size	49.27 MB	49.26 MB	🎉 -16 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/cli`

	Before	After	Difference
Dependency count	216	216	0
Self size	582 KB	582 KB	0 B
Dependency size	94.59 MB	94.57 MB	🎉 -16 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/codemod`

	Before	After	Difference
Dependency count	185	185	0
Self size	31 KB	31 KB	🚨 +4 B 🚨
Dependency size	78.72 MB	78.71 MB	🎉 -16 KB 🎉
Bundle Size Analyzer	Link	Link

mrginglymus · 2025-06-16T07:56:17Z

Unfortunately the line from typescript appears to be "Don't have type errors in your public API": microsoft/TypeScript#38628

valentinpalkovic · 2025-07-07T13:34:14Z

@ndelangen and @JReinhold. Can you take a brief look at this PR as part of the ESM project, since you are about to revamp the build process entirely. Thanks!

mrginglymus · 2025-09-16T06:45:21Z

Closing in favour of #32447

greptile-apps bot reviewed May 23, 2025

View reviewed changes

scripts/prepare/check.ts Show resolved Hide resolved

code/nx.json Show resolved Hide resolved

Type check production files

2884be3

mrginglymus force-pushed the typecheck-dist branch from 293b741 to 2884be3 Compare May 24, 2025 19:06

mrginglymus added the ci:normal label May 25, 2025

Run check task in prod mode

898fa94

mrginglymus force-pushed the typecheck-dist branch from 8de30d0 to 898fa94 Compare May 25, 2025 15:47

Update remaining packages to support production type checking

977ccc0

mrginglymus force-pushed the typecheck-dist branch from 5dfd25a to 977ccc0 Compare May 25, 2025 16:13

valentinpalkovic assigned ndelangen and valentinpalkovic May 26, 2025

valentinpalkovic added the build Internal-facing build tooling & test updates label May 26, 2025

mrginglymus added 2 commits June 8, 2025 16:16

Merge branch 'next' into typecheck-dist

a6143d7

Remove seemingly redudant declaration from nextjs-vite

8066cc2

mrginglymus mentioned this pull request Jun 10, 2025

CSF Factories: Add parameters/globals types, extend API, portable stories #30601

Merged

8 tasks

ndelangen reviewed Jun 11, 2025

View reviewed changes

mrginglymus added 2 commits June 12, 2025 08:11

Check task only depends on install

ef21329

Merge branch 'next' into typecheck-dist

ae65c7e

mrginglymus requested a review from ndelangen June 12, 2025 07:24

Merge branch 'next' into typecheck-dist

bcffd61

github-actions bot added the Stale label Jul 3, 2025

github-actions bot removed the Stale label Jul 10, 2025

github-actions bot added the needs triage label Jul 10, 2025

github-actions bot added the Stale label Jul 25, 2025

vanessayuenn removed needs triage Stale labels Sep 1, 2025

vanessayuenn unassigned valentinpalkovic Sep 1, 2025

github-actions bot added the Stale label Sep 13, 2025

mrginglymus closed this Sep 16, 2025

mrginglymus deleted the typecheck-dist branch December 8, 2025 14:00

		"check": "jiti ../../../scripts/prepare/check.ts",
		"prep": "jiti ../../../scripts/prepare/addon-bundle.ts"

Uh oh!

Type check production files #31555

Type check production files #31555

Uh oh!

Conversation

mrginglymus commented May 23, 2025 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nx-cloud bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrginglymus commented Jun 8, 2025

Uh oh!

mrginglymus commented Jun 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

storybook-app-bot bot commented Jun 12, 2025

Package Benchmarks

storybook

sb

@storybook/cli

@storybook/codemod

Uh oh!

mrginglymus commented Jun 16, 2025

Uh oh!

valentinpalkovic commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrginglymus commented Sep 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mrginglymus commented May 23, 2025 •

edited by greptile-apps bot

Loading

nx-cloud bot commented May 23, 2025 •

edited

Loading

`storybook`

`sb`

`@storybook/cli`

`@storybook/codemod`

valentinpalkovic commented Jul 7, 2025 •

edited

Loading