From a201cd495f5965fde13ad549fbee06132f66fed4 Mon Sep 17 00:00:00 2001 From: patak Date: Wed, 22 Jun 2022 12:48:55 +0200 Subject: [PATCH] fix: use esbuild platform browser/node instead of neutral (#8714) Co-authored-by: sapphi-red --- packages/vite/src/node/optimizer/index.ts | 21 ++++++++++++++++--- packages/vite/src/node/plugins/define.ts | 7 ++++++- .../__tests__/optimize-deps.spec.ts | 4 ++++ .../dep-relative-to-main/entry.js | 1 + .../dep-relative-to-main/lib/main.js | 1 + .../dep-relative-to-main/package.json | 6 ++++++ playground/optimize-deps/index.html | 9 ++++++++ playground/optimize-deps/package.json | 1 + pnpm-lock.yaml | 11 ++++++++++ 9 files changed, 57 insertions(+), 4 deletions(-) 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/packages/vite/src/node/optimizer/index.ts b/packages/vite/src/node/optimizer/index.ts index 1ef86c6aad9b2f..d9b5d4897d667e 100644 --- a/packages/vite/src/node/optimizer/index.ts +++ b/packages/vite/src/node/optimizer/index.ts @@ -533,15 +533,30 @@ 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' + : 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't 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( 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 9817093816f686..55ad67574c0778 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -607,6 +607,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 @@ -633,6 +634,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 @@ -683,6 +685,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: {} @@ -8783,6 +8788,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