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

Consider treeshaking module for serving files in dev mode #8237

Open
4 tasks done
AlexandreBonaventure opened this issue May 19, 2022 · 42 comments
Open
4 tasks done

Consider treeshaking module for serving files in dev mode #8237

AlexandreBonaventure opened this issue May 19, 2022 · 42 comments
Labels
enhancement New feature or request p4-important Violate documented behavior or significantly improves performance (priority) performance Performance related enhancement

Comments

@AlexandreBonaventure
Copy link
Contributor

AlexandreBonaventure commented May 19, 2022

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:
#8232
and I totally agree with what @patak-dev says here:
#7608 (comment)

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: #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
Copy link

yaldram commented May 20, 2022

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

Aaron-Pool commented May 20, 2022

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

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

bellizio commented Jul 6, 2022

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

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

Miofly commented Jul 30, 2022

has a good way to solve this problem now?

@Dschungelabenteuer
Copy link

Dschungelabenteuer commented Oct 20, 2022

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

ryan-mcginty-alation commented Apr 3, 2023

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

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

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

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

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

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

maylortaylor commented Jul 18, 2023

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

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

jguddas commented Aug 13, 2023

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

jguddas commented Aug 26, 2023

Maybe it is getting batter after all vercel/next.js#54572 🤞

@yogeshjain999
Copy link

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

BelkaDev commented Aug 30, 2023

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

leonardoprimieri commented Sep 8, 2023

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

@vikingair
Copy link

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

benmccann commented Nov 1, 2023

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

bluwy commented Nov 1, 2023

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

This comment has been minimized.

@bluwy

This comment has been minimized.

@MathiasWP
Copy link

MathiasWP commented Nov 7, 2023

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

bluwy commented Nov 7, 2023

It's because of side-effects as well (#8237 (comment)). That plus the * combined makes analyzing hard in general.

@MathiasWP

This comment has been minimized.

@Shakeskeyboarde

This comment has been minimized.

@jmroon

This comment has been minimized.

@bluwy

This comment has been minimized.

@stanleyxu2005
Copy link

stanleyxu2005 commented Dec 4, 2023

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

3pleFly commented Feb 14, 2024

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

KMJ-007 commented May 15, 2024

facing same issue

@Shakeskeyboarde
Copy link
Contributor

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

benmccann commented Jul 10, 2024

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

budarin commented Jul 19, 2024

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

@tufailra97
Copy link

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
}

@Idonoghue did you manage to get this working with import alias?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p4-important Violate documented behavior or significantly improves performance (priority) performance Performance related enhancement
Projects
None yet
Development

No branches or pull requests