fix: ensure version query for direct node_modules imports (#9848)

Author: patak-cat

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

@@ -8,7 +8,12 @@ import type {
 } from '..'
 import type { Plugin } from '../plugin'
 import { createIsConfiguredAsSsrExternal } from '../ssr/ssrExternal'
-import { bareImportRE, isOptimizable, moduleListContains } from '../utils'
+import {
+  bareImportRE,
+  cleanUrl,
+  isOptimizable,
+  moduleListContains
+} from '../utils'
 import { getDepsOptimizer } from '../optimizer'
 import { tryOptimizedResolve } from './resolve'
 
@@ -48,7 +53,7 @@ export function preAliasPlugin(config: ResolvedConfig): Plugin {
           })
           if (resolved && !depsOptimizer.isOptimizedDepFile(resolved.id)) {
             const optimizeDeps = depsOptimizer.options
-            const resolvedId = resolved.id
+            const resolvedId = cleanUrl(resolved.id)
             const isVirtual = resolvedId === id || resolvedId.includes('\0')
             if (
               !isVirtual &&

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

@@ -6,9 +6,11 @@ import { resolve as _resolveExports } from 'resolve.exports'
 import { hasESMSyntax } from 'mlly'
 import type { Plugin } from '../plugin'
 import {
+  CLIENT_ENTRY,
   DEFAULT_EXTENSIONS,
   DEFAULT_MAIN_FIELDS,
   DEP_VERSION_RE,
+  ENV_ENTRY,
   FS_PREFIX,
   OPTIMIZABLE_ENTRY_RE,
   SPECIAL_QUERY_RE
@@ -42,12 +44,17 @@ import type { SSROptions } from '..'
 import type { PackageCache, PackageData } from '../packages'
 import { loadPackageData, resolvePackageData } from '../packages'
 
+const normalizedClientEntry = normalizePath(CLIENT_ENTRY)
+const normalizedEnvEntry = normalizePath(ENV_ENTRY)
+
 // special id for paths marked with browser: false
 // https://github.com/defunctzombie/package-browser-field-spec#ignore-a-module
 export const browserExternalId = '__vite-browser-external'
 // special id for packages that are optional peer deps
 export const optionalPeerDepId = '__vite-optional-peer-dep'
 
+const nodeModulesInPathRE = /(^|\/)node_modules\//
+
 const isDebug = process.env.DEBUG
 const debug = createDebugger('vite:resolve-details', {
   onlyWhenFocused: true
@@ -155,14 +162,38 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
         return optimizedPath
       }
 
+      const ensureVersionQuery = (resolved: string): string => {
+        if (
+          !options.isBuild &&
+          depsOptimizer &&
+          !(
+            resolved === normalizedClientEntry ||
+            resolved === normalizedEnvEntry
+          )
+        ) {
+          // Ensure that direct imports of node_modules have the same version query
+          // as if they would have been imported through a bare import
+          // Use the original id to do the check as the resolved id may be the real
+          // file path after symlinks resolution
+          const isNodeModule = !!normalizePath(id).match(nodeModulesInPathRE)
+          if (isNodeModule && !resolved.match(DEP_VERSION_RE)) {
+            const versionHash = depsOptimizer.metadata.browserHash
+            if (versionHash && OPTIMIZABLE_ENTRY_RE.test(resolved)) {
+              resolved = injectQuery(resolved, `v=${versionHash}`)
+            }
+          }
+        }
+        return resolved
+      }
+
       // explicit fs paths that starts with /@fs/*
       if (asSrc && id.startsWith(FS_PREFIX)) {
         const fsPath = fsPathFromId(id)
         res = tryFsResolve(fsPath, options)
         isDebug && debug(`[@fs] ${colors.cyan(id)} -> ${colors.dim(res)}`)
         // always return here even if res doesn't exist since /@fs/ is explicit
         // if the file doesn't exist it should be a 404
-        return res || fsPath
+        return ensureVersionQuery(res || fsPath)
       }
 
       // URL
@@ -171,7 +202,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
         const fsPath = path.resolve(root, id.slice(1))
         if ((res = tryFsResolve(fsPath, options))) {
           isDebug && debug(`[url] ${colors.cyan(id)} -> ${colors.dim(res)}`)
-          return res
+          return ensureVersionQuery(res)
         }
       }
 
@@ -201,26 +232,6 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
           return normalizedFsPath
         }
 
-        const pathFromBasedir = normalizedFsPath.slice(basedir.length)
-        if (pathFromBasedir.startsWith('/node_modules/')) {
-          // normalize direct imports from node_modules to bare imports, so the
-          // hashing logic is shared and we avoid duplicated modules #2503
-          const bareImport = pathFromBasedir.slice('/node_modules/'.length)
-          if (
-            (res = tryNodeResolve(
-              bareImport,
-              importer,
-              options,
-              targetWeb,
-              depsOptimizer,
-              ssr
-            )) &&
-            res.id.startsWith(normalizedFsPath)
-          ) {
-            return res
-          }
-        }
-
         if (
           targetWeb &&
           (res = tryResolveBrowserMapping(fsPath, importer, options, true))
@@ -229,6 +240,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
         }
 
         if ((res = tryFsResolve(fsPath, options))) {
+          res = ensureVersionQuery(res)
           isDebug &&
             debug(`[relative] ${colors.cyan(id)} -> ${colors.dim(res)}`)
           const pkg = importer != null && idToPkgMap.get(importer)
@@ -250,7 +262,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
         if ((res = tryFsResolve(fsPath, options))) {
           isDebug &&
             debug(`[drive-relative] ${colors.cyan(id)} -> ${colors.dim(res)}`)
-          return res
+          return ensureVersionQuery(res)
         }
       }
 
@@ -260,7 +272,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {
         (res = tryFsResolve(id, options))
       ) {
         isDebug && debug(`[fs] ${colors.cyan(id)} -> ${colors.dim(res)}`)
-        return res
+        return ensureVersionQuery(res)
       }
 
       // external
@@ -405,7 +417,7 @@ function tryFsResolve(
 
   let res: string | undefined
 
-  // if we fould postfix exist, we should first try resolving file with postfix. details see #4703.
+  // if there is a postfix, try resolving it as a complete path first (#4703)
   if (
     postfix &&
     (res = tryResolveFile(

playground/optimize-deps/__tests__/optimize-deps.spec.ts

@@ -146,6 +146,12 @@ test('flatten id should generate correctly', async () => {
   expect(await page.textContent('.clonedeep-dot')).toBe('clonedeep-dot')
 })
 
+test('non optimized module is not duplicated', async () => {
+  expect(
+    await page.textContent('.non-optimized-module-is-not-duplicated')
+  ).toBe('from-absolute-path, from-relative-path')
+})
+
 test.runIf(isServe)('error on builtin modules usage', () => {
   expect(browserLogs).toEqual(
     expect.arrayContaining([

playground/optimize-deps/dep-non-optimized/index.js

@@ -0,0 +1,6 @@
+// Scheme check that imports from different paths are resolved to the same module
+const messages = []
+export const add = (message) => {
+  messages.push(message)
+}
+export const get = () => messages

playground/optimize-deps/dep-non-optimized/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "dep-non-optimized",
+  "private": true,
+  "version": "1.0.0",
+  "type": "module"
+}

playground/optimize-deps/index.html

@@ -87,6 +87,9 @@ <h2>Flatten Id</h2>
 <div class="clonedeep-slash"></div>
 <div class="clonedeep-dot"></div>
 
+<h2>Non Optimized Module isn't duplicated</h2>
+<div class="non-optimized-module-is-not-duplicated"></div>
+
 <script>
   function text(el, text) {
     document.querySelector(el).textContent = text
@@ -145,6 +148,18 @@ <h2>Flatten Id</h2>
   text('.url', parse('https://vitejs.dev').hostname)
 
   import './index.astro'
+
+  // All these imports should end up resolved to the same URL (same ?v= injected on them)
+  import { add as addFromDirectAbsolutePath } from '/node_modules/dep-non-optimized/index.js'
+  import { add as addFromDirectRelativePath } from './node_modules/dep-non-optimized/index.js'
+  import { get as getFromBareImport } from 'dep-non-optimized'
+
+  addFromDirectAbsolutePath('from-absolute-path')
+  addFromDirectRelativePath('from-relative-path')
+  text(
+    '.non-optimized-module-is-not-duplicated',
+    getFromBareImport().join(', ')
+  )
 </script>
 
 <script type="module">

playground/optimize-deps/package.json

@@ -24,6 +24,7 @@
     "dep-with-builtin-module-esm": "file:./dep-with-builtin-module-esm",
     "dep-with-dynamic-import": "file:./dep-with-dynamic-import",
     "dep-with-optional-peer-dep": "file:./dep-with-optional-peer-dep",
+    "dep-non-optimized": "file:./dep-non-optimized",
     "added-in-entries": "file:./added-in-entries",
     "lodash-es": "^4.17.21",
     "nested-exclude": "file:./nested-exclude",

playground/optimize-deps/vite.config.js

@@ -22,7 +22,7 @@ module.exports = {
       // will throw if optimized (should log warning instead)
       'non-optimizable-include'
     ],
-    exclude: ['nested-exclude'],
+    exclude: ['nested-exclude', 'dep-non-optimized'],
     esbuildOptions: {
       plugins: [
         {