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

fix: add react and react-dom to resolve.dedupe at config #223

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lazarv
Copy link

@lazarv lazarv commented Jun 30, 2024

Adds "react" and "react-dom" to resolve.dedupe in the config hook. @vitejs/plugin-react does the same at https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/src/index.ts#L289-L291.

For some context, the missing dedupe of these React packages caused issues in my own React meta-framework (@lazarv/react-server) when adding support for @vitejs/plugin-react-swc. Using Vite Environment API it was not able to resolve "react/jsx-dev-runtime". I usually work on the framework in a pnpm workspace and the framework also provides it's own strict experimental React version.

@ArnaudBarre
Copy link
Member

My plan is to do the opposite and remove the dedupe option from the Babel plugin: vitejs/vite-plugin-react#280 (comment)

For me users should always have exactly one version of React installed. Why is this not the case for you?
How can you be sure that the dedupe option will keep the one you want?

@lazarv
Copy link
Author

lazarv commented Jun 30, 2024

My issue is that Vite uses dedupe to select the root at https://github.com/vitejs/vite/blob/14e14726d11ac55c37cc2d15aaf935be18ef925b/packages/vite/src/node/plugins/resolve.ts#L726-L737

if (dedupe?.includes(pkgId)) {
  basedir = root
} else if (
  importer &&
  path.isAbsolute(importer) &&
  // css processing appends `*` for importer
  (importer[importer.length - 1] === '*' || fs.existsSync(cleanUrl(importer)))
) {
  basedir = path.dirname(importer)
} else {
  basedir = root
}

With a root at /Users/lazarv/Projects/react-server/packages/react-server and having an importer at /Users/lazarv/Projects/react-server/examples/todo/src/index.tsx, when it's trying to resolve react/jsx-dev-runtime and if I don't have react in dedupe then the basedir will be incorrect as it's using the importer's directory, while I only have a resolvable React in the root.

I had a lot of resolution issues because React is not installed in the consumer project, so I do a lot of resolution workarounds to achieve what's necessary for the framework to work like a CLI tool too on any given React component file.

Sorry @ArnaudBarre I missed the existing PR at @vitejs/plugin-react removing resolve.dedupe. My current workaround is to add both react and react-dom to the Vite config in my framework. As everything worked as expected with the Babel version of the plugin, I suspected that resolve.dedupe is just missing here.

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