vitejs / vite

Next generation frontend tooling. It's fast!
http://vitejs.dev
MIT License
66.89k stars 6.01k forks source link

Consider treeshaking module for serving files in dev mode #8237

Open AlexandreBonaventure opened 2 years ago

AlexandreBonaventure commented 2 years ago

Clear and concise description of the problem

Hello! Lately as our codebase has grown bigger and bigger over the last months we've been experiencing slow page reloading using Vite. That is not surprising given that our module size went off the chart, but it really is becoming a real pain point for us. I hear that we are not alone in this case, reading through some related issues: https://github.com/vitejs/vite/discussions/8232 and I totally agree with what @patak-dev says here: https://github.com/vitejs/vite/issues/7608#issuecomment-1087877492

The thing is, HMR has some reliability issues in our large codebase (where sometimes patching generates some issues) and these issues are very hard to debug, and creating a minimal reproduction is a huge amount of energy (for potentially no results) I failed to follow up on that ticket as well: https://github.com/vitejs/vite/issues/3719


With all that said, we are guilty of doing something that really does not help: we often import shared code with an index file because it is handy:

// /shared/index.ts
export { a } from './a'
export { b } from './b'
export { c } from './c'
// Component.vue
import { a, b } from '/shared'

// instead of 
import a from '/shared/a'
import b  from '/shared/b'

Of course, I already hear people say: 'Duh! just don't do this and import every single module individually you lazy dumbass' I would answer: easier said than done when your codebase is a big monorepo and you share hundreds of compositions/components/libraries/helpers.... We obviously see that this is way forward if we want to address our pain points with slow reloads.

END OF CONTEXT----

Suggested solution

Like I said above, we have clear directions for improving the perf of our app in development but this whole issue got me thinking about the module loading strategy of Vite's dev server, and I wanted to start a discussion about considering to support treeshaking for module delivering because I think it could potentially help others.

Let's take the example above:

// /shared/index.ts
export { a } from './a'
export { b } from './b'
export { c } from './c'
// Component.vue
import { a, b } from '/shared'

// instead of 
import a from '/shared/a'
import b  from '/shared/b'

Currently Vite delivers /shared/index.ts as is and the browser will load c module even though we're not using it. If c has some other dependencies, browser will load them as well, etc.. You end up loading a whole module dep branch that we're not even using. In my wildest dreams, Vite could transform Component.vue as such:

// Component.vue
import { a, b } from '/shared?dep=a,b'
// /shared/index.ts?dep=a,b
export { a } from './a'
export { b } from './b'
// c module is ruled out

I have created a small stackblitz to highlight the current Vite behaviour: https://stackblitz.com/edit/vitejs-vite-cjqcd4?file=src/App.vue image

Did you guys ever consider it ? Am I out of my mind ?

Basically my thought is: if Vite was "smart" enough to treeshake module for creating the dep tree, we would not have the need to refactor at all, and potentially it could speed up the app loading for a lot of users.

I most likely have a naive conception of how the whole thing works and I don't how much is feasible on that front but I wanted to kickstart discussion nonetheless. There are probably a lot of different requirements regarding side-effects etc.. and I understand it is potentially a huge Pandora's box...

Alternative

No response

Additional context

No response

Validations

yaldram commented 2 years ago

@AlexandreBonaventure thanks a lot for bringing this up here. I migrated our project to vite it took us almost 3 weeks fixing dependencies and errors but we hit this issue unfortunately. You mentioned changing imports for improving dev server performance can you please share a guide or docs link do we have it anywhere. Our project is open source - https://github.com/appsmithorg/appsmith/tree/release/app/client/src. And for a redux file save I see my browser sends about 1500 requests to the vite dev server, should I need to change any imports or restructure I can look into it. Thanks

Aaron-Pool commented 2 years ago

My team has also run into this issue on my project. Index files make logically grouping sections of your codebase much cleaner, but using them in import statements significantly effects the startup speed of vite, as things stand now.

Also, it's worth pointing out that this could also have significant positive impacts on vite-based tools, such as Vitest, and playwright's new experimental component testing setup.

volkandkaya commented 2 years ago

Running into this issue as well. 700 files and can take 10+ seconds for a HMR.

I followed https://carljin.com/vite-resolve-request-files-a-ton.html which didn't help much for my use case.

bellizio commented 2 years ago

My team hit this issue as well in our migration from Webpack to Vite. It wasn't obvious initially as we were starting with small chunks of code pertaining to only 1 React app, but as we added more React apps and corresponding entrypoints (we have a multi-page app setup) the issue surfaced, causing a page not to load in local development mode.

We were scratching our heads for a bit trying to figure it out because the error was coming from a module way down in the dependency tree for another module tied to a route that we weren't viewing/loading. We looked through the network requests and started swapping out import statements, which resolved the issue.

I wasn't able to find anything in the docs about using index.js files with multiple module exports, but I think it would be helpful to at least have this documented somewhere.

volkandkaya commented 2 years ago

Resolved it in a similar way, Vite seems to take a different approach than webpack (which is apparently better).

So I had to change a ton of imports and it got better.

Miofly commented 2 years ago

has a good way to solve this problem now?

Dschungelabenteuer commented 1 year ago

I've came across a similar issue with my team while working on our company's software overhaul.

Just for the record, we're using Storybook to document all kinds of things, from components to data strategies. We've came up with a lot of custom components to make our documentation even more interactive and impactful. All these components are made available and imported through an index file.

Now, whenever we'd import any component from that entry file - in development mode - it would initiate browser requests to load all files imported by this entry file. This makes some pages really slow to (re)load since they would load a whole bunch of files even though it only actively consumes a very specific piece of code.

Given the lack of actual solution on Vite side, I've ended up writing a simple Vite plugin which mimics tree-shaking when importing code from entry files. This appears to work pretty well with our codebase as the number or browser requests drastically decreased (since we've adopted it). I thought it could maybe help some of you guys out there, so i've tried to make it a public package. This is still very experimental and I can't tell for sure this will suit your needs, but I would love to see it being field-tested and get any feedback.

vite-plugin-entry-shaking

ryan-mcginty-alation commented 1 year ago

I've came across a similar issue with my team while working on our company's software overhaul.

Just for the record, we're using Storybook to document all kinds of things, from components to data strategies. We've came up with a lot of custom components to make our documentation even more interactive and impactful. All these components are made available and imported through an index file.

Now, whenever we'd import any component from that entry file - in development mode - it would initiate browser requests to load all files imported by this entry file. This makes some pages really slow to (re)load since they would load a whole bunch of files even though it only actively consumes a very specific piece of code.

Given the lack of actual solution on Vite side, I've ended up writing a simple Vite plugin which mimics tree-shaking when importing code from entry files. This appears to work pretty well with our codebase as the number or browser requests drastically decreased (since we've adopted it). I thought it could maybe help some of you guys out there, so i've tried to make it a public package. This is still very experimental and I can't tell for sure this will suit your needs, but I would love to see it being field-tested and get any feedback.

vite-plugin-entry-shaking

@Dschungelabenteuer Does this only work for entry files? Could it be adapted to work for stories: ['../src/lib/**/*.stories.@(js|jsx|ts|tsx)'] array?

probablykasper commented 1 year ago

When I added a simple icon import, every page load/navigation started taking 30-60s and build times also increased by about 20s. Would be nice to see this prioritized

Dschungelabenteuer commented 1 year ago

@Dschungelabenteuer Does this only work for entry files? Could it be adapted to work for stories: ['../src/lib/*/.stories.@(js|jsx|ts|tsx)'] array?

@ryan-mcginty-alation I am so sorry I've just noticed your message, could you be a bit more explicit about what you're trying to achieve here ? Please feel free to open an issue on that repo, I'll be happy to answer :-)

kadoshms commented 1 year ago

We experience the same beahior on our project as well. Our project is a relatively large monorepo, and currently importing from a package's entry point, birngs along all of the elements exported by that package on the first load. This was also observed in our storybook project that takes a while to load at first because of that massive file fetching.

maylortaylor commented 1 year ago

My team is starting a React + Vite project right now and I would love some info or traction on this issue before we get too deep into the project. If the simple solution is "just use webpack" then I'd rather do that now than get months into this project with Vite and have to refactor a lot.

wuifdesign commented 1 year ago

My team is starting a React + Vite project right now and I would love some info or traction on this issue before we get too deep into the project. If the simple solution is "just use webpack" then I'd rather do that now than get months into this project with Vite and have to refactor a lot.

It's just about the way you handle imports. don't use barrel files and you may be good. the problem gets worse if you use monorepos and import many files.

maylortaylor commented 1 year ago
// Component.vue
import { a, b } from '/shared'

// instead of 
import a from '/shared/a'
import b  from '/shared/b'

Which is the best/preferred method then? Using an index.ts file with all of your files and then bring the index.ts file in like below:

// /shared/index.ts
export { a } from './a'
export { b } from './b'
export { c } from './c'

// Component.vue
import { a, b } from '/shared'

Or should I just directly import in each file like below:

// /shared/a.ts
export { a } from './a'

// /shared/b.ts
export { b } from './b'

// Component.vue
import a from '/shared/a'
import b  from '/shared/b'
100terres commented 1 year ago

@maylortaylor

should I just directly import in each file

If there's a lot of imported files in the different index files of your application, it might cause slow down in development.

The current project I'm working on, while migrating from webpack to vite, we had to change a lot of imports to from one import statement to multiple one, because our shared module had too many import statements which caused network bottleneck each time we reloaded the page. By changing the imports we've manage to make our vite setup fast.

To do the migration, we've build a couple of codemods using https://github.com/facebook/jscodeshift.

jguddas commented 1 year ago

Back in the day we use things like babel-plugin-lodash and today it hasn't really gotten much better, this is how nextjs does it.

jguddas commented 1 year ago

Maybe it is getting batter after all https://github.com/vercel/next.js/pull/54572 :crossed_fingers:

yogeshjain999 commented 1 year ago

We've started to experience same thing in our project for development environment when we removed multiple entry points and kept only one (to keep production bundle in check).

Previously, we had multiple entry points which didn't load all the modules (more than 1000) but only required ones. Although, it made production build to generate lot of chunks.

BelkaDev commented 12 months ago

Any updates on this issue? At the moment it seems like using barrel files are counter indicated in any vite project.
Webpack even has a solution for this matter. But when I tried setting side effects in the rollup config, it didn't work as intended

leonardoprimieri commented 11 months ago

Facing the same issue here, did anyone find any solution for this?

Idonoghue commented 11 months ago

We also struggle because of how many index.ts files we have. We use Dschungelabenteuer's tree-shaking plugin with a function that finds all our index.ts files to pass as entry points (ignoring the few folders that we want to keep the index.ts for). This took our JS modules imported on first load from 830 to about 260.

// inside defineConfig
const indexPaths = await findFoldersWithIndexTs(resolve(__dirname, 'src'))

return {
  plugins: [
      // for every import that goes to an index.ts replace with importing directly from the sub-file
      await EntryShakingPlugin({
        targets: indexPaths
          .map((path) => path.split('src/').pop())
          .filter((path): path is string => !!path),
      }),
  ],
  resolve: {
    alias: {
       ...Object.fromEntries(
        indexPaths.map((path) => {
           return [path.split('src/').pop(), path]
        })
      ),
    },
  },
}

async function findFoldersWithIndexTs(folderPath: string): Promise<string[]> {
  const result: string[] = []

  async function crawl(directory: string): Promise<void> {
    const entries = await fs.promises.readdir(directory, {
      withFileTypes: true,
    })

    for (const entry of entries) {
      const fullPath = path.join(directory, entry.name)

      if (entry.isDirectory()) {
        if (
          entry.name !== 'node_modules' &&
          entry.name !== '.git' &&
          entry.name !== 'pages' &&
          entry.name !== 'test-utils'
        ) {
          const folderFiles = await fs.promises.readdir(fullPath)
          if (folderFiles.includes('index.ts')) {
            result.push(fullPath)
          }
          await crawl(fullPath)
        }
      }
    }
  }

  await crawl(folderPath)
  return result
}
sk-roofr commented 11 months ago

I guess this issue is ignored .. I was so happy that HMR was working then it fetches hundreds of files.

bluwy commented 11 months ago

Putting my 2c on the issue.

What's slowing it down

Through some profiling, we found that the amount of requests isn't an exact indicator of the slowness. You can have a JS file that imports 10000 raw JS files and it will be as fast too.

The slowness is caused by the need to transform the file, e.g. .ts -> .js and .vue -> .js. And this causes an internal waterfall where imports of files take turn to transform sequentially. So while we keep our internal Vite plugins fast, third-party plugins can also attribute to the performance impact, and they are out of our control.

What can we do today

One way we can address this is by warming up some files so that they are transformed early and ready to go when requested: https://github.com/vitejs/vite/pull/14291. Here's also an in-depth explanation of why this approach. The feature however doesn't cover warming up after HMR, but definitely doable soon.

What about treeshaking

It's great that https://github.com/Dschungelabenteuer/vite-plugin-entry-shaking works too, but one of the main things I'm wary of is side-effects, as some files from the barrel export may rely on it work. Analyzing it is more expensive than warming up. But admittedly, most barrel files don't really export side-effects, so this shouldn't always apply, but it's still something we need to make correct if it were to be a core feature. (Also, not to mention risks of duplicated references of singletons with this approach.)

Conclusion

So IMO, I think we should see how https://github.com/vitejs/vite/pull/14291 works, and only go with what this issue proposed as a last resort. If you'd like to debug slow plugins today, you can do so by running DEBUG="vite:plugin-*" vite dev, but take the logs with a grain of salt as measuring async code isn't reliable, but should still surface the more intensive work done.

intelcoder commented 11 months ago

Anyone is actually using vite HMR in a large project with fetching hundreds of files? I tried warmup but it doesn't help reduce files need to fetch on load.

fdc-viktor-luft commented 10 months ago

I would like to push for a build-in option here as well. My use case are "Cypress Component Tests", which runs using the Vite dev server.

Since all test files are loaded in isolation the whole dependency tree of a tested component is being loaded. Without tree-shaking this can quickly result in downloading too many files were all requests are also intercepted by Cypress itself. This can slow down all tests significantly.

benmccann commented 10 months ago

What about treeshaking It's great that https://github.com/Dschungelabenteuer/vite-plugin-entry-shaking works too, but one of the main things I'm wary of is side-effects, as some files from the barrel export may rely on it work. Analyzing it is more expensive than warming up. But admittedly, most barrel files don't really export side-effects, so this shouldn't always apply, but it's still something we need to make correct if it were to be a core feature. (Also, not to mention risks of duplicated references of singletons with this approach.)

Couldn't we do it only when sideEffects: false or another sideEffects specification which covers the barrel file? Of course this would not help all the time as many libraries won't have that, but some help is better than none and I imagine it would encourage library developers to add it and is something we could document on the performance docs

Given that transforming is the bottleneck, I'd expect this would still help as it would greatly reduce the number of files to transform.

bluwy commented 10 months ago

I don't think the issue is within libraries (which we do support sideEffects today) as we already prebundle libraries. I think the bottleneck reported so far is in large codebases and source code where barrel files are used often.

We currently don't support sideEffects in source code (I think?) and it would be tricky to configure especially that you can't use it to tell "im not sure if this directory has side-effects, can you detect it for me?". A new field (or plugin option) would be needed to scope that down.

Plus even with side-effects configured, there's extra work needed to decipher what import { foo } from './barrel' means if:

// barrel.js
export * from './a'
export * from './b'
export * from './c'

Each file would need to be loaded and transformed anyways.

jguddas commented 10 months ago

I don't think the issue is within libraries

Example of it being an issue with libraries

  1. I run yarn add lucide-react (a library with a big barrel file as entry and "sideEffects": false).
  2. I add console.log('I should not be run') to an icon that is barrel exported node_modules/lucide-react/dist/esm/icons/bug-play.js.
  3. I add an unrelated icon import { Sticker } from 'lucide-react'; and <Sticker/> to app.ts.
  4. I start the development server.
  5. I open the browser.
  6. I open the console.
  7. I see I should not be run :(

Barrel file /lucide-react/dist/esm/lucide-react.js

https://www.npmjs.com/package/lucide-react?activeTab=code

/**
 * lucide-react v0.291.0 - ISC
 */

import * as index from './icons/index.js';
export { index as icons };
export { default as Accessibility, default as AccessibilityIcon, default as LucideAccessibility } from './icons/accessibility.js';
export { default as ActivitySquare, default as ActivitySquareIcon, default as LucideActivitySquare } from './icons/activity-square.js';
export { default as Activity, default as ActivityIcon, default as LucideActivity } from './icons/activity.js';
export { default as AirVent, default as AirVentIcon, default as LucideAirVent } from './icons/air-vent.js';
export { default as Airplay, default as AirplayIcon, default as LucideAirplay } from './icons/airplay.js';
export { default as AlarmCheck, default as AlarmCheckIcon, default as LucideAlarmCheck } from './icons/alarm-check.js';
export { default as AlarmClockOff, default as AlarmClockOffIcon, default as LucideAlarmClockOff } from './icons/alarm-clock-off.js';
export { default as AlarmClock, default as AlarmClockIcon, default as LucideAlarmClock } from './icons/alarm-clock.js';
export { default as AlarmMinus, default as AlarmMinusIcon, default as LucideAlarmMinus } from './icons/alarm-minus.js';
export { default as AlarmPlus, default as AlarmPlusIcon, default as LucideAlarmPlus } from './icons/alarm-plus.js';
export { default as Album, default as AlbumIcon, default as LucideAlbum } from './icons/album.js';
export { default as AlertCircle, default as AlertCircleIcon, default as LucideAlertCircle } from './icons/alert-circle.js';
export { default as AlertOctagon, default as AlertOctagonIcon, default as LucideAlertOctagon } from './icons/alert-octagon.js';
export { default as AlertTriangle, default as AlertTriangleIcon, default as LucideAlertTriangle } from './icons/alert-triangle.js';
export { default as AlignCenterHorizontal, default as AlignCenterHorizontalIcon, default as LucideAlignCenterHorizontal } from './icons/align-center-horizontal.js';
export { default as AlignCenterVertical, default as AlignCenterVerticalIcon, default as LucideAlignCenterVertical } from './icons/align-center-vertical.js';
export { default as AlignCenter, default as AlignCenterIcon, default as LucideAlignCenter } from './icons/align-center.js';
…
bluwy commented 10 months ago

@jguddas I don't think that is quite related to this issue? Libraries are prebundled in dev so sideEffects doesn't really work, but it doesn't cause performance issues as it prebundles as a single file and request. This issue currently focuses on the performance aspect of it.

sideEffects not working in dev is somewhat of a tradeoff, unless you add that library to optimizeDeps.exclude. If you notice this in builds, there's also an issue for that: https://github.com/vitejs/vite/issues/14558

MathiasWP commented 9 months ago

I understand that the following barrel file is difficult to transform:

export * from './path-1';
export * from './path-2';

But why are barrel files with named exports slow? Can't Vite see exactly where the import is coming from?

export { thingy, thing } from './path-1';
export { something, somethingy } from './path-2';
bluwy commented 9 months ago

It's because of side-effects as well (https://github.com/vitejs/vite/issues/8237#issuecomment-1734949491). That plus the * combined makes analyzing hard in general.

MathiasWP commented 9 months ago

How does barrel files affect performance in production? Will importing one module from a barrel file result in all modules from the barrel file being imported?

Shakeskeyboarde commented 9 months ago

How does barrel files affect performance in production? Will importing one module from a barrel file result in all modules from the barrel file being imported?

Nope. Tree shaking is done in production, as part of the bundle generation process. It's in dev where individual source files are transpiled but not bundled that barrel files result in a large number of client/browser side fetches for unused code.

jmroon commented 9 months ago

How does barrel files affect performance in production? Will importing one module from a barrel file result in all modules from the barrel file being imported?

Nope. Tree shaking is done in production, as part of the bundle generation process. It's in dev where individual source files are transpiled but not bundled that barrel files result in a large number of client/browser side fetches for unused code.

I'm seeing that they ARE being included in production, at least in my case.

I have a barrel file in a monorepo which is re-exporting Vue components from a 3rd party library (PrimeVue). My intention is to alias the components and centralize their usage, so that I can easily swap to internally developed components if needed. As soon as I include a Data Table for example, but do not import it anywhere in my app, my production bundle grows by 31kB.

image

If you're wondering why I'm doing this, it's for a few reasons. As mentioned above, aliasing the imports is useful as PrimeVue components are unfortunately named without a prefix. This also makes swapping to internally developed components easier. And there's the obvious benefit of discoverability. Unfortunately in a monorepo I'm unable to leverage unplugin-vue-components as that will not provide type defs to my libraries.

bluwy commented 9 months ago

Note that this issue focuses on treeshaking and barrel files in dev, not in production. If they are issues in production, please open another issue.

stanleyxu2005 commented 8 months ago

I understand that the following barrel file is difficult to transform:

export * from './path-1';
export * from './path-2';

But why are barrel files with named exports slow? Can't Vite see exactly where the import is coming from?

export { thingy, thing } from './path-1';
export { something, somethingy } from './path-2';

I hijack your reply to share my two cents in general of best practise for treeshaking.

I really dont encourage to explicitly import certain functions. It drops the benefit of the namespace concept.

// How would I know, `map(...)` function belongs to lodash
import { map } from 'lodash-es' 

// It's more straightfoward, that I'm using lodash, when I call `_.map(...)`
import * as _ from 'lodash-es' 

Maybe cherry-picking of used functions is a doable solution for now, but I would rather importing/exporting everything and let build tool to find a way to keep the used functions. If a human being can do, so can a build tool. (maybe some GPT4 based build tool can breakthrougth in the near future)

3pleFly commented 6 months ago

Man I can't wait for this to work :). For now we decided to remove all the barrel files from our small project.

KMJ-007 commented 3 months ago

facing same issue

Shakeskeyboarde commented 2 months ago

If you're here because barrel files make the dev server very slow, and you've basically wanted something like vite preview but with building and watching, then you might be interested in the following tool I wrote for myself:

Hope this helps some of you. Feel free to post issues on the repo or contribute.

benmccann commented 1 month ago

I don't think the issue is within libraries (which we do support sideEffects today) as we already prebundle libraries.

I think it can be at times. E.g. consider some of the Svelte icon libraries we've seen with thousands of .svelte files. They take quite awhile to prebundle since we have to call out to a JS library and can't prebundle them natively with esbuild. Perhaps if we new which imports were used and knew there were no sideEffects we could prebundle only those components rather than all of them.

We currently don't support sideEffects in source code (I think?) and it would be tricky to configure especially that you can't use it to tell "im not sure if this directory has side-effects, can you detect it for me?". A new field (or plugin option) would be needed to scope that down.

The sideEffects field in package.json should already cover this. It can be used to say that a certain directory is side-effects free. If not covered by that field then you assume there can be side-effects and don't try to detect whether or not there are, which I'm not sure would be possible.

Each file would need to be loaded and transformed anyways.

That may be true currently. Though I imagine we could add something that works like the scanning from dependency prebundling that just quickly looks for imports and builds a graph of the project. That may be easier in a rolldown world where we can more easily share code between prebundling and building.

budarin commented 1 month ago

Unfortunately, sometimes the recommendation to avoid using Barrel Files cannot be fulfilled.

We have a layered architecture with static DI. During the initialization of the application, for example, we must pass a link to the UseCases layer to the UI layer. A link is naturally an Barrel File as an instance of the UseCases layer. The UI layer cannot and should not know anything about the UseCases layer, so there can be no question of any access to specific UseCases modules. There may be several such dependencies with multiple re-exports in the project: Domain, Store, UseCases, Providers ...

// di.mts
...
export let useCases: UseCases; // <== link to Barrel File

export function initUIUseCases(uc: UseCases) {
  useCases = uc;
}
...

the application initializes it with a real link at startup

// app.mts
import useCases from '../use-cases/index.mts'; // <== Barrel File
import { initUIUseCases } from '../ui/di.mts';

initUIUseCases(useCases);
...

and then it's used in container as

import { useCases } from '../ui/di.mts'; 

function Container() {
   const data = useCases.useUCase1();

   return { ...}
}

export default UserContainer;

It seems to me that a possible solution could be a hint to vite which dependencies in the form of Barrel Files should be preprocessed to resolve real modules or developer can prepare a module which resolves the dependencies for dev server and internally the resolving result should be


useCases.useCase1 -> /src/use-cases/useCase1.mts