From fd43915632de49aec43806cc770b87293943eff9 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Wed, 22 Jun 2022 15:00:05 +0900 Subject: [PATCH 1/7] test(optimizer): add failing test for #8704 --- .../optimize-deps/__tests__/optimize-deps.spec.ts | 4 ++++ .../optimize-deps/dep-relative-to-main/entry.js | 1 + .../optimize-deps/dep-relative-to-main/lib/main.js | 1 + .../optimize-deps/dep-relative-to-main/package.json | 6 ++++++ playground/optimize-deps/index.html | 9 +++++++++ playground/optimize-deps/package.json | 1 + pnpm-lock.yaml | 11 +++++++++++ 7 files changed, 33 insertions(+) create mode 100644 playground/optimize-deps/dep-relative-to-main/entry.js create mode 100644 playground/optimize-deps/dep-relative-to-main/lib/main.js create mode 100644 playground/optimize-deps/dep-relative-to-main/package.json diff --git a/playground/optimize-deps/__tests__/optimize-deps.spec.ts b/playground/optimize-deps/__tests__/optimize-deps.spec.ts index cc712fbbdc4a09..df1713b3fc76f8 100644 --- a/playground/optimize-deps/__tests__/optimize-deps.spec.ts +++ b/playground/optimize-deps/__tests__/optimize-deps.spec.ts @@ -77,6 +77,10 @@ test('import from dep with .notjs files', async () => { expect(await page.textContent('.not-js')).toMatch(`[success]`) }) +test('Import from dependency which uses relative path which needs to be resolved by main field', async () => { + expect(await page.textContent('.relative-to-main')).toMatch(`[success]`) +}) + test('dep with dynamic import', async () => { expect(await page.textContent('.dep-with-dynamic-import')).toMatch( `[success]` diff --git a/playground/optimize-deps/dep-relative-to-main/entry.js b/playground/optimize-deps/dep-relative-to-main/entry.js new file mode 100644 index 00000000000000..d70a1adf8fee78 --- /dev/null +++ b/playground/optimize-deps/dep-relative-to-main/entry.js @@ -0,0 +1 @@ +module.exports = require('./') diff --git a/playground/optimize-deps/dep-relative-to-main/lib/main.js b/playground/optimize-deps/dep-relative-to-main/lib/main.js new file mode 100644 index 00000000000000..d27286071c483f --- /dev/null +++ b/playground/optimize-deps/dep-relative-to-main/lib/main.js @@ -0,0 +1 @@ +module.exports = '[success] imported from main' diff --git a/playground/optimize-deps/dep-relative-to-main/package.json b/playground/optimize-deps/dep-relative-to-main/package.json new file mode 100644 index 00000000000000..bc11dc9397ed36 --- /dev/null +++ b/playground/optimize-deps/dep-relative-to-main/package.json @@ -0,0 +1,6 @@ +{ + "name": "dep-relative-to-main", + "private": true, + "version": "1.0.0", + "main": "lib/main.js" +} diff --git a/playground/optimize-deps/index.html b/playground/optimize-deps/index.html index 00cf4c48e6e9e6..7b0c43e82fdcbc 100644 --- a/playground/optimize-deps/index.html +++ b/playground/optimize-deps/index.html @@ -50,6 +50,12 @@

Import from dependency with process.env.NODE_ENV

Import from dependency with .notjs files

+

+ Import from dependency which uses relative path which needs to be resolved by + main field +

+
+

Import from dependency with dynamic import

@@ -109,6 +115,9 @@

Flatten Id

import { notjsValue } from 'dep-not-js' text('.not-js', notjsValue) + import foo from 'dep-relative-to-main/entry' + text('.relative-to-main', foo) + import { lazyFoo } from 'dep-with-dynamic-import' lazyFoo().then((foo) => { text('.dep-with-dynamic-import', foo) diff --git a/playground/optimize-deps/package.json b/playground/optimize-deps/package.json index 9470641620b4a3..7fbb2b6577d69f 100644 --- a/playground/optimize-deps/package.json +++ b/playground/optimize-deps/package.json @@ -19,6 +19,7 @@ "dep-linked-include": "link:./dep-linked-include", "dep-node-env": "file:./dep-node-env", "dep-not-js": "file:./dep-not-js", + "dep-relative-to-main": "file:./dep-relative-to-main", "dep-with-builtin-module-cjs": "file:./dep-with-builtin-module-cjs", "dep-with-builtin-module-esm": "file:./dep-with-builtin-module-esm", "dep-with-dynamic-import": "file:./dep-with-dynamic-import", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5f3a9c1313adfb..74f1fcd8046608 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -582,6 +582,7 @@ importers: dep-linked-include: link:./dep-linked-include dep-node-env: file:./dep-node-env dep-not-js: file:./dep-not-js + dep-relative-to-main: file:./dep-relative-to-main dep-with-builtin-module-cjs: file:./dep-with-builtin-module-cjs dep-with-builtin-module-esm: file:./dep-with-builtin-module-esm dep-with-dynamic-import: file:./dep-with-dynamic-import @@ -608,6 +609,7 @@ importers: dep-linked-include: link:dep-linked-include dep-node-env: file:playground/optimize-deps/dep-node-env dep-not-js: file:playground/optimize-deps/dep-not-js + dep-relative-to-main: file:playground/optimize-deps/dep-relative-to-main dep-with-builtin-module-cjs: file:playground/optimize-deps/dep-with-builtin-module-cjs dep-with-builtin-module-esm: file:playground/optimize-deps/dep-with-builtin-module-esm dep-with-dynamic-import: file:playground/optimize-deps/dep-with-dynamic-import @@ -658,6 +660,9 @@ importers: playground/optimize-deps/dep-not-js: specifiers: {} + playground/optimize-deps/dep-relative-to-main: + specifiers: {} + playground/optimize-deps/dep-with-builtin-module-cjs: specifiers: {} @@ -8742,6 +8747,12 @@ packages: version: 1.0.0 dev: false + file:playground/optimize-deps/dep-relative-to-main: + resolution: {directory: playground/optimize-deps/dep-relative-to-main, type: directory} + name: dep-relative-to-main + version: 1.0.0 + dev: false + file:playground/optimize-deps/dep-with-builtin-module-cjs: resolution: {directory: playground/optimize-deps/dep-with-builtin-module-cjs, type: directory} name: dep-with-builtin-module-cjs From 79c10e67b3e47b5db2ddbba36846095d758ce48f Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Wed, 22 Jun 2022 15:05:26 +0900 Subject: [PATCH 2/7] fix(optimizer): resolve relative path by Vite resolver --- .../src/node/optimizer/esbuildDepPlugin.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 09866eb300c2a3..8a93f4367bc7b9 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -149,35 +149,35 @@ export function esbuildDepPlugin( } } - build.onResolve( - { filter: /^[\w@][^:]/ }, - async ({ path: id, importer, kind }) => { - if (moduleListContains(config.optimizeDeps?.exclude, id)) { - return { - path: id, - external: true - } + build.onResolve({ filter: /./ }, async ({ path: id, importer, kind }) => { + if ( + /^[\w@][^:]/.test(id) && + moduleListContains(config.optimizeDeps?.exclude, id) + ) { + return { + path: id, + external: true } + } - // ensure esbuild uses our resolved entries - let entry: { path: string; namespace: string } | undefined - // if this is an entry, return entry namespace resolve result - if (!importer) { - if ((entry = resolveEntry(id))) return entry - // check if this is aliased to an entry - also return entry namespace - const aliased = await _resolve(id, undefined, true) - if (aliased && (entry = resolveEntry(aliased))) { - return entry - } + // ensure esbuild uses our resolved entries + let entry: { path: string; namespace: string } | undefined + // if this is an entry, return entry namespace resolve result + if (!importer) { + if ((entry = resolveEntry(id))) return entry + // check if this is aliased to an entry - also return entry namespace + const aliased = await _resolve(id, undefined, true) + if (aliased && (entry = resolveEntry(aliased))) { + return entry } + } - // use vite's own resolver - const resolved = await resolve(id, importer, kind) - if (resolved) { - return resolveResult(id, resolved) - } + // use vite's own resolver + const resolved = await resolve(id, importer, kind) + if (resolved) { + return resolveResult(id, resolved) } - ) + }) // For entry files, we'll read it ourselves and construct a proxy module // to retain the entry's raw id instead of file path so that esbuild From 3ce8b932ae00010c977d058c7d422898ef1f36e3 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Wed, 22 Jun 2022 11:20:41 +0200 Subject: [PATCH 3/7] chore: revert esbuildDepPlugin changes --- .../src/node/optimizer/esbuildDepPlugin.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 8a93f4367bc7b9..09866eb300c2a3 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -149,35 +149,35 @@ export function esbuildDepPlugin( } } - build.onResolve({ filter: /./ }, async ({ path: id, importer, kind }) => { - if ( - /^[\w@][^:]/.test(id) && - moduleListContains(config.optimizeDeps?.exclude, id) - ) { - return { - path: id, - external: true + build.onResolve( + { filter: /^[\w@][^:]/ }, + async ({ path: id, importer, kind }) => { + if (moduleListContains(config.optimizeDeps?.exclude, id)) { + return { + path: id, + external: true + } } - } - // ensure esbuild uses our resolved entries - let entry: { path: string; namespace: string } | undefined - // if this is an entry, return entry namespace resolve result - if (!importer) { - if ((entry = resolveEntry(id))) return entry - // check if this is aliased to an entry - also return entry namespace - const aliased = await _resolve(id, undefined, true) - if (aliased && (entry = resolveEntry(aliased))) { - return entry + // ensure esbuild uses our resolved entries + let entry: { path: string; namespace: string } | undefined + // if this is an entry, return entry namespace resolve result + if (!importer) { + if ((entry = resolveEntry(id))) return entry + // check if this is aliased to an entry - also return entry namespace + const aliased = await _resolve(id, undefined, true) + if (aliased && (entry = resolveEntry(aliased))) { + return entry + } } - } - // use vite's own resolver - const resolved = await resolve(id, importer, kind) - if (resolved) { - return resolveResult(id, resolved) + // use vite's own resolver + const resolved = await resolve(id, importer, kind) + if (resolved) { + return resolveResult(id, resolved) + } } - }) + ) // For entry files, we'll read it ourselves and construct a proxy module // to retain the entry's raw id instead of file path so that esbuild From cd1afdf0a0c6d49a1943fd3852fd83b881d5bac3 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Wed, 22 Jun 2022 11:56:22 +0200 Subject: [PATCH 4/7] fix: use esbuild platform browser/node instead of neutral --- packages/vite/src/node/optimizer/index.ts | 17 ++++++++++++++--- packages/vite/src/node/plugins/define.ts | 7 ++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index 1ef86c6aad9b2f..f7f61192896cda 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -533,15 +533,26 @@ export async function runOptimizeDeps( flatIdToExports[flatId] = exportsData } + const define = { + 'process.env.NODE_ENV': isBuild + ? '__vite_process_env_NODE_ENV' + : JSON.stringify(process.env.NODE_ENV || config.mode) + } + const start = performance.now() const result = await build({ absWorkingDir: process.cwd(), entryPoints: Object.keys(flatIdDeps), bundle: true, - // Ensure resolution is handled by esbuildDepPlugin and - // avoid replacing `process.env.NODE_ENV` for 'browser' - platform: 'neutral', + // We can't use platform 'neutral', as esbuild has custom handling + // when the platform is 'node' or 'browser' that can be emulated + // by using mainFields and conditions + platform: + config.build.ssr && config.ssr?.target !== 'webworker' + ? 'node' + : 'browser', + define, format: 'esm', target: config.build.target || undefined, external: config.optimizeDeps?.exclude, diff --git a/packages/vite/src/node/plugins/define.ts b/packages/vite/src/node/plugins/define.ts index 6edb1bd8858a11..9859676e693a23 100644 --- a/packages/vite/src/node/plugins/define.ts +++ b/packages/vite/src/node/plugins/define.ts @@ -25,7 +25,8 @@ export function definePlugin(config: ResolvedConfig): Plugin { Object.assign(processNodeEnv, { 'process.env.NODE_ENV': JSON.stringify(nodeEnv), 'global.process.env.NODE_ENV': JSON.stringify(nodeEnv), - 'globalThis.process.env.NODE_ENV': JSON.stringify(nodeEnv) + 'globalThis.process.env.NODE_ENV': JSON.stringify(nodeEnv), + __vite_process_env_NODE_ENV: JSON.stringify(nodeEnv) }) } @@ -65,6 +66,10 @@ export function definePlugin(config: ResolvedConfig): Plugin { ...(replaceProcessEnv ? processEnv : {}) } + if (isBuild && !replaceProcessEnv) { + replacements['__vite_process_env_NODE_ENV'] = 'process.env.NODE_ENV' + } + const replacementsKeys = Object.keys(replacements) const pattern = replacementsKeys.length ? new RegExp( From 656eb02075687bd23409f8ea1f3156b4c82e35e7 Mon Sep 17 00:00:00 2001 From: patak Date: Wed, 22 Jun 2022 12:20:51 +0200 Subject: [PATCH 5/7] chore: update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/optimizer/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index f7f61192896cda..8c01869ce04fd5 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -546,7 +546,7 @@ export async function runOptimizeDeps( entryPoints: Object.keys(flatIdDeps), bundle: true, // We can't use platform 'neutral', as esbuild has custom handling - // when the platform is 'node' or 'browser' that can be emulated + // when the platform is 'node' or 'browser' that can't be emulated // by using mainFields and conditions platform: config.build.ssr && config.ssr?.target !== 'webworker' From 06ac13c4bd1aab622d250b717864e026e63cb3b1 Mon Sep 17 00:00:00 2001 From: patak Date: Wed, 22 Jun 2022 12:31:11 +0200 Subject: [PATCH 6/7] chore: explain why we need __vite_process_env_NODE_ENV --- packages/vite/src/node/optimizer/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index 8c01869ce04fd5..743bd9dabdc7a0 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -533,6 +533,10 @@ export async function runOptimizeDeps( flatIdToExports[flatId] = exportsData } + // esbuild automatically replaces process.env.NODE_ENV for platform 'browser' + // In lib mode, we need to keep process.env.NODE_ENV untouched, so to at build + // time we replace it by __vite_process_env_NODE_ENV. This placeholder will be + // later replaced by the define plugin const define = { 'process.env.NODE_ENV': isBuild ? '__vite_process_env_NODE_ENV' From 32c00bb7b552ae342303dcb937bc6c04efa00357 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Wed, 22 Jun 2022 12:34:11 +0200 Subject: [PATCH 7/7] chore: format --- packages/vite/src/node/optimizer/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index 743bd9dabdc7a0..d9b5d4897d667e 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -535,7 +535,7 @@ export async function runOptimizeDeps( // esbuild automatically replaces process.env.NODE_ENV for platform 'browser' // In lib mode, we need to keep process.env.NODE_ENV untouched, so to at build - // time we replace it by __vite_process_env_NODE_ENV. This placeholder will be + // time we replace it by __vite_process_env_NODE_ENV. This placeholder will be // later replaced by the define plugin const define = { 'process.env.NODE_ENV': isBuild