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.

Sign up for GitHub

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

Jump to bottom

fix(define): should not stringify vite internal env #12120

Merged
merged 4 commits into from
Feb 21, 2023
Merged

fix(define): should not stringify vite internal env #12120

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ddd892
fix(define): should not stringify vite internal env
sun0day Feb 20, 2023
b150e84
fix: generic env define type
bluwy Feb 21, 2023
60cbf86
fix: use global
bluwy Feb 21, 2023
ec97499
perf: single replace
bluwy Feb 21, 2023
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
26 changes: 19 additions & 7 deletions packages/vite/src/node/plugins/define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,24 @@ export function definePlugin(config: ResolvedConfig): Plugin {
})
}

const env = { ...config.env }
const userDefine: Record<string, string> = {}
const userDefineEnv: Record<string, string> = {}
for (const key in config.define) {
const val = config.define[key]
userDefine[key] = typeof val === 'string' ? val : JSON.stringify(val)

// make sure `import.meta.env` object has user define properties
const match = key.match(metaEnvRe)
if (match) {
env[match[1]] = val
if (isBuild) {
bluwy marked this conversation as resolved.
Show resolved Hide resolved
const match = key.match(metaEnvRe)
if (match) {
userDefineEnv[match[1]] =
// test if value is raw identifier to wrap with __vite__ so when
// stringified for `import.meta.env`, we can remove the quotes and
// retain being an identifier
typeof val === 'string' && /^[\p{L}_$]/u.test(val.trim())
sun0day marked this conversation as resolved.
Show resolved Hide resolved
? `__vite__${val}__vite__`
: val
}
}
}

Expand All @@ -49,16 +57,20 @@ export function definePlugin(config: ResolvedConfig): Plugin {
const importMetaKeys: Record<string, string> = {}
const importMetaFallbackKeys: Record<string, string> = {}
if (isBuild) {
env.SSR = !!config.build.ssr

const env: Record<string, any> = {
...config.env,
SSR: !!config.build.ssr,
}
// set here to allow override with config.define
importMetaKeys['import.meta.hot'] = `undefined`
for (const key in env) {
importMetaKeys[`import.meta.env.${key}`] = JSON.stringify(env[key])
}
Comment on lines 66 to 68
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split off env and userDefineEnv as otherwise the define envs get mixed into this for loop, which is harmless in practice, but would be nice to avoid.

Object.assign(importMetaFallbackKeys, {
'import.meta.env.': `({}).`,
'import.meta.env': JSON.stringify(env),
'import.meta.env': JSON.stringify({ ...env, ...userDefineEnv })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that import.meta.env.SSR will be replaced by user define values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might be fine for now as in dev (import analysis), it's allowed to overwrite import.meta.env.SSR too 🤔

vite/packages/vite/src/node/plugins/importAnalysis.ts

Lines 636 to 652 in 3551f75

if (hasEnv) {
// inject import.meta.env
let env = `import.meta.env = ${JSON.stringify({
...config.env,
SSR: !!ssr,
})};`
// account for user env defines
for (const key in config.define) {
if (key.startsWith(`import.meta.env.`)) {
const val = config.define[key]
env += `${key} = ${
typeof val === 'string' ? val : JSON.stringify(val)
};`
}
}
str().prepend(env)
}

this code injects the import.meta.env code for each file. Also I just realized we could probably cache this code.

.replace('"__vite__', '')
bluwy marked this conversation as resolved.
Show resolved Hide resolved
.replace('__vite__"', ''),
})
}

Expand Down
1 change: 1 addition & 0 deletions playground/legacy/__tests__/legacy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ test('import.meta.env.LEGACY', async () => {
isBuild ? 'true' : 'false',
true,
)
await untilUpdated(() => page.textContent('#env-equal'), 'true', true)
})

// https://github.com/vitejs/vite/issues/3400
Expand Down
1 change: 1 addition & 0 deletions playground/legacy/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<h1 id="app"></h1>
<div id="env"></div>
<div id="env-equal"></div>
<div id="iterators"></div>
<div id="features-after-corejs-3"></div>
<div id="async-generator"></div>
Expand Down
3 changes: 3 additions & 0 deletions playground/legacy/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ if (import.meta.env.LEGACY) {

text('#env', `is legacy: ${isLegacy}`)

const metaEnvObj = import.meta.env
text('#env-equal', import.meta.env.LEGACY === metaEnvObj.LEGACY)

// Iterators
text('#iterators', [...new Set(['hello'])].join(''))

Expand Down