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

react: Fix build with next/webpack + react@<18.0.0 #810

Merged
merged 1 commit into from
Nov 20, 2022

Conversation

strickczq
Copy link
Contributor

@strickczq strickczq commented Nov 19, 2022

build only cjs to be compatible with react@<18.0.0, and use a wrapper to support esm.

About this PR

This is going to fix #782 .

What's the problem

In @giscus/[email protected], next/webpack will resolve @giscus/react to only ./dist/index.cjs.js whatever you write require or import in your project, which escapes the issue.

(console.log was added manually for debugging)
image

But in @giscus/[email protected], with .mjs extension for esm import configured, @giscus/react will be resolved to ./dist/index.cjs when using require, and ./dist/index.mjs when using import.

./dist/index.mjs imports react/jsx-runtimewhich triggers this issue from react facebook/react#20235 .

image

The workaround

To be compatible with react@<18.0.0, we should only require the react/jsx-runtime (no esm import) in our build products.

To be esm compatible, we can use a wrapper mjs that imports the cjs product. Official approach here https://nodejs.org/api/packages.html#approach-1-use-an-es-module-wrapper

So I wrote a vite plugin to emit the wrapper file.

(console.log was added manually for debugging)
image

Thus, everything works fine!

My testing environment

node: 16.18.1
npm: 8.19.2
next: 12.3.3
react: 17.0.2

@vercel
Copy link

vercel bot commented Nov 19, 2022

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

Name Status Preview Updated
giscus-component ✅ Ready (Inspect) Visit Preview Nov 20, 2022 at 6:35PM (UTC)

Build only CommonJS to be compatible with `react@<18.0.0`,
and use a wrapper to support ESM.
@laymonage
Copy link
Member

Thank you very much for the excellent, in-depth writeup and fix! I don't think I could figure this out on my own 😆

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.

[bug] React component failed to build with Next.js 12
2 participants