From 2017a330f5576dfc9db1538e0b899a1776cd100a Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 10 Oct 2024 16:48:36 +0900 Subject: [PATCH] fix(resolve): fix resolve cache to consider `conditions` and more (#18302) --- .../src/node/__tests__/environment.spec.ts | 209 ++++++++++++++++++ .../test-dep-conditions-app/entry.css | 1 + .../fixtures/test-dep-conditions-app/entry.js | 2 + .../test-dep-conditions/index.browser.js | 1 + .../fixtures/test-dep-conditions/index.css | 3 + .../test-dep-conditions/index.custom1.js | 1 + .../test-dep-conditions/index.custom2.js | 1 + .../test-dep-conditions/index.custom3.js | 1 + .../test-dep-conditions/index.default.js | 1 + .../test-dep-conditions/index.worker.js | 1 + .../fixtures/test-dep-conditions/package.json | 16 ++ packages/vite/src/node/__tests__/package.json | 3 +- packages/vite/src/node/packages.ts | 46 ++-- packages/vite/src/node/plugins/resolve.ts | 19 +- pnpm-lock.yaml | 10 + 15 files changed, 284 insertions(+), 31 deletions(-) create mode 100644 packages/vite/src/node/__tests__/environment.spec.ts create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.css create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.browser.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.css create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom1.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom2.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom3.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.default.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.worker.js create mode 100644 packages/vite/src/node/__tests__/fixtures/test-dep-conditions/package.json diff --git a/packages/vite/src/node/__tests__/environment.spec.ts b/packages/vite/src/node/__tests__/environment.spec.ts new file mode 100644 index 00000000000000..991eae3e76961a --- /dev/null +++ b/packages/vite/src/node/__tests__/environment.spec.ts @@ -0,0 +1,209 @@ +import path from 'node:path' +import { describe, expect, onTestFinished, test } from 'vitest' +import type { RollupOutput } from 'rollup' +import { createServer } from '../server' +import { defineConfig } from '../config' +import { createBuilder } from '../build' +import { createServerModuleRunner } from '../ssr/runtime/serverModuleRunner' + +describe('custom environment conditions', () => { + function getConfig() { + return defineConfig({ + root: import.meta.dirname, + logLevel: 'error', + server: { + middlewareMode: true, + }, + environments: { + // no web / default + ssr: { + resolve: { + noExternal: true, + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/ssr', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + // web / worker + worker: { + webCompatible: true, + resolve: { + noExternal: true, + conditions: ['worker'], + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/worker', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + // web / custom1 + custom1: { + webCompatible: true, + resolve: { + noExternal: true, + conditions: ['custom1'], + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/custom1', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + // no web / custom2 + custom2: { + webCompatible: false, + resolve: { + noExternal: true, + conditions: ['custom2'], + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/custom2', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + // no web / custom3 + custom3: { + webCompatible: false, + resolve: { + noExternal: true, + conditions: ['custom3'], + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/custom3', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + // same as custom3 + custom3_2: { + webCompatible: false, + resolve: { + noExternal: true, + conditions: ['custom3'], + }, + build: { + outDir: path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist/custom3_2', + ), + rollupOptions: { + input: { index: '@vitejs/test-dep-conditions' }, + }, + }, + }, + }, + }) + } + + test('dev', async () => { + const server = await createServer(getConfig()) + onTestFinished(() => server.close()) + + const results: Record = {} + for (const key of [ + 'ssr', + 'worker', + 'custom1', + 'custom2', + 'custom3', + 'custom3_2', + ]) { + const runner = createServerModuleRunner(server.environments[key], { + hmr: { + logger: false, + }, + sourcemapInterceptor: false, + }) + const mod = await runner.import('@vitejs/test-dep-conditions') + results[key] = mod.default + } + expect(results).toMatchInlineSnapshot(` + { + "custom1": "index.custom1.js", + "custom2": "index.custom2.js", + "custom3": "index.custom3.js", + "custom3_2": "index.custom3.js", + "ssr": "index.default.js", + "worker": "index.worker.js", + } + `) + }) + + test('css', async () => { + const server = await createServer(getConfig()) + onTestFinished(() => server.close()) + + const modJs = await server.ssrLoadModule( + '/fixtures/test-dep-conditions-app/entry.js', + ) + const modCss = await server.ssrLoadModule( + '/fixtures/test-dep-conditions-app/entry.css?inline', + ) + expect([modCss.default.replace(/\s+/g, ' '), modJs.default]) + .toMatchInlineSnapshot(` + [ + ".test-css { color: orange; } ", + "index.default.js", + ] + `) + }) + + test('build', async () => { + const builder = await createBuilder(getConfig()) + const results: Record = {} + for (const key of [ + 'ssr', + 'worker', + 'custom1', + 'custom2', + 'custom3', + 'custom3_2', + ]) { + const output = await builder.build(builder.environments[key]) + const chunk = (output as RollupOutput).output[0] + const mod = await import( + path.join( + import.meta.dirname, + 'fixtures/test-dep-conditions/dist', + key, + chunk.fileName, + ) + ) + results[key] = mod.default + } + expect(results).toMatchInlineSnapshot(` + { + "custom1": "index.custom1.js", + "custom2": "index.custom2.js", + "custom3": "index.custom3.js", + "custom3_2": "index.custom3.js", + "ssr": "index.default.js", + "worker": "index.worker.js", + } + `) + }) +}) diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.css b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.css new file mode 100644 index 00000000000000..879201e33190f0 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.css @@ -0,0 +1 @@ +@import '@vitejs/test-dep-conditions'; diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.js new file mode 100644 index 00000000000000..3598e0cada91c0 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions-app/entry.js @@ -0,0 +1,2 @@ +import dep from '@vitejs/test-dep-conditions' +export default dep diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.browser.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.browser.js new file mode 100644 index 00000000000000..ad3606555f351e --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.browser.js @@ -0,0 +1 @@ +export default 'index.browser.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.css b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.css new file mode 100644 index 00000000000000..4bd7def9d93f25 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.css @@ -0,0 +1,3 @@ +.test-css { + color: orange; +} diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom1.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom1.js new file mode 100644 index 00000000000000..2334670f241cc2 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom1.js @@ -0,0 +1 @@ +export default 'index.custom1.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom2.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom2.js new file mode 100644 index 00000000000000..1125676cc5f4a7 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom2.js @@ -0,0 +1 @@ +export default 'index.custom2.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom3.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom3.js new file mode 100644 index 00000000000000..d12538a6469888 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.custom3.js @@ -0,0 +1 @@ +export default 'index.custom3.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.default.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.default.js new file mode 100644 index 00000000000000..1def227e300fdb --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.default.js @@ -0,0 +1 @@ +export default 'index.default.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.worker.js b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.worker.js new file mode 100644 index 00000000000000..8ee1b837255065 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/index.worker.js @@ -0,0 +1 @@ +export default 'index.worker.js' diff --git a/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/package.json b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/package.json new file mode 100644 index 00000000000000..cfed8db16c6bd6 --- /dev/null +++ b/packages/vite/src/node/__tests__/fixtures/test-dep-conditions/package.json @@ -0,0 +1,16 @@ +{ + "name": "@vitejs/test-dep-conditions", + "private": true, + "type": "module", + "exports": { + ".": { + "style": "./index.css", + "custom1": "./index.custom1.js", + "custom2": "./index.custom2.js", + "custom3": "./index.custom3.js", + "worker": "./index.worker.js", + "browser": "./index.browser.js", + "default": "./index.default.js" + } + } +} diff --git a/packages/vite/src/node/__tests__/package.json b/packages/vite/src/node/__tests__/package.json index a5f6e053817561..90a5d7c132c1ee 100644 --- a/packages/vite/src/node/__tests__/package.json +++ b/packages/vite/src/node/__tests__/package.json @@ -3,6 +3,7 @@ "private": true, "version": "0.0.0", "dependencies": { - "@vitejs/cjs-ssr-dep": "link:./fixtures/cjs-ssr-dep" + "@vitejs/cjs-ssr-dep": "link:./fixtures/cjs-ssr-dep", + "@vitejs/test-dep-conditions": "file:./fixtures/test-dep-conditions" } } diff --git a/packages/vite/src/node/packages.ts b/packages/vite/src/node/packages.ts index dc25c004f9214f..175505d49e5373 100644 --- a/packages/vite/src/node/packages.ts +++ b/packages/vite/src/node/packages.ts @@ -9,6 +9,7 @@ import { tryStatSync, } from './utils' import type { Plugin } from './plugin' +import type { InternalResolveOptions } from './plugins/resolve' let pnp: typeof import('pnpapi') | undefined if (process.versions.pnp) { @@ -23,10 +24,15 @@ export type PackageCache = Map export interface PackageData { dir: string hasSideEffects: (id: string) => boolean | 'no-treeshake' | null - webResolvedImports: Record - nodeResolvedImports: Record - setResolvedCache: (key: string, entry: string, targetWeb: boolean) => void - getResolvedCache: (key: string, targetWeb: boolean) => string | undefined + setResolvedCache: ( + key: string, + entry: string, + options: InternalResolveOptions, + ) => void + getResolvedCache: ( + key: string, + options: InternalResolveOptions, + ) => string | undefined data: { [field: string]: any name: string @@ -201,31 +207,35 @@ export function loadPackageData(pkgPath: string): PackageData { hasSideEffects = () => null } + const resolvedCache: Record = {} const pkg: PackageData = { dir: pkgDir, data, hasSideEffects, - webResolvedImports: {}, - nodeResolvedImports: {}, - setResolvedCache(key: string, entry: string, targetWeb: boolean) { - if (targetWeb) { - pkg.webResolvedImports[key] = entry - } else { - pkg.nodeResolvedImports[key] = entry - } + setResolvedCache(key, entry, options) { + resolvedCache[getResolveCacheKey(key, options)] = entry }, - getResolvedCache(key: string, targetWeb: boolean) { - if (targetWeb) { - return pkg.webResolvedImports[key] - } else { - return pkg.nodeResolvedImports[key] - } + getResolvedCache(key, options) { + return resolvedCache[getResolveCacheKey(key, options)] }, } return pkg } +function getResolveCacheKey(key: string, options: InternalResolveOptions) { + // cache key needs to include options which affect + // `resolvePackageEntry` or `resolveDeepImport` + return [ + key, + options.webCompatible ? '1' : '0', + options.isRequire ? '1' : '0', + options.conditions.join('_'), + options.extensions.join('_'), + options.mainFields.join('_'), + ].join('|') +} + export function watchPackageDataPlugin(packageCache: PackageCache): Plugin { // a list of files to watch before the plugin is ready const watchQueue = new Set() diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index b206a2fa1d6e7c..b930324db960fe 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -1022,7 +1022,7 @@ export function resolvePackageEntry( ): string | undefined { const { file: idWithoutPostfix, postfix } = splitFileAndPostfix(id) - const cached = getResolvedCache('.', !!options.webCompatible) + const cached = getResolvedCache('.', options) if (cached) { return cached + postfix } @@ -1094,7 +1094,7 @@ export function resolvePackageEntry( resolvedEntryPoint, )}${postfix !== '' ? ` (postfix: ${postfix})` : ''}`, ) - setResolvedCache('.', resolvedEntryPoint, !!options.webCompatible) + setResolvedCache('.', resolvedEntryPoint, options) return resolvedEntryPoint + postfix } } @@ -1151,16 +1151,10 @@ function resolveExportsOrImports( function resolveDeepImport( id: string, - { - webResolvedImports, - setResolvedCache, - getResolvedCache, - dir, - data, - }: PackageData, + { setResolvedCache, getResolvedCache, dir, data }: PackageData, options: InternalResolveOptions, ): string | undefined { - const cache = getResolvedCache(id, !!options.webCompatible) + const cache = getResolvedCache(id, options) if (cache) { return cache } @@ -1200,7 +1194,8 @@ function resolveDeepImport( if (mapped) { relativeId = mapped + postfix } else if (mapped === false) { - return (webResolvedImports[id] = browserExternalId) + setResolvedCache(id, browserExternalId, options) + return browserExternalId } } @@ -1214,7 +1209,7 @@ function resolveDeepImport( debug?.( `[node/deep-import] ${colors.cyan(id)} -> ${colors.dim(resolved)}`, ) - setResolvedCache(id, resolved, !!options.webCompatible) + setResolvedCache(id, resolved, options) return resolved } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6b3c6cf220a5e3..76a0dea2cfcced 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -427,9 +427,14 @@ importers: '@vitejs/cjs-ssr-dep': specifier: link:./fixtures/cjs-ssr-dep version: link:fixtures/cjs-ssr-dep + '@vitejs/test-dep-conditions': + specifier: file:./fixtures/test-dep-conditions + version: file:packages/vite/src/node/__tests__/fixtures/test-dep-conditions packages/vite/src/node/__tests__/fixtures/cjs-ssr-dep: {} + packages/vite/src/node/__tests__/fixtures/test-dep-conditions: {} + packages/vite/src/node/__tests__/packages/module: {} packages/vite/src/node/__tests__/packages/name: {} @@ -3602,6 +3607,9 @@ packages: '@vitejs/test-dep-cjs-with-assets@file:playground/optimize-deps/dep-cjs-with-assets': resolution: {directory: playground/optimize-deps/dep-cjs-with-assets, type: directory} + '@vitejs/test-dep-conditions@file:packages/vite/src/node/__tests__/fixtures/test-dep-conditions': + resolution: {directory: packages/vite/src/node/__tests__/fixtures/test-dep-conditions, type: directory} + '@vitejs/test-dep-css-require@file:playground/optimize-deps/dep-css-require': resolution: {directory: playground/optimize-deps/dep-css-require, type: directory} @@ -9215,6 +9223,8 @@ snapshots: '@vitejs/test-dep-cjs-with-assets@file:playground/optimize-deps/dep-cjs-with-assets': {} + '@vitejs/test-dep-conditions@file:packages/vite/src/node/__tests__/fixtures/test-dep-conditions': {} + '@vitejs/test-dep-css-require@file:playground/optimize-deps/dep-css-require': {} '@vitejs/test-dep-esbuild-plugin-transform@file:playground/optimize-deps/dep-esbuild-plugin-transform': {}