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

fix: optimizeDeps.entries with literal-only pattern(s) #15853

Merged
merged 3 commits into from Feb 21, 2024

Conversation

Jinjiang
Copy link
Contributor

@Jinjiang Jinjiang commented Feb 9, 2024

Description

Skip running fast-glob if optimizeDeps.entries is specified without any special characters like $^*+?[]().

So user can specify a entry in literal value to skip the fast-glob ignore policy.

Additional context

#15613 (reply in thread)
#15746


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link

stackblitz bot commented Feb 9, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@Plumbiu
Copy link
Contributor

Plumbiu commented Feb 10, 2024

I think fast-glob already provides this method: https://github.com/mrmlnc/fast-glob?tab=readme-ov-file#isdynamicpatternpattern-options

patak-dev
patak-dev previously approved these changes Feb 14, 2024
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

This sounds good to me. An alternative would be to let the user specify a pattern that has node_modules, in which case node_modules won't be ignored. Maybe we could also do that in a future PR if needed. But your solution is better for now.

@patak-dev
Copy link
Member

/ecosystem-ci run

@patak-dev patak-dev added feat: dev dev server p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) labels Feb 14, 2024
@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on 61687b0: Open

suite result latest scheduled
analogjs failure success
astro success success
histoire failure failure
ladle success success
laravel success success
marko success success
nuxt success success
nx failure failure
previewjs success success
qwik failure failure
rakkas success success
sveltekit success success
unocss success success
vike failure failure
vite-plugin-pwa success success
vite-plugin-react success success
vite-plugin-react-pages success success
vite-plugin-react-swc success success
vite-plugin-svelte success success
vite-plugin-vue success success
vite-setup-catalogue success success
vitepress success success
vitest success success

@patak-dev
Copy link
Member

The AnalogJS fail is unrelated to this PR.

@Jinjiang
Copy link
Contributor Author

So I guess this is ready to merge? @patak-dev 🙂

@patak-dev
Copy link
Member

We wait for two approvals for this kind of PR.

bluwy
bluwy previously approved these changes Feb 21, 2024
packages/vite/src/node/optimizer/scan.ts Outdated Show resolved Hide resolved
@Jinjiang Jinjiang dismissed stale reviews from bluwy and patak-dev via 0a56639 February 21, 2024 13:05
@patak-dev patak-dev merged commit 49300b3 into vitejs:main Feb 21, 2024
10 checks passed
@Jinjiang Jinjiang deleted the jinjiang/fix/get-scan-entries-2 branch February 21, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: dev dev server p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants