ndelangen commented 1 month ago

Closes https://github.com/storybookjs/storybook/issues/29082

What I did

I replaced esbuild-register with jiti.

We use it to be able to require() .ts-files (such as main.ts) during the loading of presets.

Jiti has a different approach to enabling loading .ts-files than esbuild-register.

Jiti seems to transpile the files individually, and skip files that don't need to be transpiled.

esbuild-register actually bundles the entrypoint into a single (in memory?) file.

The difference between these approaches is that esbuild-register solves any incorrect CJS > EMS usage by bundling; jiti will leave those problematic connections in place, which then fail at runtime.

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/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>_

tobiasdiez commented 1 month ago

What error do you see? Jiti should be able to handle cjs > esm imports.

You might need to play around with the esmResolve and interopDefault options for jiti. (Also jiti v2 has a few improvements around loading esm but I have no experience yet with it.)

ndelangen commented 1 month ago

@tobiasdiez I was seeing 2 issues.

1: We have a require() for an ESM-only module, which is plainly invalid. However due to esbuild' nature of bundling everything, it's resolved. This is not a problem with jiti, it's -I think- how it's intended to work.

2: I had trouble getting jiti v2 to work for our tool chain to compile. This problem I spend less time on investigating, so I don't have a lot of details on this one. I left a comment to should help you reproduce the error I ran into if you have the appetite.

If you do investigate and find something we are doing wrong, please let me know 🙏

github-actions[bot] commented 1 month ago

	Fails
:no_entry_sign:	PR is marked with "ci: do not merge", "BREAKING CHANGE" labels.
:no_entry_sign:	Please choose only one of these labels: ["BREAKING CHANGE","maintenance"]

Generated by :no_entry_sign: dangerJS against f05f8e2ca4a9a2b88aa29137465ac2190be5a796

nx-cloud[bot] commented 1 month ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f05f8e2ca4a9a2b88aa29137465ac2190be5a796. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

- [`nx run-many -t build --parallel=3`](https://cloud.nx.app/runs/MM0vWReQI8?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

JReinhold commented 1 month ago

To give a real life example, a user might have a main.js file (which will be handled by jiti/esbuild-register) that imports stuff:

// .storybook/main.js
import myUtil from './myUtil.js';

export default {
  ...
}

This is written in ESM, but it's in a project that does not have "type": "module" in package.json, thus it's a CJS project, and Node will treat the file as CJS because it doesn't have the .mjs extension. So the file itself is invalid, it should either be renamed to main.mjs or use const myUtil = require('./myUtil.js') instead.

With our current approach with esbuild-register, this invalidity is handled automatically, esbuild-register compiles everything to CJS, so it's fine. Jiti is more strict, it doesn't try to compile it, so when Node imports this, it fails.

I'd argue that Jiti is doing the right thing here, but it doesn't change the fact that this would break a lot of users - especially because we generate a default main.js file that is very likely to be invalid.

tobiasdiez commented 1 month ago

Strange, I thought in such an ambiguous case (js extension, no type in package.json) jiti would check for esm-like syntax in the code and then transpile it to cjs. What is the debug output in this case?

JReinhold commented 1 month ago

These are the relevant parts from the log with debug enabled:

JITI_DEBUG=1 book
@storybook/core v8.4.0-alpha.1

[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
...
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/@storybook/react/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
...
info => Starting manager..
info => Starting preview..
=> Failed to build the preview
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/jeppe/dev/work/storybook/storybook/code/node_modules/react-docgen/dist/main.js from /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js not supported.
Instead change the require of main.js in /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js to a dynamic import() which is available in all CommonJS modules.
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at src/plugins/react-docgen.ts (/Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:220:28)
    at __init (/Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:9:56)
    at /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:266:79

WARN Broken build, fix the error above.
WARN You may need to refresh the browser.

[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false

The only thing that stands out to me is that jiti doesn't log anything for the /Users/jeppe/dev/work/storybook/storybook/code/node_modules/react-docgen/dist/main.js file, which is the one that is failing to be require'd.

here's the full log:

JITI_DEBUG=1 book
@storybook/core v8.4.0-alpha.1

[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [cache] [hit] ./.storybook/main.ts ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/.storybook-main.09965c17.cjs
[jiti] [transpile] [cjs] ./.storybook/main.ts (0.466ms)
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/core/dist/core-server/presets/common-override-preset.cjs
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/addon-essentials/dist/index.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [cache] [hit] ./node_modules/@chromatic-com/storybook/dist/preset.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/dist-preset.fed2d0cb.cjs
[jiti] [transpile] [cjs] ./node_modules/@chromatic-com/storybook/dist/preset.js (12.961ms)
[jiti] [native] [require] ./node_modules/chromatic/dist/node.js
[jiti] [native] [require] ./node_modules/filesize/dist/filesize.cjs
[jiti] [native] [require] ./node_modules/jsonfile/index.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/addon-onboarding/dist/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/addon-interactions/dist/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/addon-essentials/dist/docs/preset.js
[jiti] Native require error: {"code":"ERR_REQUIRE_ESM"}
[jiti] [fallback] ./node_modules/@storybook/addon-essentials/dist/docs/preset.js
[jiti] [cache] [hit] ./node_modules/@storybook/addon-essentials/dist/docs/preset.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/docs-preset.8f1c0d5e.cjs
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/addons/docs/dist/preset.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/dist-preset.d01fd87e.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/addons/docs/dist/preset.js (6.64ms)
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/lib/cli/core/node-logger/index.cjs
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-external-links/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/rehype-external-links-index.83513c24.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-external-links/index.js (0.28ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-external-links/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.20dce800.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-external-links/lib/index.js (0.229ms)
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/@ungap/structured-clone/cjs/index.js
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-is-element/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/hast-util-is-element-index.a36c994e.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-is-element/index.js (0.212ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-is-element/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.878897a3.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-is-element/lib/index.js (0.151ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/is-absolute-url/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/is-absolute-url-index.f5e34f56.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/is-absolute-url/index.js (0.115ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/space-separated-tokens/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/space-separated-tokens-index.fbc0f836.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/space-separated-tokens/index.js (0.126ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/unist-util-visit-index.3775f987.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit/index.js (0.096ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.ce09ab75.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit/lib/index.js (0.149ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/unist-util-visit-parents-index.8761eb35.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/index.js (0.082ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.a37a0c91.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/lib/index.js (0.135ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-is/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/unist-util-is-index.452c87b5.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-is/index.js (0.076ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-is/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.e160688e.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-is/lib/index.js (0.106ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/lib/color.node.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-color.node.dd4feb28.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/unist-util-visit-parents/lib/color.node.js (0.08ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-slug/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/rehype-slug-index.8c40b46d.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-slug/index.js (0.146ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-slug/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.050730ef.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/rehype-slug/lib/index.js (0.134ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/github-slugger/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/github-slugger-index.1dc02d85.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/github-slugger/index.js (0.088ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/github-slugger/regex.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/github-slugger-regex.d77afe54.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/github-slugger/regex.js (0.094ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-heading-rank/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/hast-util-heading-rank-index.515a7935.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-heading-rank/index.js (0.089ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-heading-rank/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.e737fc88.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-heading-rank/lib/index.js (0.067ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-to-string/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/hast-util-to-string-index.2e0c1eee.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-to-string/index.js (0.068ms)
[jiti] [cache] [hit] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-to-string/lib/index.js ~> /var/folders/tv/nghbl2bd5y772bg1br3ql76w0000gn/T/jiti/lib-index.1a25d019.cjs
[jiti] [transpile] [cjs] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/hast-util-to-string/lib/index.js (0.077ms)
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/lib/react-dom-shim/dist/preset.js
(node:4684) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] ./node_modules/@storybook/core/dist/core-server/presets/common-preset.cjs
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/@storybook/react/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [native] [require] /Users/jeppe/dev/work/storybook/storybook/code/node_modules/@storybook/react-dom-shim/dist/preset.js
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
info => Starting manager..
info => Starting preview..
=> Failed to build the preview
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/jeppe/dev/work/storybook/storybook/code/node_modules/react-docgen/dist/main.js from /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js not supported.
Instead change the require of main.js in /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js to a dynamic import() which is available in all CommonJS modules.
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at src/plugins/react-docgen.ts (/Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:220:28)
    at __init (/Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:9:56)
    at /Users/jeppe/dev/work/storybook/storybook/code/frameworks/react-vite/dist/preset.js:266:79

WARN Broken build, fix the error above.
WARN You may need to refresh the browser.

[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false
✔ Would you like to help improve Storybook by sending anonymous crash reports? … no
[jiti] [init] version: 2.0.0 module-cache: true fs-cache: true interop-defaults: false          /25.3s

You can experience this yourself @tobiasdiez by cloning the branch and reverting the changes in this line - which was done to mitigate the issue. https://github.com/storybookjs/storybook/pull/29196/files#diff-71fb14d7214019004e69757298d9055859d208da11f28dbc363eacb5d672d0ceR44

The issue is happening because the preset in @storybook/react-vite is compiled to CJS, and that dist includes these lines:

// src/plugins/docgen-handlers/actualNameHandler.ts
var import_react_docgen, getNameOrValue, isReactForwardRefCall, actualNameHandler, actualNameHandler_default;
var init_actualNameHandler = __esm({
  "src/plugins/docgen-handlers/actualNameHandler.ts"() {
    "use strict";
    import_react_docgen = require("react-docgen");
    ({ getNameOrValue, isReactForwardRefCall } = import_react_docgen.utils);

But react-docgen is ESM only, which makes the require() fail: https://www.npmjs.com/package/react-docgen?activeTab=code

While we could solve this instance internally (and we have), but this would only be one of many such cases. And we expect that this would break for users/addons too pretty easily in similar scenarios.

It's entirely possible we've missed something glaringly obvious 🙏

tobiasdiez commented 1 month ago

So that's the problem

1: We have a require() for an ESM-only module, which is plainly invalid. However due to esbuild' nature of bundling everything, it's resolved. This is not a problem with jiti, it's -I think- how it's intended to work.

that @ndelangen mentions above (https://github.com/storybookjs/storybook/pull/29196#issuecomment-2373300004), right? It is different to what you describe in https://github.com/storybookjs/storybook/pull/29196#issuecomment-2374022271

I don't think that there is much here that you can do. If someone ships a js file that requires an esm-module, then this is bound to fail in some runtime sooner or later (probably bun would have the same issue as jiti). So the question would be more how to find these issues without reducing the user experience in the meantime. Perhaps you could first try to import the module using jiti and when this fails log the error but fail back to esbuild for some time?

What may also help is to use jiti.import(...) over jiti(...) to give preference to ESM.

storybookjs / storybook

ESM: Replace `esbuild-register` with `jiti` in core #29196

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

☁️ Nx Cloud Report