Thoughts on dependencies handling

[yyx990803]

Some thoughts regarding dependencies / SSR / prod vs dev:

1. Apply plugins to non js/ts files found in dependencies during the pre-bundle phase.

   • fixes e.g. Pre-bundled dependencies doesn't dedupe imports in external files #3910
   • avoids having to manually exclude packages that include raw vue/svelte/jsx files, and also makes them eligible for pre-bundle caching.

2. Use esbuild to also pre-bundle dependencies for prod, then alias deps to the pre-bundled chunks during the Rollup build phase. This would

   • allow us to drop @rollup/plugin-commonjs and reduce prod/dev inconsistency due to difference in handling CJS between esbuild & @rollup/plugin-commonjs
   • allow us to cache the pre-bundled deps like the dev deps. Although they would still need to be processed by Rollup, it now needs to process fewer files and no longer need to do CJS transforms (essentially these are done by esbuild instead). Could potentially speed up the build to some extent.

3. For SSR deps handling - probably want to (1) get rid of the external/non-external thing altogether (2) run a similar pre-bundle step using esbuild, apply ssr transforms on the resulting ESM, and cache them. Overall the goal is to make prod/dev/ssr-prod/ssr-dev all be as consistent as possible.

These are just general directions that I think may resolve some of the main issues I currently consider important - I don't have time to work on them properly just yet, but want to throw it out there for discussion, and maybe someone can experiment with these ideas.

[brillout]

Do we need to pre-bundle/transform SSR dependencies in the first place?

We are having many difficulties because Vite tries to transpile SSR code living in node_modules/. Server libraries just aren't designed to work with bundlers. For example, I wrote many Node.js libraries in the past that do const filePath = './path/to/some.js'; require(filePath) instead of require('./path/to/some.js') simply because I know that my libraries are consumed by Node.js directly. Many other Node.js libraries (e.g. firebase, AWS) also assume to be consumed directly by Node.js.

I believe a more sensible default would be to not transpile and not even touch SSR dependencies at all. As long as we correctly transpile user SSR code we should be good.

As for ESM support, I believe we should respect the following rule:

• If the user's package.json has "type": "module" then Vite should keep the user's SSR code ESM.
• Otherwise, Vite should transpile the user's SSR code to CJS.

This would solve the ESM compat problems we are having (e.g. Vuetify).

[yyx990803]

I'm not sure if I follow the "don't process Node.js deps" part. Vite doesn't bundle all dependencies. It only bundles dependencies actually imported in your code. So if a user is having issue with e.g. AWS SDK, it means they are actually importing AWS SDK in their client-facing code.

[brillout]

So if a user is having issue with e.g. AWS SDK, it means they are actually importing AWS SDK

I'm not familiar with the AWS SDK issue, but I'm guessing that the problem is that the user's server code is importing some library from the AWS SDK which Vite tries to bundle and fails.

I know that's what is happening with Firebase: the user's server code imports firebase/functions and Vite fails to bundle that import.

Thus my proposal to have Vite not bundle any SSR dependency by default. Other than bundling Svelte/Vite/Marko components, or bundling for serverless environments such as Cloudflare Workers, there is no need to bundle SSR dependencies. Bundling SSR dependencies such as Firebase or AWS SDK is asking for trouble (these libraries have not been designed to work with bundlers).

Vite doesn't bundle all dependencies. It only bundles dependencies actually imported in your code.

I'm aware of this and that's actually why vite-plugin-ssr users don't encounter the problems that SvelteKit users are having: the server code of vite-plugin-ssr apps is not processed by Vite whereas the server code of SvelteKit apps is. So if there is a import { something } from 'firebase/functions' in a vite-plugin-ssr app then Vite will not discover that import, whereas in a SvelteKit app Vite does discover the import and tries to bundle it and chokes.

I'm not sure if I follow the "don't process Node.js deps" part.

With "don't process Node.js deps" I meant "don't process Node.js imports".

I'm not sure whether I answered your question or I'm just repeating myself, but I'm happy to elaborate further if there is anything left unclear.

[benmccann]

I believe SvelteKit solves this by basically telling users: "please add all SvelteKit components to ssr.noExternal". This is not ideal: it is not the Vite user but the library author that should decide whether the library should be transpiled or not.

I'd suggest a new field package.json#transpile: boolean to allow library authors to tell Vite (and other bundlers) whether that library should be transpiled.

This doesn't make much sense for Svelte. We can't go ask the author of every single svelte package to add a new field to their package.json. It also is unnecessary and redundant. 100% of Svelte packages need to be run through the Svelte compiler and we can already detect them without adding a new field. We do this automatically in vite-plugin-svelte today.

I'm not sure if I follow the "don't process Node.js deps" part. Vite doesn't bundle all dependencies. It only bundles dependencies actually imported in your code. So if a user is having issue with e.g. AWS SDK, it means they are actually importing AWS SDK in their client-facing code.

I'm a bit confused here. Are you talking about the esbuild pre-bundling? Because I think @brillout is talking about the ssr.noExternal bundling and I'm not sure why the SSR bundling would happen only in client-facing code

[brillout]

This doesn't make much sense for Svelte. We can't go ask the author of every single svelte package to add a new field to their package.json. It also is unnecessary and redundant. 100% of Svelte packages need to be run through the Svelte compiler and we can already detect them without adding a new field. We do this automatically in vite-plugin-svelte today.

Ok. Maybe there are other ecosystem where a package.json#transpile: boolean would be useful. My point being that in general it is the library author who should determine whether the library should be always transpiled or not (i.e. with Vite's current terminology: it is the library author who should determine whether her library should be in ssr.noExternal). But yea maybe a package.json#transpile: boolean has no practical application.

Anyways my main point in this whole thread is that Vite should treat all SSR dependencies (more precisely speaking: all imports of ssrLoadModule()) as ssr.external by default.

[aleclarson]

Vite should treat all SSR dependencies as ssr.external by default

I agree. Perhaps one solution is to let framework plugins (like Svelte's) mark dependencies as non-external. I don't think this is possible right now, but not 100% sure. In any case, it should be.

[bluwy]

Re 1, if we do start applying plugins during pre-bundling, it would be similar to how snowpack's esinstall works. I've made a comment regarding this (#3910 (comment)) but I'll rephrase here for clarity:

1. Some plugins (e.g. Svelte) generate different code for SSR and non-SSR, pre-bundling needs to be able to differentiate this and resolve properly in different contexts.
2. Vue and Svelte files may also output CSS, pre-bundling would need to be adjusted for that. Plugins may need a special API just for pre-bundling
3. We need to re-prebundle whenever a plugin's config change. To detect this, each plugin would need to provide a hash so Vite can determine when to re-prebundle.

Regardless of the concerns, this might be the only way to truly support raw vue/svelte/jsx. We will need to address these if we go down this path. Hoping @FredKSchott can give some insights for esinstall as well.

[FredKSchott]

I'll give a strong +1 to running your build plugins during dependency pre-bundling. This solved a bunch of issues for us in Snowpack and also reduced a bunch of inconsistencies/complexity by passing every file through the same X -> .js build pipeline. Our build pipeline integration was written in Rollup, but I assume you can create a similar plugin to do this for esbuild.

I'll also add that after building SSR dep prebundling into Snowpack/esinstall, I now think that this was the wrong default behavior. Instead, if a package manifest has a "main" entrypoint then it should be safe to assume that it will run natively in Node.js and can be treated as external for SSR purposes. If it also has some other entrypoint (like "svelte": "./src/Component.svelte") that we prefer, then the package should be treated as unbundled and passed through the same X -> .js build pipeline that use for everything else. This would also solve the CSS issue that you mention.

[dominikg]

Invalidating pre-bundled dependencies that ran through a plugins build could prove to be a hard problem. There are many configs and factors that can contribute to the output
Just a few that immediately come to mind

• postcss config
• browserslist
• files that are not classic "imports" but mixed in during transform (eg. scss files that generate the css for svelte components with <style lang="scss")
• typescript config

a hash over the known files could work, but even then you could still run into "stale" issues.

That being said i remember that vite1 used to run build for it's optimizer and the biggest issue i had with that was that the plugin couldn't tell apart if it was ran on optimize or build.
If the plugin is made aware that it's a pre-bundle transform and maybe gets a way to register files to watch that should discard the output it could work. (maybe with vite taking care of the postcss + typescript configs mentioned above)

[benmccann]

Apply plugins to non js/ts files found in dependencies during the pre-bundle phase.

Right now the plugins are Rollup plugins and the pre-bundling is done with esbuild, so how would this work exactly?

[aleclarson]

Using the same "plugin container" that Vite's dev server uses.

[benmccann]

There are two big issues I see frequently hit:

• We need to get rid of resolve and then resolve the actual import instead of .. resolve doesn't support exports (Support ESM resolution browserify/resolve#222), which is becoming more and more used. Perhaps we could use enhanced-resolve instead?
• Trying to put library code in a new AsyncFunction often doesn't work. I'm not really sure why that's being done and we don't just import it instead

[brillout]

Perhaps we could use enhanced-resolve instead?

Couldn't we use Node.js' resolve?

Node.js's built-in require.resolve instead. (With const require = createRequire(import.meta.url) for ESM.)

#4447

[brillout]

Trying to put library code in a new AsyncFunction often doesn't work. I'm not really sure why that's being done and we don't just import it instead

How would you import code that needs to be transpiled? I believe enabling transpiliation is the whole raison d'etre of using new AsyncFunction.

[benmccann]

Couldn't we use Node.js' resolve? (#4447)

Just updating this to save people from having to click into the link, but we agreed you probably can't because we use advanced functionality that isn't available in Node's version

How would you import code that needs to be transpiled? I believe enabling transpiliation is the whole raison d'etre of using new AsyncFunction.

Weren't you basically saying above that we shouldn't transpile code? Everytime I hit an issue it's always in code that doesn't need to be transpiled, so I'm always frustrated that it's going through this code path. Though I guess some transpilation is required because we need to handle non-JS extensions like .vue, .svelte, etc. But at least for Svelte we could detect Svelte libraries and skip transpilation on the plain JS libraries.

[brillout]

@benmccann Yes exactly I only meant .vue, .svelte, etc. files, and I still believe that SSR dependencies shouldn't be transpiled by default.

[aleclarson]

If nothing else, we should disable the optimizer in SSR middleware mode, since it's not even used, IIUC.

[davidwallacejackson]

Are there any serious downsides to option 2 (bundling deps for production with esbuild)? I know it wouldn't solve the problem with deps that contain raw .vue and .svelte files, but as I understand it it'd at least solve the dev->prod inconsistencies, and those are by far the most serious bug I've encountered in Vite so far.

For context, my team's software has an on-premises version, and new releases are a pain. When I deployed my first Vite build, we caught some issues in our cloud deployment that hadn't manifested in development. We immediately rolled back, but things could have gone much worse if, for instance, the bugs hadn't been caught until after we cut a new onprem version. This is something that's really stressing me out as I work on getting our Vite build ready to re-merge, to the point where I'm seriously considering adding tooling that will require us to whitelist each individual CommonJS package in our frontend build.

I'm curious how other developers using Vite in production right now are dealing with this -- there might well be less drastic mitigation steps that I'm missing. If there aren't, however, I'd really like to see a solution prioritized that gets dev and prod in line with each other, or even just identifies patterns in dependencies that trip common edge cases and warns or fails builds when a dependency exhibiting that pattern is imported. Of course, I'm happy to contribute in either regard if I can 🙂

[aolyang]

I have meet this problem while I use cjs export only module @mdi/react.

It exports Icon component by cjs way only.

At development(esbuild):

import Icon from "@mdi/react"
Icon // function Icon () => { ... }

At production(rollup, and cause react-dom errors):

import Icon from "@mdi/react"
Icon // { default: function Icon() { ... } }

After some debugging I come here, and what I did is simply:

import { Icon } from "@mdi/react"

/* OR
import * as all from "@mdi/react"

const iconComponent = <all.Icon />
*/

In most cases, we don’t have time to wait for the library to release a new version that original supports esm. It doesn't matter for me to change the import manually.

Sounds 2. pre-bundle dependencies for prod can give me a really good DX.

This is my humble opinion, thank you.

[ljharb]

I’m confused; nothing is CJS-only unless it’s c++ - in node, you can import every CJS module.

[aleclarson]

@ljharb He means it's a package that only provides CJS modules, whereas many packages provide both CJS and ESM formats.

[ljharb]

@aleclarson the vast majority of packages only provide CJS modules, and that will likely be true for a long time - but a package that only provides CJS modules is perfectly consumable by everything, so it should Just Work in vite as well.

[aleclarson]

@ljharb Yeah it's a known bug with CJS-only packages + vite build. See #2139

[Maniload]

Hey 👋 Is there any update on this? Especially point 1 would help us out a lot since it seems like it would fix what I described a while ago here: #7945

[aolyang]

Update somthing for No.2 (Use esbuild to also pre-bundle dependencies for prod).

Vite 3.0.0 has supported an experimental feature: 3.0.0 2022-07-13 Release log: Experimental Features

Experimental Features
Build Advanced Base Options
HMR Partial Accept
Vite now allows the use of esbuild to optimize dependencies during build time avoiding the need of @rollup/plugin-commonjs, removing one of the difference id dependency handling between dev and prod.

BUT it's not enabled by default: official docs

If you want to try this build strategy, you can use optimizeDeps.disabled: false (the default in v3 is disabled: 'build'). @rollup/plugin-commonjs can be removed by passing build.commonjsOptions: { include: [] }

[nikolamar]

If you want to try this build strategy, you can use optimizeDeps.disabled: false (the default in v3 is disabled: 'build'). @rollup/plugin-commonjs can be removed by passing build.commonjsOptions: { include: [] }

I can confirm that this solves an issue with @tensorflow-models/face-landmarks-detection

[Bersaelor]

Came here looking for an answer to the error: Ik.FaceMesh is not a constructor in @mediapipe/facemesh and can confirm,

import { defineConfig } from 'vite';

export default defineConfig({
  optimizeDeps: {
    disabled: false
  },
  build: {
    commonjsOptions: {
      include: []
    },
    ...

does indeed allow my project to run without the error.

[benmccann]

@Bersaelor you may try https://publint.dev/ to see if there's an error in the way that @mediapipe/facemesh is packaged. It doesn't appear to be a public package, so I was not able to check