-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Core: Add CJS entrypoints to errors in core events #24038
Conversation
Are you sure this is a cjs issue? I notice that the error message includes ".js" on the end of the file, which we don't include in our export map. Or, does jest just not respect / understand export maps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why this isn't handled by the export map? E.g.
e7e3132
to
abd016e
Compare
I thought maybe the exports maps were an ESM only thing, but it might as well be an issue with Jest. I'm not entirely sure! |
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
@SimenB, sorry for the ping, but I was wondering if Jest does not support export maps in package.json. This PR feels a little like a hack, and I was wondering if you knew of any better ways to add support for Jest in this case. |
@yannbf do you know where the ".js" on the file name came from? Is that something in our code, or did it get added somewhere (maybe by jest itself)? |
Alright I did a proper investigation and here's what's going on. This issue does not happen in Jest 29. jest-resolve v29 is able to resolve a package correctly based on its exports map. Therefore, this PR is a workaround to maintain compatibility with jest 27 projects, and because we support CRA, it's important. cc @shilman |
Yeah, Jest has full support from v29 (partial support ( |
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
…re-events Core: Add CJS entrypoints to errors in core events (cherry picked from commit 509dc79)
Closes #24009
What I did
When running tests with
composeStories
in Jest where node with ESM is not enabled, the tests fail with:This PR solves that.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Use this repro:
Button.test.js
in that repo:0.0.0-pr-24038-sha-abd016e1
npm run test
– it should workDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/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 pull request has been released as version
0.0.0-pr-24038-sha-abd016e1
. Install it by pinning all your Storybook dependencies to that version.More information
0.0.0-pr-24038-sha-abd016e1
yann/fix-cjs-entries-on-core-events
abd016e1
1693770530
)To request a new release of this pull request, mention the
@storybookjs/core
team.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=24038