Skip to content

Import React namespace to improve React 17 compatibility with ESM #12

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

Merged
merged 1 commit into from
Mar 6, 2023

Conversation

scottrippey
Copy link
Owner

In React 17, useInsertionEffect does not exist.
In #10, we are now exporting ESM, so import { useInsertionEffect } fails with React 17. Reported as #11.

This PR fixes that by importing the React namespace instead.

Considerations: does this defeat the purpose of building as ESM? Does this disable "tree shaking"? I don't imagine that React can be tree-shaken, so probably not.

@shkreios
Copy link
Contributor

shkreios commented Mar 2, 2023

@scottrippey AFAIK react itself only has a cjs output and is not tree shakable.
I tested 0.9.4 and the version of this pr both in react 17, 18 & preact all using vite, but couldn't get any error.
I added a comment to the #11 to ask in which dev env the error came up.
For now, I think the solution should be fine, but maybe it's worth looking at the internal package from emotion/react @emotion/use-insertion-effect-with-fallbacks (see code here).

They are using string interpolation to access useInsertionEffect, probably because some bundlers would try to optimize the access, which would cause errors.

@scottrippey scottrippey merged commit ab4cac2 into main Mar 6, 2023
@scottrippey scottrippey deleted the fix-esm-r17 branch March 6, 2023 15:20
@scottrippey
Copy link
Owner Author

Published as 0.9.5!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants