Skip to content

fix(react): Fix React jsx runtime import for esm #12740

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 3, 2024
Merged

fix(react): Fix React jsx runtime import for esm #12740

merged 2 commits into from
Jul 3, 2024

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Jul 3, 2024

Fixes #12608

Sucrase injects a "react/jsx-runtime" import, however, ESM requires this to end in .js because the react package doesn't have an exports field.

With this PR we rewrite the import to "react/jsx-runtime.js" for our ESM output.

I tried importing running require.resolve("react/jsx-runtime.js") with node and it crashed so we likely shouldn't do it for CJS.

@lforst lforst requested a review from andreiborza July 3, 2024 12:55
andreiborza
andreiborza approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

nice

@lforst lforst merged commit 071adae into develop Jul 3, 2024
83 of 84 checks passed
@lforst lforst deleted the lforst-fix-react-jsx-runtime-import-for-esm branch July 3, 2024 15:15
@lforst lforst mentioned this pull request Jul 4, 2024
Closed
@alexandersorokin
Copy link

alexandersorokin commented Jul 4, 2024 •
edited
Loading

It's look like this MR should break compatibility with react@18. js extension is only need if there are no an exports field in package.json or that field contains js extension.

But react@18 does have exports field without js extension. See: https://www.npmjs.com/package/react?activeTab=code
image

Only react@17 doesn't have exports field.

@andreiborza andreiborza mentioned this pull request Jul 5, 2024
Merged
andreiborza added a commit that referenced this pull request Jul 5, 2024
@andreiborza
fix(react): Revert back to jsxRuntime: 'classic' to prevent breakin…
69b6531 
…g react 17 (#12775)

Undoes some of the changes in
#12204 and
#12740 to fix
#12608.
@andreiborza andreiborza mentioned this pull request Jul 5, 2024
Merged
andreiborza added a commit that referenced this pull request Jul 5, 2024
@andreiborza
chore(e2e): Add react-17 e2e test app (#12778)
f4e7203 
Adds a react 17 test app so we can hopefully catch breaking react 17, as
we did recently
(#12608) with
#12204 and
#12740.
@TheSlimvReal TheSlimvReal mentioned this pull request Jul 25, 2024
Merged
This was referenced Jul 26, 2024
Open
Open
@amykapernick amykapernick mentioned this pull request Jul 27, 2024
Open
@aam-digital-ci aam-digital-ci mentioned this pull request Jul 27, 2024
Closed
This was referenced Jul 30, 2024
Open
Open
@amykapernick amykapernick mentioned this pull request Aug 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreiborza andreiborza andreiborza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't resolve 'react/jsx-runtime' in xxx/node_modules/@sentry/react/esm
3 participants
@lforst @alexandersorokin @andreiborza