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] Add meta internal option for useMemoCache import #31654

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Conversation

mofeiZ
Copy link
Contributor

@mofeiZ mofeiZ commented Dec 2, 2024

Adds target: 'donotuse_meta_internal', which inserts useMemoCache imports directly from react. Note that this is only valid for Meta bundles, as others do not re-export the c function.

// target=donotuse_meta_internal
import {c as _c} from 'react';

// target=19
import {c as _c} from 'react/compiler-runtime';

// target=17,18
import {c as _c} from 'react-compiler-runtime';

Meta is a bit special in that react runtime and compiler are guaranteed to be up-to-date and compatible. It also has its own bundling and module resolution logic, which makes importing from react/compiler-runtime tricky.

I'm also fine with implementing the alternative which adds an internal stub for react-compiler-runtime and bundles the runtime for internal builds.

Copy link

vercel bot commented Dec 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 2, 2024 10:02pm

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 2, 2024
@mofeiZ mofeiZ marked this pull request as ready for review December 2, 2024 21:28
@mofeiZ mofeiZ requested review from poteto and josephsavona December 2, 2024 21:28
Adds `target: 'donotuse_meta_internal'`, which inserts useMemoCache imports directly from `react`. Note that this is only valid for Meta bundles, as others do not [re-export the `c` function](https://github.com/facebook/react/blob/5b0ef217ef32333a8e56f39be04327c89efa346f/packages/react/index.fb.js#L68-L70).

```js
// target=donotuse_meta_internal
import {c as _c} from 'react';

// target=19
import {c as _c} from 'react/compiler-runtime';

// target=17,18
import {c as _c} from 'react-compiler-runtime';
```

Meta is a bit special in that react runtime and compiler are guaranteed to be up-to-date and compatible. It also has its own bundling and module resolution logic, which makes importing from `react/compiler-runtime` tricky.

I'm also fine with implementing the alternative which adds an internal stub for `react-compiler-runtime` and [bundles](https://github.com/facebook/react/blob/5b0ef217ef32333a8e56f39be04327c89efa346f/scripts/rollup/bundles.js#L120) the runtime for internal builds.
Comment on lines +135 to +138
z.object({
kind: z.literal('donotuse_meta_internal'),
runtimeModule: z.string().default('react'),
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the flexibility of also specifying the runtimeModule here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do since we're rolled out to RN desktop apps which currently have react-forget-runtime patched in (as their fork of react doesn't include useMemoCache)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants