chore: squash + rebase

This commit improves the parity of behaviour between 'vite build' and
'vite dev' when Rollup plugins retuning non-null 'meta' properties
from resolveId() hooks are used.

If one takes the Rollup behaviour to be any kind of reference in these
cases, this is fixing a small bug in Vite, which was in a very large
part already fixed by PR vitejs#5465.

Here's an explanation of the faulty behaviour:

The normalizeUrl() helper calls user plugins's resolveId() twice.

- Once with the URL string containing a query suffix.  Here it
  _ignores_ the meta reply.

- Again, with the URL no longer containing said suffix.  Here it
  doesn't ignore the meta reply.  It is stored in the moduleGraph
  node.

As can be seen, if the user's plugin meta reply depends on the query
suffix being passed in the imported URL, that meta reply won't be
registered correctly in the module graph and is not retrievable via
getModuleInfo() later on.

This change makes it so that the first call doesn't ignore the meta
reply. This makes Vite's dev server match Rollup's plugin-calling
behaviour.

Fixes vitejs#6766

packages/vite/src/node/plugins/importAnalysis.ts

@@ -51,6 +51,7 @@ import {
   shouldExternalizeForSSR
 } from '../ssr/ssrExternal'
 import { transformRequest } from '../server/transformRequest'
+import type { ResolvedUrl } from '../server/moduleGraph'
 import {
   getDepsCacheDirPrefix,
   getDepsOptimizer,
@@ -262,7 +263,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
       const normalizeUrl = async (
         url: string,
         pos: number
-      ): Promise<[string, string]> => {
+      ): Promise<ResolvedUrl> => {
         if (base !== '/' && url.startsWith(base)) {
           url = url.replace(base, '/')
         }
@@ -292,7 +293,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
         if (!resolved) {
           // in ssr, we should let node handle the missing modules
           if (ssr) {
-            return [url, url]
+            return [url, url, null]
           }
           return this.error(
             `Failed to resolve import "${url}" from "${path.relative(
@@ -323,7 +324,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
         }
 
         if (isExternalUrl(url)) {
-          return [url, url]
+          return [url, url, resolved.meta]
         }
 
         // if the resolved id is not a valid browser import specifier,
@@ -359,9 +360,8 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           // up-to-date version of this module.
           try {
             // delay setting `isSelfAccepting` until the file is actually used (#7870)
-            const depModule = await moduleGraph.ensureEntryFromUrl(
-              unwrapId(url),
-              ssr,
+            const depModule = moduleGraph.ensureEntryFromResolved(
+              [unwrapId(url), resolved.id, resolved.meta],
               canSkipImportAnalysis(url)
             )
             if (depModule.lastHMRTimestamp > 0) {
@@ -378,7 +378,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           url = base + url.replace(/^\//, '')
         }
 
-        return [url, resolved.id]
+        return [url, resolved.id, resolved.meta]
       }
 
       for (let index = 0; index < imports.length; index++) {
@@ -473,11 +473,24 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           }
 
           // normalize
-          const [url, resolvedId] = await normalizeUrl(specifier, start)
+          const [url, resolvedId, resolvedMeta] = await normalizeUrl(
+            specifier,
+            start
+          )
 
           // record as safe modules
-          server?.moduleGraph.safeModulesPath.add(fsPathFromUrl(url))
+          moduleGraph.safeModulesPath.add(fsPathFromUrl(url))
+
+          // ensure module is in the graph under the correct
+          // resolvedId and sporting correct meta properties.
+          const mod = moduleGraph.getModuleById(resolvedId)
+          if (mod) {
+            mod.meta = { ...mod.meta, ...resolvedMeta }
+          } else {
+            moduleGraph.ensureEntryFromResolved([url, resolvedId, resolvedMeta])
+          }
 
+          // rewrite
           if (url !== specifier) {
             let rewriteDone = false
             if (

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

@@ -5,6 +5,8 @@ import type { Plugin } from '../../plugin'
 import { ModuleGraph } from '../moduleGraph'
 import type { PluginContainer } from '../pluginContainer'
 import { createPluginContainer } from '../pluginContainer'
+import { importAnalysisPlugin } from '../../plugins/importAnalysis'
+import type { ViteDevServer } from '..'
 
 let resolveId: (id: string) => any
 let moduleGraph: ModuleGraph
@@ -68,6 +70,44 @@ describe('plugin container', () => {
       expect(metaArray).toEqual([{ x: 1 }, { x: 2 }, { x: 3 }])
     })
 
+    it('preserves metadata calculated from import query string', async () => {
+      const entryUrl = '/main.js'
+      const testedId = 'x.js'
+
+      const plugin: Plugin = {
+        name: 'p1',
+        resolveId(url) {
+          if (url === entryUrl) {
+            return url
+          }
+          const [id, query] = url.split('?')
+          const match = query?.match(/test=([^&]+)/)
+          if (match) {
+            return { id, meta: { test: +match[1] } }
+          }
+        },
+        load(id) {
+          if (id === entryUrl) {
+            return `import "${testedId}?test=1"`
+          } else if (id === testedId) {
+            const meta = this.getModuleInfo(id).meta
+            expect(meta.test).toBe(1)
+            return ''
+          }
+        }
+      }
+
+      const container = await getPluginContainer({ plugins: [plugin] })
+      await moduleGraph.ensureEntryFromUrl(entryUrl, false)
+      await container.transform(
+        (await container.load(entryUrl)) as any,
+        entryUrl
+      )
+
+      await container.load(testedId)
+      expect.assertions(1)
+    })
+
     it('can pass metadata between plugins', async () => {
       const entryUrl = '/x.js'
 
@@ -153,13 +193,21 @@ async function getPluginContainer(
   inlineConfig?: UserConfig
 ): Promise<PluginContainer> {
   const config = await resolveConfig(
-    { configFile: false, ...inlineConfig },
+    {
+      configFile: false,
+      server: { preTransformRequests: false },
+      ...inlineConfig
+    },
     'serve'
   )
 
   // @ts-ignore: This plugin requires a ViteDevServer instance.
   config.plugins = config.plugins.filter((p) => !/pre-alias/.test(p.name))
 
+  // @ts-ignore: So does this one and this mock one seems to work
+  const iap = config.plugins.find((p) => p.name === 'vite:import-analysis')
+  iap.configureServer(<ViteDevServer>{ moduleGraph })
+
   resolveId = (id) => container.resolveId(id)
   const container = await createPluginContainer(config, moduleGraph)
   return container

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

@@ -149,7 +149,7 @@ export class ModuleGraph {
     for (const imported of importedModules) {
       const dep =
         typeof imported === 'string'
-          ? await this.ensureEntryFromUrl(imported, ssr)
+          ? this.urlToModuleMap.get(imported)!
           : imported
       dep.importers.add(mod)
       nextImports.add(dep)
@@ -179,13 +179,11 @@ export class ModuleGraph {
     return noLongerImported
   }
 
-  async ensureEntryFromUrl(
-    rawUrl: string,
-    ssr?: boolean,
+  ensureEntryFromResolved(
+    [url, resolvedId, meta]: ResolvedUrl,
     setIsSelfAccepting = true
-  ): Promise<ModuleNode> {
-    const [url, resolvedId, meta] = await this.resolveUrl(rawUrl, ssr)
-    let mod = this.idToModuleMap.get(resolvedId)
+  ): ModuleNode {
+    let mod = this.urlToModuleMap.get(url)
     if (!mod) {
       mod = new ModuleNode(url, setIsSelfAccepting)
       if (meta) mod.meta = meta
@@ -208,6 +206,15 @@ export class ModuleGraph {
     return mod
   }
 
+  async ensureEntryFromUrl(
+    rawUrl: string,
+    ssr?: boolean,
+    setIsSelfAccepting = true
+  ): Promise<ModuleNode> {
+    const resolvedUrl = await this.resolveUrl(rawUrl, ssr)
+    return this.ensureEntryFromResolved(resolvedUrl, setIsSelfAccepting)
+  }
+
   // some deps, like a css file referenced via @import, don't have its own
   // url because they are inlined into the main css import. But they still
   // need to be represented in the module graph so that they can trigger