Skip to content

Core: Skip TypeScript loader hook registration under Jest - #36790

Merged
ghengeveld merged 4 commits into
nextfrom
fix/import-module-jest-module-register
Oct 21, 2026
Merged

ghengeveld merged 4 commits into
nextfrom
fix/import-module-jest-module-register

Conversation

@obvious-autobuild

@obvious-autobuild obvious-autobuild Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #36116, closes #36269

What I did

Storybook's importModule unconditionally registers TypeScript-loading module hooks: module.register on Node <26, module.registerHooks on Node 26+. Jest sandboxes module loading and throws the moment that happens, so every Jest-based consumer that loads Storybook config through importModule dies at setup.

Concrete case: @storybook/test-runner's jest-setup loads the user's .storybook/test-runner.ts through importModule. Against 11.0.0-alpha.6, every test suite fails at setup before a single test runs (Tests: 0 total), with an identical stack on two independent real-world repos (reaviz/reaviz, reaviz/reablocks):

importModule                      # unconditionally awaits registerTypescriptLoader
  registerTypescriptLoader
    nodeModule.register(...)      # ← Jest forbids this inside its sandbox
      throwHooksUnsupported (jest-runtime/build/index.js:4102)
    ← getTestRunnerConfig (@storybook/test-runner) ← jest-setup.js

Jest compiles TypeScript itself, so the hooks are never needed under Jest. This PR skips the registration when a Jest worker is detected:

 function registerTypescriptLoader() {
+  // Jest forbids module hook registration inside its sandbox (it compiles TS itself).
+  if (process.env.JEST_WORKER_ID) {
+    return Promise.resolve();
+  }
   if (!typescriptLoaderRegistration) {

With this fix, the migration guide's claim that existing test-runner setups can keep running against 11 at their own risk holds again: the runner prints its end-of-support banner and then executes the suites. The banner behavior itself is unchanged.

Evidence

Before — minimal Jest fixture (Storybook 11.0.0-alpha.6 + @storybook/test-runner@0.26.0, one trivial story, --index-json):

FAIL   browser: chromium  example-button.test.js
  ● Test suite failed to run
    module.register() is not supported in Jest: the hooks would attach to the module
    loader running Jest itself, not to the sandboxed require/import used by test code...
      at Function.throwHooksUnsupported [as register] (node_modules/jest-runtime/build/index.js:4102:17)
Test Suites: 1 failed, 1 total
Tests:       0 total

After — same fixture, same command, Storybook dist rebuilt with this PR:

PASS   browser: chromium  example-button.test.js
  Example/Button
    Primary
      ✓ smoke-test (200 ms)
Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total

Both runs print the test-runner end-of-support banner and the Vitest-addon advisory; only the setup crash is gone.

Additional verification:

  • New unit tests in code/core/src/shared/utils/module.test.ts: the Jest regression test fails on next (the Jest-forbidden register call is reached and throws through the real importModule) and passes with this fix; a second test asserts the TypeScript loader hooks still register exactly once outside Jest.
  • yarn test shared/utils: 9 files, 104 tests, all passing; oxlint and oxfmt clean.
  • Non-Jest loader path end-to-end: storybook build on a consumer repo with a TypeScript .storybook config, run against the rebuilt dist — exit 0.

Merge Danger

Door: two-way

Blast Radius: Minimal

The guard only short-circuits when JEST_WORKER_ID is set, and Jest never needs Storybook's hooks because it transforms TypeScript itself. Behavior outside Jest is unchanged: the registration call, its Node-version branch, and the caching are byte-identical. The only conceivable regression would be a Jest environment that also wanted the hooks registered — Jest forbids registering them there, so no such consumer can exist.

Checklist for Contributors

Testing

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

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

Manual testing

Caution

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

  1. Check out this branch and build core: yarn install, then yarn nx compile core.
  2. In a Storybook 11 project with a TypeScript .storybook/test-runner.ts, run the test runner against a dev server, e.g. npx @storybook/test-runner --url http://127.0.0.1:6006 --index-json.
  3. Expect the end-of-support banner, then suites to run and report real results — previously every suite failed at setup with module.register() is not supported in Jest and Tests: 0 total.
  4. In the same project (outside Jest), run storybook dev and storybook build and expect the TypeScript config to load exactly as before.

Documentation

  • Add or update documentation reflecting your changes

No documentation changes are needed: nothing user-facing changes, and no MIGRATION.md entry is required (the fix removes a regression that contradicts the existing "test runner support ended" entry).

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

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • 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.

Jest sandboxes module loading and throws 'module.register() is not
supported in Jest' when Storybook's importModule registers its
TypeScript loader hooks, killing every Jest test suite at setup
(e.g. via @storybook/test-runner's jest-setup loading the user's
.storybook/test-runner.ts). Jest compiles TypeScript itself, so the
hooks are never needed there - skip registration when a Jest worker
is detected.
@obvious-autobuild obvious-autobuild Bot added bug ci:normal Run our default set of CI jobs (choose this for most PRs). qa:needed Pull Requests that will need manual QA prior to release. labels Oct 14, 2026
@storybook-contribution-review storybook-contribution-review Bot added core review:1-architecture In automated review pipeline: awaiting stage 1 architecture review sev:S3 Medium priority. Fix within months if possible. test-runner labels Oct 15, 2026
@storybook-app-bot

storybook-app-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Package Benchmarks

Commit: 89931f2, ran on 21 October 2026 at 09:35:02 UTC

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

storybook

Before After Difference
Dependency count 62 68 🚨 +6 🚨
Self size 18.94 MB 21.99 MB 🚨 +3.05 MB 🚨
Dependency size 25.95 MB 27.06 MB 🚨 +1.11 MB 🚨
Bundle Size Analyzer Link Link

@storybook/svelte-vite

Before After Difference
Dependency count 12 12 0
Self size 56 KB 56 KB 🎉 -431 B 🎉
Dependency size 30.75 MB 30.73 MB 🎉 -13 KB 🎉
Bundle Size Analyzer Link Link

@storybook/sveltekit

Before After Difference
Dependency count 13 13 0
Self size 59 KB 58 KB 🎉 -1 KB 🎉
Dependency size 30.80 MB 30.79 MB 🎉 -14 KB 🎉
Bundle Size Analyzer Link Link

@storybook/vue3-vite

Before After Difference
Dependency count 95 95 0
Self size 32 KB 32 KB 🚨 +14 B 🚨
Dependency size 18.76 MB 18.78 MB 🚨 +21 KB 🚨
Bundle Size Analyzer Link Link

@storybook/web-components-vite

Before After Difference
Dependency count 14 14 0
Self size 19 KB 19 KB 0 B
Dependency size 25.50 MB 25.52 MB 🚨 +18 KB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 174 174 0
Self size 891 KB 884 KB 🎉 -7 KB 🎉
Dependency size 81.25 MB 83.26 MB 🚨 +2.01 MB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 167 167 0
Self size 42 KB 44 KB 🚨 +3 KB 🚨
Dependency size 79.74 MB 81.74 MB 🚨 +2.00 MB 🚨
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 63 69 🚨 +6 🚨
Self size 1.07 MB 1.07 MB 🚨 +5 KB 🚨
Dependency size 44.88 MB 49.04 MB 🚨 +4.16 MB 🚨
Bundle Size Analyzer node node

@storybook/svelte

Before After Difference
Dependency count 4 4 0
Self size 264 KB 253 KB 🎉 -11 KB 🎉
Dependency size 4.85 MB 4.85 MB 🎉 -10 B 🎉
Bundle Size Analyzer Link Link

@storybook/vue3

Before After Difference
Dependency count 91 91 0
Self size 209 KB 232 KB 🚨 +23 KB 🚨
Dependency size 18.22 MB 18.22 MB 🎉 -10 B 🎉
Bundle Size Analyzer Link Link

@storybook/web-components

Before After Difference
Dependency count 12 12 0
Self size 193 KB 213 KB 🚨 +20 KB 🚨
Dependency size 25.10 MB 25.10 MB 0 B
Bundle Size Analyzer Link Link

@storybook-contribution-review

Copy link
Copy Markdown

🤖 I am the Storybook contribution review bot; a human maintainer approved this message before it was posted.
Review phase: Architecture review (stage 1 of 3)

We are trying a three-stage automated review: architecture, code quality, and runtime verification. This message states which stages ran and which were skipped. A human maintainer approves each public message and makes the final decision. Please tell us whether the feedback and evidence are useful.

Thanks for this! The diagnosis is right, and registerTypescriptLoader in code/core/src/shared/utils/module.ts is the right place for the fix, because it is the single choke point. The architecture review asks for one structural change before the review continues.

Stages run on head 3cb80ec: architecture (stage 1): changes requested. Not run: code quality (stage 2) and runtime verification (stage 3). This comment therefore makes no claim about whether the original problem reproduces or whether this change fixes it.

Check the capability, not JEST_WORKER_ID.
The guard if (process.env.JEST_WORKER_ID) return has two problems:

  • The variable is inherited. Child processes get the parent's environment by default. A Jest suite that spawns storybook build or storybook dev would therefore skip the TypeScript loader in a plain Node process that could register it, and .storybook/main.ts would then depend on the Node version's built-in type stripping. We haven't run this; it follows from how Node passes the environment to child processes. Anyone who sets the variable for another reason is affected in the same way.
  • It adds runner-specific policy to shared core. The real condition is "this runtime refuses module hooks". Jest is one example, and other sandboxes would each need their own check.

Suggested change: make hook registration best effort. If the runtime refuses it, log at debug level and let import() decide. Jest, Bun, Deno and Node with type stripping all handle TypeScript themselves, and if TypeScript really can't load, the import still fails with a real error.

function registerTypescriptLoader() {
  typescriptLoaderRegistration ??= (async () => {
    /* existing Node-version branch, unchanged */
  })().catch((error) => {
    // Some runtimes (e.g. Jest's sandbox) forbid module hooks; importing may still succeed without them.
    logger.debug(`Storybook's TypeScript loader hooks could not be registered: ${error}`);
  });
  return typescriptLoaderRegistration;
}
  • Use the logger from storybook/internal/node-logger. Caching the failed result is intentional: a runtime that refuses once refuses every time.
  • In the regression test, remove vi.stubEnv('JEST_WORKER_ID', '1'). The test already makes register/registerHooks throw, so it then checks the behaviour itself. Optionally, add a case where a process that only inherits JEST_WORKER_ID still registers the hooks.
  • Optional: vi.spyOn(nodeModule, 'register') with vi.restoreAllMocks() is typed and avoids the as any assignments. Guard registerHooks with 'registerHooks' in nodeModule.

When a new commit is pushed, the review restarts at stage 1 on the new head.

@storybook-contribution-review storybook-contribution-review Bot added review:changes-requested Review paused: changes requested; the review comment names the failing stage and removed review:1-architecture In automated review pipeline: awaiting stage 1 architecture review labels Oct 15, 2026
Replace the JEST_WORKER_ID environment guard with a best-effort catch.
If the runtime forbids module hooks (e.g. Jest's sandbox), log at debug
level and let import() decide instead of keying off an inherited
environment variable. Cache the failed result: a runtime that refuses
registration once refuses it every time.
@obvious-autobuild

Copy link
Copy Markdown
Contributor Author

Addressed the stage 1 findings in 0353c7f:

  1. Removed the process.env.JEST_WORKER_ID guard from registerTypescriptLoader in code/core/src/shared/utils/module.ts. Hook registration is now best effort: when the runtime refuses module hooks, the code logs the failure at debug level through the logger from storybook/internal/node-logger and lets import() decide. The failed result stays cached, so a runtime that refuses once is not retried.
  2. Removed vi.stubEnv('JEST_WORKER_ID', '1') from the regression test. Both hook APIs throw inside the test now, and the test checks the behavior itself: importModule still resolves. Added the inherited-environment case: with JEST_WORKER_ID set, the hooks still register exactly once.
  3. Applied the optional typing cleanup: vi.spyOn(nodeModule, 'register') and vi.spyOn(nodeModule, 'registerHooks') with vi.restoreAllMocks() in afterEach, the 'registerHooks' in nodeModule guard, and no as any assignments.

Verification: oxfmt --check clean on both touched files, oxlint reports 0 errors, tsc --noEmit on core's tsconfig reports no errors in the touched files (the remaining errors are pre-existing on the base head), and vitest run src/shared/utils/module.test.ts passes 3/3. The refusal and inherited-environment tests fail against the pre-fix commit, so they pin the new behavior.

@valentinpalkovic valentinpalkovic self-assigned this Oct 15, 2026
@storybook-contribution-review storybook-contribution-review Bot added review:1-architecture In automated review pipeline: awaiting stage 1 architecture review review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review and removed review:changes-requested Review paused: changes requested; the review comment names the failing stage review:1-architecture In automated review pipeline: awaiting stage 1 architecture review labels Oct 15, 2026

@storybook-contribution-review storybook-contribution-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 I am the Storybook contribution review bot. This message was posted automatically.
Review phase: Architecture review (stage 1 of 3) and Thermo-nuclear code quality review (stage 2 of 3)

We are trying a three-stage automated review: architecture, code quality, and runtime verification. This message states which stages ran and which were skipped. Messages are posted automatically, and a human maintainer makes the final decision. Please tell us whether the feedback and evidence are useful.

Inline findings for this round. Only the comment on module.ts line 10 is blocking; the others are optional.

Comment thread code/core/src/shared/utils/module.ts Outdated
Comment thread code/core/src/shared/utils/module.ts
Comment thread code/core/src/shared/utils/module.ts Outdated
Comment thread code/core/src/shared/utils/module.test.ts Outdated
Comment thread code/core/src/shared/utils/module.test.ts Outdated
@storybook-contribution-review

Copy link
Copy Markdown

🤖 I am the Storybook contribution review bot. This message was posted automatically.
Review phase: Architecture review (stage 1 of 3) and Thermo-nuclear code quality review (stage 2 of 3)

We are trying a three-stage automated review: architecture, code quality, and runtime verification. This message states which stages ran and which were skipped. Messages are posted automatically, and a human maintainer makes the final decision. Please tell us whether the feedback and evidence are useful.

Thanks for the quick update in 0353c7f. The round 1 architecture findings are resolved: the JEST_WORKER_ID check is gone, hook registration is now best effort, and the tests no longer depend on that variable. Architecture review (stage 1) passes on this commit.

Code quality review (stage 2) asks for one change before the review can continue. The new top-level node-logger import in module.ts breaks the core build, and it is the cause of the red build--linux and normal-generated checks. The inline comment on module.ts line 10 has the details and a small suggested fix. The other inline comments are optional suggestions.

Runtime verification (stage 3) has not run yet. It runs after the blocking item is fixed. A human maintainer makes the final decision on this PR.

@storybook-contribution-review storybook-contribution-review Bot added review:changes-requested Review paused: changes requested; the review comment names the failing stage and removed review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review labels Oct 15, 2026
The top-level `import { logger } from 'storybook/internal/node-logger'`
broke the core build: scripts/build/utils/generate-bundle.ts imports
resolvePackageDir from this module's source and runs under plain Node
while core itself is being built, when code/core/dist does not exist
yet, so loading the build script failed with ERR_MODULE_NOT_FOUND. Load
the logger lazily inside the failure handler instead, matching how the
module already lazy-imports storybook/internal/bin/loader.

Also applies the remaining round-2 review suggestions:

- keep the caught registration error and attach it as `cause` when
  importModule throws, so a generic import failure does not hide the
  real loader problem
- try registerHooks on Node >= 26 with an early return and a single
  register fall-through, removing the duplicated resolve+register branch
- dedupe the register/registerHooks spy setup into one
  stubHookRegistration helper that returns call counters; call
  importModule twice so the exactly-once assertions prove memoization;
  keep the inherited JEST_WORKER_ID case as an it.each row

Verified: with code/core/dist/node-logger absent, plain Node fails to
load this module source before the change (ERR_MODULE_NOT_FOUND) and
loads it after; oxlint 0 errors, oxfmt clean, core tsc reports no
errors in the touched files, vitest passes 3/3.
@obvious-autobuild

Copy link
Copy Markdown
Contributor Author

Round-2 response, pushed in 89931f2a:

S2-1 (blocking, module.ts:10) — fixed. Removed the top-level storybook/internal/node-logger import; the failure handler now lazy-imports it (catch(async (error) => { const { logger } = await import('storybook/internal/node-logger'); ... })), the same pattern the module already uses for storybook/internal/bin/loader. Verified the build break directly on the touched commit: with code/core/dist/node-logger removed, node -e "import('file:///home/user/work/storybook/code/core/src/shared/utils/module.ts')" failed with ERR_MODULE_NOT_FOUND: Cannot find module '.../code/core/dist/node-logger/index.js' before the fix and loaded the module (importMetaResolve, importModule, resolvePackageDir, safeResolveModule) after it, under plain Node v22.22.3.

S1-4 (module.ts) — fixed. The caught registration error is kept in a module-level typescriptLoaderRegistrationError and attached as cause on the error importModule throws when both import and require fail. Hook refusal is never detected by message matching.

S2-4 (module.ts:66) — fixed. registerHooks is tried on Node ≥ 26 with an early return, falling through to a single register call; behavior is unchanged.

S2-2 (module.test.ts:53) — fixed. The allowed-runtime test calls importModule twice and still asserts exactly one registration, so the assertion now proves memoization; the refusal test also calls importModule twice and asserts the throwing register spy was called only once.

S2-3 (module.test.ts:68) — fixed. One stubHookRegistration helper stubs both APIs and returns call counters; the JEST_WORKER_ID case is kept as an it.each row next to the allowed-runtime case as an explicit guard against environment checks.

Gates on the touched files: oxlint 0 errors (3 warnings are pre-existing on untouched lines), oxfmt --check clean, core tsc --noEmit reports no errors in module.ts/module.test.ts (remaining errors are pre-existing in unrelated files), vitest run src/shared/utils/module.test.ts passes 3/3. The review evidence was useful — the inline CI trace pinpointed the failure exactly.

Note for reviewers: the branch history also includes 6c77e7062d, a merge of origin/next into this branch that predated this commit; it does not touch the two files changed here.

@storybook-contribution-review storybook-contribution-review Bot added review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review and removed review:changes-requested Review paused: changes requested; the review comment names the failing stage labels Oct 19, 2026
@ghengeveld

Copy link
Copy Markdown
Member

I ran this against real Jest 30.5.2, in a small project that loads .storybook/test-runner.ts through serverRequire inside a Jest test, the same path @storybook/test-runner uses.

  • On next, the config load fails with module.register() is not supported in Jest.
  • With this PR (89931f2), the config loads on Node 22.16, 24.13 and 26.12.

The failing init-features, init-empty-react-vite-ts and normal-generated checks are not caused by this PR. They fail the same way on other open PRs, because @storybook/addon-vitest is missing in the generated project.

I added patch:yes, since this also breaks fresh test-runner installs on 10.x.

@ghengeveld ghengeveld added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 21, 2026
@ghengeveld
ghengeveld merged commit 8ccbb5b into next Oct 21, 2026
164 of 166 checks passed
@ghengeveld
ghengeveld deleted the fix/import-module-jest-module-register branch October 21, 2026 10:01
Sidnioulz pushed a commit that referenced this pull request Oct 21, 2026
…ule-register

Core: Skip TypeScript loader hook registration under Jest
(cherry picked from commit 8ccbb5b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). core patch:yes Bugfix & documentation PR that need to be picked to main branch qa:needed Pull Requests that will need manual QA prior to release. review:2-code-quality Stage 1 passed: awaiting stage 2 code quality review sev:S3 Medium priority. Fix within months if possible. test-runner

Projects

Status: Done

3 participants