fix: enable failing dependencies to be optimised by pre-processing them with esbuild

[fwouts]

Description

Some packages such as Gatsby ship with JSX which is expected to be compiled by the library users' bundler.

While there are workarounds to force Vite to interpret .js files as JSX (see discussion), these are not sufficient when the JSX is inside node_modules/. In particular, es-module-lexer doesn't support JSX (this was considered infeasible as per guybedford/es-module-lexer#47). This means that the parse() call at:

vite/packages/vite/src/node/optimizer/index.ts

Line 245 in 887c247

const exportsData = parse(entryContent) as ExportsData

fails when it encounters JSX.

While the best approach is certainly to ensure that packages don't ship as JSX, it's possible to improve the situation by adding a fallback mechanism which transforms the code with esbuild prior to re-trying.

This fix enables Gatsby to work when passing Vite the following options:

        esbuild: {
          loader: "jsx",
        },
        optimizeDeps: {
          esbuildOptions: {
            loader: {
              ".js": "jsx",
            },
          },
        },

Since it's only a fallback mechanism, it shouldn't affect performance adversely.

Additional context

This was originally surfaced as a bug report for the Viteshot project, see fwouts/viteshot#53.

Example project: https://github.com/fwouts/vite-gatsby-sample-app

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[aleclarson]

Why not use { loader: { '.js': 'jsx' } } automatically, removing the need for user to configure it?

[fwouts]

That's a good question. Even if we hardcode this here, you still do need the following to make it work, because it would otherwise fail at a later stage when optimising deps:

        optimizeDeps: {
          esbuildOptions: {
            loader: {
              ".js": "jsx",
            },
          },
        },

[aleclarson]

We could move this statement up:

vite/packages/vite/src/node/optimizer/index.ts

Lines 266 to 267 in 5935055

	const { plugins = [], ...esbuildOptions } =
	config.optimizeDeps?.esbuildOptions ?? {}

..then mutate the esbuildOptions from here:

esbuildOptions.loader ??= {}
esbuildOptions.loader['.js'] ??= 'jsx'
const transformed = await transform(entryContent, esbuildOptions)

[fwouts]

Nice idea. I did that, although couldn't directly use esbuildOptions for the transform call (since the loader field type isn't compatible). Let me know what you think!

[aleclarson]

A git repo that reproduces the issue would be super handy. :)

[fwouts]

A git repo that reproduces the issue would be super handy. :)

You're right, I should have thought about that! Here you go: https://github.com/fwouts/vite-gatsby-sample-app

[TomokiMiyauci]

I faced the same problem.

When I use storybook as a vite bandler,
I couldn't handle the jsx files with Gatsby js extension correctly.

[patak-dev]

I think we should discuss this PR with Evan before merging it, as it introduced special handling for JSX. It would be useful for that discussion to know what is preventing this fallback to be added as a userland plugin (or as a more generic feature that could enable the userland plugin)

[fwouts]

I think we should discuss this PR with Evan before merging it, as it introduced special handling for JSX. It would be useful for that discussion to know what is preventing this fallback to be added as a userland plugin (or as a more generic feature that could enable the userland plugin)

That's a great point. If special handling of JSX isn't desired, perhaps a good alternative would be to enable plugins to somehow be able to override the handling in this part of the code.

[antfu]

Using try-catch to fallback to jsx parsing for every js file in order to fix this niche case is not ideal to me. I think at least on their side they'd better to ship with .jsx instead of .js in their dist.

I will add this to this week's meeting with Evan.

[fwouts]

Hey @antfu, I was just wondering what was the outcome of the meeting? :)

[Shinigami92]

Hey @antfu, I was just wondering what was the outcome of the meeting? :)

The meeting was rescheduled to today

[Shinigami92]

After this we are good to go and got an approval from Evan at todays meeting

Accidentally pressed the wrong button :D