feat: allow react/jsx-dev-runtime imports

[aleclarson]

Description

This PR contains the following changes:

• Imports of react/jsx-runtime and react/jsx-dev-runtime are now resolved with the same module ID and globally deduplicated
• Skip over the actual react/jsx-runtime.js module (which uses process.env.NODE_ENV checking) in favor of manual resolution using the isProduction local variable
• Ensure both react/cjs/react-jsx-dev-runtime.development.js and react/cjs/react-jsx-runtime.production.min.js are processed by Vite's optimizer

Additional context

Fixes #6215

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[bluwy]

Looks like the lockfile needs an update for the CI to pass. Code lgtm though.

[bluwy]

With react 18.0.0, it has an exports field that is blocking importing this path, so I'm not sure this works anymore. Perhaps we can optimize react/jsx-runtime and react/jsx-dev-runtime directly? Since the process.env.NODE_ENV should be available in prebundling too.

I also noticed a bug with this Vite code 🤔

[patak-dev]

Linking to the PR opened after this comment since GitHub didn't link them #7673.

[patak-dev]

@aleclarson is this PR still valid given @bluwy's message above?

[aleclarson]

Perhaps we can optimize react/jsx-runtime and react/jsx-dev-runtime directly?

Can't do that because we redirect them to a virtual module:
https://github.com/aleclarson/vite/blob/1069536cb481a8c1c7835d75b0ebeae840fe9bd0/packages/plugin-react/src/index.ts#L374-L378

I think resolving devEntry and prodEntry to absolute paths should work, since that will sidestep the Node.js resolution algorithm (where pkg.exports is enforced).

[bluwy]

Is there a reason we need to provide a virtual module for them? I tested optimizing react/jsx-runtime locally, and I notice that esbuild builds the final output as

var react_jsx_runtime_default = require_jsx_runtime();
export {
  react_jsx_runtime_default as default
};

which breaks named imports, but Vite handles that with the needsInterop flag so it should still work. Unless plugin-react is handling more too?

[aleclarson]

See here:
https://github.com/aleclarson/vite/blob/1069536cb481a8c1c7835d75b0ebeae840fe9bd0/packages/plugin-react/src/index.ts#L380-L397

The motivation is to reduce production bundle size, and we use the same method in development to avoid any production-only surprises.

[bluwy]

I see. It feels weird to resolve to a different module in dev/prod since jsx-dev-runtime and jsx-runtime have different exports. Not sure how often that is an issue though.

I guess this is fine then. I also wonder if it's simpler then to use resolveId to resolve react/jsx-dev-runtime => react/jsx-runtime in prod; vice-versa in dev. But either ways they should work the same.

[aleclarson]

It already works that way :)

Of course, the virtual module complicates it a little more.

[bluwy]

Yeah my concern is that the virtual module trick adds another layer of complexity, and it's tightly coupled to the named exports. I'll see if the resolveId flow I mentioned above works.

[aleclarson]

Not sure how those failing tests could possibly be related to this PR 🤔

[patak-dev]

Closing as we have now merged #8546, and test are still failing here. @aleclarson if you think we should continue to discuss the dedupe part of this PR, let's check it in a new PR against main.