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

Make sure process.env.NODE_ENV is stringified #6471

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Make sure process.env.NODE_ENV is stringified #6471

merged 1 commit into from
Jul 25, 2024

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Jul 25, 2024

We had some rollup config to replace process.env.NODE_ENV with their actual values, but incorrectly set to do it as a direct replacement.

That meant that if NODE_ENV=production, it would replace process.env.NODE_ENV by the literal value production, which is then treated as a variable, causing a ReferenceError: production is not defined error.

This PR fixes that by JSON-encoding the value of process.env.NODE_ENV before replacing it in code, as seen in the docs https://www.npmjs.com/package/@rollup/plugin-replace

@@ -38,7 +38,7 @@ function bundleConfig(name, entryFile) {
replace({
preventAssignment: true,
values: {
'process.env.NODE_ENV': process.env.NODE_ENV,
'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures it is still undefined if not provided, but it gets quoted as an inline string if it is.

@acelaya acelaya merged commit 2531350 into main Jul 25, 2024
9 checks passed
@acelaya acelaya deleted the fix-node-env branch July 25, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant