Skip to content
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

[compiler]: use default import for useMemoCache #32071

Closed

Conversation

stipsan
Copy link
Contributor

@stipsan stipsan commented Jan 14, 2025

Summary

Follows up on #31993 not fully resolving the regression #31963, see https://github.com/sanity-io/react-compiler-runtime-regression for all the deets.
This PR explores the solution that follows what Node.js' own error outlines, using a default export:

import c from 'react-compiler-runtime';
const {c: _c} = c;

Instead of what we currently do:

import {c as _c} from 'react-compiler-runtime';

Which doesn't work in a native node.js ESM env since the polyfill is exporting c as module.exports = {c};.

The react/compiler-runtime is still using exports.c = c;, and thus were unaffected by the changes in #31963.
Changing its import still works:

Test here.

How did you test this change?

The changes here were conceptually tested on the reproduction repo. I also ran cd compiler && yarn && yarn snap:build && yarn snap --watch. AFAIK all tests pass. I did look at the 900+ changed snapshots and AFAIK they seem correct.

@stipsan stipsan force-pushed the fix-react-compiler-runtime-import branch from 10e7062 to e3e1d14 Compare January 15, 2025 12:04
poteto added a commit that referenced this pull request Jan 16, 2025
Alternative to #32071. As a follow up to #31993, the `platform` target was incorrectly being set to `browser` since it was the default argument for the build script. This corrects it to `node` and `cjs` which I think should resolve node 20 issues.
poteto added a commit that referenced this pull request Jan 16, 2025
Alternative to #32071. As a follow up to #31993, the `platform` target
was incorrectly being set to `browser` since it was the default argument
for the build script. This corrects it to `node` and `cjs` which I think
should resolve node 20 issues.
@stipsan
Copy link
Contributor Author

stipsan commented Jan 16, 2025

Closing in favor of #32091

@stipsan stipsan closed this Jan 16, 2025
@stipsan stipsan deleted the fix-react-compiler-runtime-import branch January 16, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants