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

build: externalize react/jsx-runtime #69

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

Conversation

KlotzJesse
Copy link

Optimizes bundeling sizes, because at the moment react/jsx-runtime is being bundled unnecessary leading to a overhead in production, by externalizing react/jsx-runtime and also for the future react-dom/client

Would be awesome for having a quick merge, because no breaking change, no problem, just removes overhead, reduce bundle size, and also provides a fix.

Also fixes #66

@FarazzShaikh
Copy link
Owner

Thanks! Will review it tomorrow. Have you tested it as a fix for 66?

package/vite.config.ts Outdated Show resolved Hide resolved
Copy link
Author

@KlotzJesse KlotzJesse left a comment

Choose a reason for hiding this comment

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

yeah, great catch thanks

@KlotzJesse
Copy link
Author

KlotzJesse commented Nov 22, 2024

Thanks! Will review it tomorrow. Have you tested it as a fix for 66?

tbh no, had no time, but had this fixed for several other repos. you can verify the unecessary bundle here:
https://unpkg.com/three-custom-shader-material/three-custom-shader-material.es.js -> see react-jsx-runtime and search for ReactCurrentDispatch.

You will see, it bundles the use of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentOwner e.g. in the dist of the package in npm

The reason why it breaks is because __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED were changed in react 19, which leads to a problem and react version mismatch, because __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED is undefined, and nextjs15 is using react 19 rc

therefore the error, which will automatically be resolved therefore, e.g. cal.com reduced bundle for the embed-react package from 20kb to 2kb with that

@FarazzShaikh
Copy link
Owner

I see, thank you very much. I will do some additional verification before merging shortly.

@gdomaradzki
Copy link

Any news on this?

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.

Doesn't seem to work with Next.js 15 (React 19)
3 participants