vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.43k stars 6.17k forks source link

Unexpected inclusion of React in a pre-bundled module #17310

Open jsimonrichard opened 5 months ago

jsimonrichard commented 5 months ago

Describe the bug

I'm working on a SolidJS project which uses the @modular-forms/solid package. This package contains no mention of React. However, when I run the project, React is included in the pre-bundled version of @modular-forms/solid inside node_modules/.vite. This causes an error for projects that don't depend on react, like mine.

The @modular-forms/solid package shouldn't normally be pre-bundled at all (it's an ESM module). However, there's another issue (which I'll post in a moment) that causes this package to be pre-bundled when it shouldn't.

For the purposes of this issue, I've included the following in my vite config.

optimizeDeps: {
  include: ["@modular-forms/solid"]
},

Reproduction

https://github.com/jsimonrichard/vite-pre-bundling-issues-mre

Steps to reproduce

First, run the project.

cd correctly-pre-bundled
npm i
npm start

Then stop it and inspect the contents of `correctly-pre-bundled/node_modules/

System Info

System:
    OS: Linux 6.2 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
    CPU: (24) x64 AMD Ryzen Threadripper 1920X 12-Core Processor
    Memory: 44.71 GB / 62.64 GB
    Container: Yes
    Shell: 5.8.1 - /usr/bin/zsh
  Binaries:
    Node: 21.6.1 - ~/.nvm/versions/node/v21.6.1/bin/node
    npm: 10.2.4 - ~/.nvm/versions/node/v21.6.1/bin/npm
    bun: 1.1.8 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 123.1.64.113
    Chromium: 125.0.6422.60

Used Package Manager

npm

Logs

No response

Validations

hi-ogawa commented 5 months ago

I was also seeing some minor issue due to jsx-runtime and pre-bundling, but forgot to raise issues here https://github.com/hi-ogawa/reproductions/blob/main/vite-optimizeDeps-jsx-runtime/vite.config.ts

Your reproduction shows an issue more clearly, but looking through it, this might be still considered packaging / solid plugin issue, so I'm not entirely sure if this is a bug. (The fix I'm thinking is to copy user's tsconfig jsx options to esbuildOptions, but it doesn't look like it would fix Solid's case since they seem to be using jsxImportSource incorrectly as explained below).

Probably the workaround to get the correct jsx-runtime setup for pre-bundling is to explicitly set optimizeDeps.esbuildOptions. Can you try something like this?

export default defineConfig({
  optimizeDeps: {
    include: ["@modular-forms/solid"],
    esbuildOptions: {
      // it cannot be "preserve"
      jsx: "automatic",
      jsxDev: true,
      // not familiar with solid, but it looks like the right one is "solid-js/h" and not "solid-js" 
      jsxImportSource: "solid-js/h"
    }
  },
})

(EDIT: Actually what Solid can try is to setup this esbuildOptions automatically from their plugin https://github.com/solidjs/vite-plugin-solid/blob/873f4cec4db1dcffac9d909191cf828a9902a418/src/index.ts#L247)


At the same time, I'm also suspecting @modular-forms/solid in particular (or maybe Solid vite integration in general) might have some issues.

Even though @modular-forms/solid ships ESM in "import": "./dist/esm/index.js" https://publint.dev/@modular-forms/solid@0.20.0, I think solid plugin is prioritizing resolve.condition: ["solid"] https://github.com/solidjs/vite-plugin-solid/blob/873f4cec4db1dcffac9d909191cf828a9902a418/src/index.ts#L238-L240, so "solid": "./dist/source/index.js" entry is used by Vite. This file has raw JSX, so it needs to be transpiled for browser.


The @modular-forms/solid package shouldn't normally be pre-bundled at all (it's an ESM module).

Regarding this, Vite doesn't try to exclude ESM specifically unless you explicitly put it into optimizeDeps.exclude. Also as mentioned above, Vite is not picking up ESM anyways due to "solid" export.

jsimonrichard commented 5 months ago

Both

export default defineConfig({
  optimizeDeps: {
    include: ["@modular-forms/solid"],
    esbuildOptions: {
      // it cannot be "preserve"
      jsx: "automatic",
      jsxDev: true,
      // not familiar with solid, but it looks like the right one is "solid-js/h" and not "solid-js" 
      jsxImportSource: "solid-js/h"
    }
  },
})

and, for external reference (if you don't need pre-bundling)

export default defineConfig({
  optimizeDeps: {
    exclude: ["@modular-forms/solid"]
  },
})

are functional workarounds.

jsimonrichard commented 5 months ago

I unearthed another bug, I think. If you pass a function to defineConfig instead of an object, it doesn't think that optimizeDeps.esbuildOptions.jsx should be included.

export default defineConfig(() => ({
  optimizeDeps: {
    include: ["@modular-forms/solid"],
    esbuildOptions: {
      // it cannot be "preserve"
      jsx: "automatic",
      jsxDev: true,
      // not familiar with solid, but it looks like the right one is "solid-js/h" and not "solid-js" 
      jsxImportSource: "solid-js/h"
    }
  },
}))

This returns this error:

No overload matches this call.
  The last overload gave the following error.
    Argument of type '() => { plugins: Plugin<any>[]; optimizeDeps: { include: string[]; esbuildOptions: { jsx: string; jsxDev: boolean; jsxImportSource: string; }; }; server: { port: number; }; build: { target: string; }; }' is not assignable to parameter of type 'UserConfigExport'.
      Type '() => { plugins: Plugin<any>[]; optimizeDeps: { include: string[]; esbuildOptions: { jsx: string; jsxDev: boolean; jsxImportSource: string; }; }; server: { port: number; }; build: { target: string; }; }' is not assignable to type 'UserConfigFnObject'.
        Call signature return types '{ plugins: Plugin<any>[]; optimizeDeps: { include: string[]; esbuildOptions: { jsx: string; jsxDev: boolean; jsxImportSource: string; }; }; server: { port: number; }; build: { target: string; }; }' and 'UserConfig' are incompatible.
          The types of 'optimizeDeps' are incompatible between these types.
            Type '{ include: string[]; esbuildOptions: { jsx: string; jsxDev: boolean; jsxImportSource: string; }; }' is not assignable to type 'DepOptimizationOptions'.
              Type '{ include: string[]; esbuildOptions: { jsx: string; jsxDev: boolean; jsxImportSource: string; }; }' is not assignable to type 'DepOptimizationConfig'.
                The types of 'esbuildOptions.jsx' are incompatible between these types.
                  Type 'string' is not assignable to type '"automatic" | "transform" | "preserve" | undefined'.ts(2769)
index.d.ts(3154, 18): The last overload is declared here.

This change has been pushed to the MRE repo.