Skip to content

Commit

Permalink
fix: use esbuild platform browser/node instead of neutral (#8714)
Browse files Browse the repository at this point in the history
Co-authored-by: sapphi-red <green@sapphi.red>
  • Loading branch information
patak-dev and sapphi-red authored Jun 22, 2022
1 parent 4f9097b commit a201cd4
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 4 deletions.
21 changes: 18 additions & 3 deletions packages/vite/src/node/optimizer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion packages/vite/src/node/plugins/define.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand Down Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions playground/optimize-deps/__tests__/optimize-deps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]`
Expand Down
1 change: 1 addition & 0 deletions playground/optimize-deps/dep-relative-to-main/entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./')
1 change: 1 addition & 0 deletions playground/optimize-deps/dep-relative-to-main/lib/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = '[success] imported from main'
6 changes: 6 additions & 0 deletions playground/optimize-deps/dep-relative-to-main/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "dep-relative-to-main",
"private": true,
"version": "1.0.0",
"main": "lib/main.js"
}
9 changes: 9 additions & 0 deletions playground/optimize-deps/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ <h2>Import from dependency with process.env.NODE_ENV</h2>
<h2>Import from dependency with .notjs files</h2>
<div class="not-js"></div>

<h2>
Import from dependency which uses relative path which needs to be resolved by
main field
</h2>
<div class="relative-to-main"></div>

<h2>Import from dependency with dynamic import</h2>
<div class="dep-with-dynamic-import"></div>

Expand Down Expand Up @@ -109,6 +115,9 @@ <h2>Flatten Id</h2>
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)
Expand Down
1 change: 1 addition & 0 deletions playground/optimize-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 11 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a201cd4

Please sign in to comment.