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

perf: skip globbing for static path in warmup #19107

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Conversation

antfu
Copy link
Member

@antfu antfu commented Dec 31, 2024

Context: nuxt/nuxt#30407 (comment)

Fix nuxt/nuxt#30407

In Nuxt, the entry we passed to warm up is an absolute path inside node_modules.

I injected into tinyglobby and this is what passed to tinyglobby:

{ 
  absolute: true,                                                                                                  
  cwd: '/Users/antfu/i/nuxt-i18n/playground/app',
  expandDirectories: false,
  ignore: [ '**/.git/**', '**/node_modules/**' ],
  patterns: [ '/Users/antfu/i/nuxt-i18n/node_modules/.pnpm/[email protected]_@[email protected]_@[email protected][email protected][email protected][email protected]__365piwb2r26wv7nx7oqnj3cmpa/node_modules/nuxt/dist/app/entry.js' ]
}

This caused tinyglobby to glob every file inside node_modules. Consider this could be a bug of tinyglobby, it's also something can be optimized on the Vite side by excluding non-glob inputs.

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍 Maybe this change should also be done or discussed in tinyglobby side?

@benmccann
Copy link
Collaborator

@SuperchupuDev has been working on some major performance improvements (SuperchupuDev/tinyglobby#76). Just waiting for fdir to cut a new release (thecodrr/fdir#131). Hopefully it will be addressed in tinyglobby very shortly...

@patak-dev patak-dev merged commit 677508b into main Jan 2, 2025
24 checks passed
@patak-dev patak-dev deleted the perf/warmup-glob branch January 2, 2025 19:43
@userquin
Copy link
Contributor

userquin commented Jan 2, 2025

It works, thx @antfu for the fix and @patak-dev for the release ❤️

Starting Nuxt playground with Vite 6.0.7 from 9.2 seconds to 2.6 seconds (with client server warmup):

imagen

renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 3, 2025
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.6 | 6.0.7 |


## [v6.0.7](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small607-2025-01-02-small)

-   fix: fix `minify` when `builder.sharedPlugins: true` ([#19025](vitejs/vite#19025)) ([f7b1964](vitejs/vite@f7b1964)), closes [#19025](vitejs/vite#19025)
-   fix: skip the plugin if it has been called before with the same id and importer ([#19016](vitejs/vite#19016)) ([b178c90](vitejs/vite@b178c90)), closes [#19016](vitejs/vite#19016)
-   fix(html): error while removing `vite-ignore` attribute for inline script ([#19062](vitejs/vite#19062)) ([a492253](vitejs/vite@a492253)), closes [#19062](vitejs/vite#19062)
-   fix(ssr): fix semicolon injection by ssr transform ([#19097](vitejs/vite#19097)) ([1c102d5](vitejs/vite@1c102d5)), closes [#19097](vitejs/vite#19097)
-   perf: skip globbing for static path in warmup ([#19107](vitejs/vite#19107)) ([677508b](vitejs/vite@677508b)), closes [#19107](vitejs/vite#19107)
-   feat(css): show lightningcss warnings ([#19076](vitejs/vite#19076)) ([b07c036](vitejs/vite@b07c036)), closes [#19076](vitejs/vite#19076)
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 4, 2025
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | vite    | 6.0.6 | 6.0.7 |


## [v6.0.7](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small607-2025-01-02-small)

-   fix: fix `minify` when `builder.sharedPlugins: true` ([#19025](vitejs/vite#19025)) ([f7b1964](vitejs/vite@f7b1964)), closes [#19025](vitejs/vite#19025)
-   fix: skip the plugin if it has been called before with the same id and importer ([#19016](vitejs/vite#19016)) ([b178c90](vitejs/vite@b178c90)), closes [#19016](vitejs/vite#19016)
-   fix(html): error while removing `vite-ignore` attribute for inline script ([#19062](vitejs/vite#19062)) ([a492253](vitejs/vite@a492253)), closes [#19062](vitejs/vite#19062)
-   fix(ssr): fix semicolon injection by ssr transform ([#19097](vitejs/vite#19097)) ([1c102d5](vitejs/vite@1c102d5)), closes [#19097](vitejs/vite#19097)
-   perf: skip globbing for static path in warmup ([#19107](vitejs/vite#19107)) ([677508b](vitejs/vite@677508b)), closes [#19107](vitejs/vite#19107)
-   feat(css): show lightningcss warnings ([#19076](vitejs/vite#19076)) ([b07c036](vitejs/vite@b07c036)), closes [#19076](vitejs/vite#19076)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants