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

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

Open
7 tasks done
jsimonrichard opened this issue May 25, 2024 · 3 comments
Open
7 tasks done

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

jsimonrichard opened this issue May 25, 2024 · 3 comments

Comments

@jsimonrichard
Copy link

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
Copy link
Collaborator

hi-ogawa commented May 25, 2024

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/[email protected], 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
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants