Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use esbuild platform browser/node instead of neutral #8714

Merged
merged 8 commits into from Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 18 additions & 3 deletions packages/vite/src/node/optimizer/index.ts
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 = {
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
'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
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
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
@@ -0,0 +1 @@
module.exports = require('./')
1 change: 1 addition & 0 deletions playground/optimize-deps/dep-relative-to-main/lib/main.js
@@ -0,0 +1 @@
module.exports = '[success] imported from main'
6 changes: 6 additions & 0 deletions 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"
}
9 changes: 9 additions & 0 deletions playground/optimize-deps/index.html
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
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.