fix: skip the plugin if it has been called before with the same id and importer #19016
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sapphi-red
added
the
p3-downstream-blocker
patak-dev
previously approved these changes
Dec 20, 2024
hi-ogawa
reviewed
Dec 20, 2024
Comment on lines
+589
to
+593
| if (sameCallIndex !== -1) { | ||
| skipCallsTemp[sameCallIndex] = { | ||
| ...skipCallsTemp[sameCallIndex], | ||
| called: true, | ||
| } |
Contributor
There was a problem hiding this comment.
Awesome investigation!
The fix looks good to merge, but while trying to understand the logic, I cannot tell why we cannot immediately do return null for such call as this seems to be destined to go resolve loop.
The new test case seems to pass with "just return null". Do you have a case such approach fails?
Member
Author
There was a problem hiding this comment.
I added a test case that fails with return null: 67c4a66
hi-ogawa
approved these changes
Dec 21, 2024
Contributor
hi-ogawa
left a comment
hi-ogawa
left a comment
There was a problem hiding this comment.
Thanks!
Call flow is still mind bending to me, but I'll try to understand this in a different time 😅
danielroe
added a commit
to nuxt/nuxt
that referenced
this pull request
Dec 23, 2024
danielroe
added a commit
to nuxt/nuxt
that referenced
this pull request
Dec 23, 2024
patak-dev
approved these changes
Jan 2, 2025
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)
renovate bot
added a commit
to andrei-picus-tink/auto-renovate
that referenced
this pull request
Jan 5, 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)
sapphi-red
deleted the
fix/skip-the-plugin-if-it-has-been-called-before-with-the-same-id-and-importer
branch
Kamsou
pushed a commit
to Kamsou/nuxt
that referenced
this pull request
Feb 5, 2025
moonlitusun
pushed a commit
to moonlitusun/vite
that referenced
this pull request
May 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
#18903 changed the
skipSelfbehavior to align with rollup.In Nuxt + Vitest scenario, a recursive call happened because of that change (it happens with rollup: rollup/rollup#5768).
Nuxt has
nuxt:resolve-bare-importsplugin that callsthis.resolve(id, nuxtRoot)and Vitest hasvitest:resolve-coreplugin that callsthis.resolve(id, viteIndexHtml).These plugins were calling each other after #18903.
This didn't happen before #18903 because the plugins were skipped even if the
importeris different.In this PR,
this.resolve(id, importer)with the same id and importer will be skipped when it's called inside the sameidandimportereven if it's called inside a differentthis.resolve.For example,
this.resolve('foo', 'bar')->this.resolve('foo', 'bar')was skipped before this PR.this.resolve('foo', 'bar')->this.resolve('foo', 'bar2')->this.resolve('foo', 'bar')was not skipped before this PR, but now this case is also skipped.fixes this CI failure in nuxt: https://github.com/nuxt/nuxt/actions/runs/12418711683/job/34672477920?pr=30306
refs #18903