From 480647b5c5c4045c21da86f4b71f72babb633727 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Thu, 25 Aug 2022 23:43:48 +0200 Subject: [PATCH 1/3] fix: ensure version query for direct node_modules imports --- packages/vite/src/node/plugins/preAlias.ts | 9 +++- packages/vite/src/node/plugins/resolve.ts | 48 +++++++++++----------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/packages/vite/src/node/plugins/preAlias.ts b/packages/vite/src/node/plugins/preAlias.ts index 9eb4cfeeffa85d..171832dbf78e3b 100644 --- a/packages/vite/src/node/plugins/preAlias.ts +++ b/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 && diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index f33105d265a27a..5c02cb078b209a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -155,6 +155,23 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { return optimizedPath } + const ensureVersionQuery = (resolved: string): string => { + if (!options.isBuild && depsOptimizer) { + // 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).includes('/node_modules/') + 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) @@ -162,7 +179,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { 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 +188,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 +218,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 +226,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 +248,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 +258,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 +403,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( From f7785ab04e943f7504703c0f3cf6b05548e440a5 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Fri, 26 Aug 2022 12:27:22 +0200 Subject: [PATCH 2/3] fix: don't add version query to vite client and env --- packages/vite/src/node/plugins/resolve.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 5c02cb078b209a..2c3a2548c2769f 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/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,6 +44,9 @@ 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' @@ -156,7 +161,14 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { } const ensureVersionQuery = (resolved: string): string => { - if (!options.isBuild && depsOptimizer) { + 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 From 0dd0e0c2e179ca1829d61232897525b3d72a6547 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Fri, 26 Aug 2022 18:17:09 +0200 Subject: [PATCH 3/3] fix: relative path + add tests --- packages/vite/src/node/plugins/resolve.ts | 4 +++- .../optimize-deps/__tests__/optimize-deps.spec.ts | 6 ++++++ .../optimize-deps/dep-non-optimized/index.js | 6 ++++++ .../optimize-deps/dep-non-optimized/package.json | 6 ++++++ playground/optimize-deps/index.html | 15 +++++++++++++++ playground/optimize-deps/package.json | 1 + playground/optimize-deps/vite.config.js | 2 +- pnpm-lock.yaml | 11 +++++++++++ 8 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 playground/optimize-deps/dep-non-optimized/index.js create mode 100644 playground/optimize-deps/dep-non-optimized/package.json diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 2c3a2548c2769f..371609fdf23c83 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -53,6 +53,8 @@ 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 @@ -173,7 +175,7 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin { // 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).includes('/node_modules/') + 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)) { diff --git a/playground/optimize-deps/__tests__/optimize-deps.spec.ts b/playground/optimize-deps/__tests__/optimize-deps.spec.ts index 19e9014d7234cf..997d3bb9da1a26 100644 --- a/playground/optimize-deps/__tests__/optimize-deps.spec.ts +++ b/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([ diff --git a/playground/optimize-deps/dep-non-optimized/index.js b/playground/optimize-deps/dep-non-optimized/index.js new file mode 100644 index 00000000000000..51b048a657d5c9 --- /dev/null +++ b/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 diff --git a/playground/optimize-deps/dep-non-optimized/package.json b/playground/optimize-deps/dep-non-optimized/package.json new file mode 100644 index 00000000000000..fae17a384ce949 --- /dev/null +++ b/playground/optimize-deps/dep-non-optimized/package.json @@ -0,0 +1,6 @@ +{ + "name": "dep-non-optimized", + "private": true, + "version": "1.0.0", + "type": "module" +} diff --git a/playground/optimize-deps/index.html b/playground/optimize-deps/index.html index fe507e9ba568f4..f3e94ca6624dc1 100644 --- a/playground/optimize-deps/index.html +++ b/playground/optimize-deps/index.html @@ -87,6 +87,9 @@

Flatten Id

+

Non Optimized Module isn't duplicated

+
+