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

Externalize all React entrypoints to support React 19 #229

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eps1lon
Copy link

@eps1lon eps1lon commented May 28, 2024

What:

Fixes compatibility with React 19. We're using mdx-bundler in vercel.com, https://nextjs.org/ and various other Vercel surfaces. We're using this patch in production already. Otherwise we couldn't use React 19.

Why:

Bundling the JSX runtime always worked incidentally even though it was never officially supported. Due to changes to the React secret internals, we now enforce not bundling parts of React while externalizing others. Either you bundle everything or none.

How:

Externalize all React entrypoints

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

Comment on lines +206 to +209
'react/compiler-runtime': {
varName: '_react_compiler_runtime',
type: 'cjs'
}
Copy link
Author

Choose a reason for hiding this comment

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

This isn't necessary for React 19 compat. But if you ever apply the React compiler, you need to externalize its runtime so I just went ahead and did it now to avoid future bugs.

@eps1lon eps1lon marked this pull request as ready for review May 28, 2024 13:11
Fixes React 19 compat issues due to bundling JSX dev runtime.
Also enables future applications of React Compiler to bundled MDX.
@eps1lon eps1lon force-pushed the externalize-react branch from 6dae7e5 to ad789be Compare May 28, 2024 13:27
React,
ReactDOM,
_jsx_runtime:
process.env.NODE_ENV === 'production' ? _jsx_runtime : _jsx_dev_runtime,
Copy link
Author

Choose a reason for hiding this comment

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

This is likely moot since you ship CJS which bundlers have a hard time tree-shaking. But once it's shipped as ESM, bundlers will be able to DCE react/jsx-dev-runtime in prod

@eps1lon eps1lon changed the title Externalize all React entrypoints Externalize all React entrypoints to support React 19 May 28, 2024
Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Sorry it took me so long to get to this. I'm running React 19 + mdx-bundler on kentcdodds.com. Is this an optimization?

I think I would like to combine this with a breaking change to drop the CJS build to avoid issues with DCE of the dev runtime.

@eps1lon
Copy link
Author

eps1lon commented Aug 19, 2024

It's not an optimization but a bug fix. mdx-bundler currently bundles the jsx-runtime which isn't supported (it's like bundling React itself). This worked because all supported versions of React were compatible with the bundled JSX runtime. However, React 19 is not compatible with the bundled JSX runtime.

Publishing a new version with an updated version of React 19 will not fix the issue since that would make mdx-bundler incompatible with React <19. By externalizing the JSX runtime, you're compatible with any React version.

It's not a breaking change if you use this library as documented i.e. use mdx-bundler/client. It is breaking if you inlined mdx-bundler/client since you'd now need to update it.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks a bunch!

@kentcdodds
Copy link
Owner

Hmmm.... Looks like this is failing the tests 🤔

@eps1lon
Copy link
Author

eps1lon commented Aug 19, 2024

I'll take a look later

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