fix: this.resolve skipSelf should not skip for different id or import (#18903)

Author: sapphi-red

packages/vite/src/node/index.ts

@@ -145,7 +145,7 @@ export type {
   WebSocketClient,
   WebSocketCustomListener,
 } from './server/ws'
-export type { PluginContainer } from './server/pluginContainer'
+export type { SkipInformation, PluginContainer } from './server/pluginContainer'
 export type {
   EnvironmentModuleGraph,
   EnvironmentModuleNode,

packages/vite/src/node/server/__tests__/pluginContainer.spec.ts

@@ -213,6 +213,53 @@ describe('plugin container', () => {
       expect(result.code).equals('3')
     })
   })
+
+  describe('resolveId', () => {
+    describe('skipSelf', () => {
+      it('should skip the plugin itself when skipSelf is true', async () => {
+        let calledCount = 0
+        const plugin: Plugin = {
+          name: 'p1',
+          async resolveId(id, importer) {
+            calledCount++
+            if (calledCount <= 1) {
+              return await this.resolve(id, importer, { skipSelf: true })
+            }
+            return id
+          },
+        }
+
+        const environment = await getDevEnvironment({ plugins: [plugin] })
+        await environment.pluginContainer.resolveId('/x.js')
+        expect(calledCount).toBe(1)
+      })
+
+      it('should skip the plugin only when id and importer is same', async () => {
+        const p1: Plugin = {
+          name: 'p1',
+          async resolveId(id, importer) {
+            if (id === 'foo/modified') {
+              return 'success'
+            }
+            return await this.resolve(id, importer, { skipSelf: true })
+          },
+        }
+        const p2: Plugin = {
+          name: 'p2',
+          async resolveId(id, importer) {
+            const resolved = await this.resolve(id + '/modified', importer, {
+              skipSelf: true,
+            })
+            return resolved ?? 'failed'
+          },
+        }
+
+        const environment = await getDevEnvironment({ plugins: [p1, p2] })
+        const result = await environment.pluginContainer.resolveId('foo')
+        expect(result).toStrictEqual({ id: 'success' })
+      })
+    })
+  })
 })
 
 async function getDevEnvironment(

packages/vite/src/node/server/pluginContainer.ts

@@ -144,6 +144,12 @@ export async function createEnvironmentPluginContainer(
   return container
 }
 
+export type SkipInformation = {
+  id: string
+  importer: string | undefined
+  plugin: Plugin
+}
+
 class EnvironmentPluginContainer {
   private _pluginContextMap = new Map<Plugin, PluginContext>()
   private _resolvedRollupOptions?: InputOptions
@@ -336,7 +342,9 @@ class EnvironmentPluginContainer {
     options?: {
       attributes?: Record<string, string>
       custom?: CustomPluginOptions
+      /** @deprecated use `skipCalls` instead */
       skip?: Set<Plugin>
+      skipCalls?: readonly SkipInformation[]
       /**
        * @internal
        */
@@ -349,17 +357,25 @@ class EnvironmentPluginContainer {
       await this._buildStartPromise
     }
     const skip = options?.skip
+    const skipCalls = options?.skipCalls
     const scan = !!options?.scan
     const ssr = this.environment.config.consumer === 'server'
-    const ctx = new ResolveIdContext(this, skip, scan)
+    const ctx = new ResolveIdContext(this, skip, skipCalls, scan)
+
+    const mergedSkip = new Set<Plugin>(skip)
+    for (const call of skipCalls ?? []) {
+      if (call.id === rawId && call.importer === importer) {
+        mergedSkip.add(call.plugin)
+      }
+    }
 
     const resolveStart = debugResolve ? performance.now() : 0
     let id: string | null = null
     const partial: Partial<PartialResolvedId> = {}
     for (const plugin of this.getSortedPlugins('resolveId')) {
       if (this._closed && this.environment.config.dev.recoverable)
         throwClosedServerError()
-      if (skip?.has(plugin)) continue
+      if (mergedSkip?.has(plugin)) continue
 
       ctx._plugin = plugin
 
@@ -534,6 +550,7 @@ class PluginContext implements Omit<RollupPluginContext, 'cache'> {
   _activeId: string | null = null
   _activeCode: string | null = null
   _resolveSkips?: Set<Plugin>
+  _resolveSkipCalls?: readonly SkipInformation[]
   meta: RollupPluginContext['meta']
   environment: Environment
 
@@ -559,16 +576,19 @@ class PluginContext implements Omit<RollupPluginContext, 'cache'> {
       skipSelf?: boolean
     },
   ) {
-    let skip: Set<Plugin> | undefined
-    if (options?.skipSelf !== false) {
-      skip = new Set(this._resolveSkips)
-      skip.add(this._plugin)
-    }
+    const skipCalls =
+      options?.skipSelf === false
+        ? this._resolveSkipCalls
+        : [
+            ...(this._resolveSkipCalls || []),
+            { id, importer, plugin: this._plugin },
+          ]
     let out = await this._container.resolveId(id, importer, {
       attributes: options?.attributes,
       custom: options?.custom,
       isEntry: !!options?.isEntry,
-      skip,
+      skip: this._resolveSkips,
+      skipCalls,
       scan: this._scan,
     })
     if (typeof out === 'string') out = { id: out }
@@ -794,10 +814,12 @@ class ResolveIdContext extends PluginContext {
   constructor(
     container: EnvironmentPluginContainer,
     skip: Set<Plugin> | undefined,
+    skipCalls: readonly SkipInformation[] | undefined,
     scan: boolean,
   ) {
     super(null!, container)
     this._resolveSkips = skip
+    this._resolveSkipCalls = skipCalls
     this._scan = scan
   }
 }
@@ -999,7 +1021,9 @@ class PluginContainer {
     options?: {
       attributes?: Record<string, string>
       custom?: CustomPluginOptions
+      /** @deprecated use `skipCalls` instead */
       skip?: Set<Plugin>
+      skipCalls?: readonly SkipInformation[]
       ssr?: boolean
       /**
        * @internal