-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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(define): should not stringify vite internal env #12120
Conversation
631d7f5
to
55221ad
Compare
55221ad
to
0ddd892
Compare
Ah I didn't anticipate this bug, if you have {
SOMETHING: rawIdentifer
} instead of a string. I think we need to do this generically for all |
for (const key in env) { | ||
importMetaKeys[`import.meta.env.${key}`] = JSON.stringify(env[key]) | ||
} |
There was a problem hiding this comment.
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.
// 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]) | ||
} | ||
Object.assign(importMetaFallbackKeys, { | ||
'import.meta.env.': `({}).`, | ||
'import.meta.env': JSON.stringify(env), | ||
'import.meta.env': JSON.stringify({ ...env, ...userDefineEnv }) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
LGTM |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
Co-authored-by: bluwy <bjornlu.dev@gmail.com>
Description
Some Vite internal env values(e.g plugin-legacy
__VITE_IS_LEGACY__
) will be stringified, which may cause the value type to be incorrect inimport.meta.env
objectAdditional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).