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

Setting undefined to process.env.* sets a "undefined" string value #48875

Closed
piranna opened this issue Jul 21, 2023 · 5 comments
Closed

Setting undefined to process.env.* sets a "undefined" string value #48875

piranna opened this issue Jul 21, 2023 · 5 comments

Comments

@piranna
Copy link
Contributor

piranna commented Jul 21, 2023

Version

v20.4.0

Platform

Linux executive 6.2.0-25-generic #25-Ubuntu SMP PREEMPT_DYNAMIC Fri Jun 16 17:05:07 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

process.env

What steps will reproduce the bug?

Assign undefined to any value on process.env, for example process.env.foo = undefined.

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

Since values assigned to entries in process.env are always casted to string and it's not possible to have any of them set to the undefined value, then it should be fully removed from the process.env object, allowing this way to get the entry unset.

What do you see instead?

I get the entry assigned the string undefined as value.

Additional information

The same happens with null, that gets assigned with the null string, so it's not possible to differenciate when setting it to null or the string "null". In this case I'm not sure of what should be the correct behaviour, since it's similar to what happens in SQL... Maybe setting the environment variable to an empty string?

For reference, assigning an object gets serialized as [object Object], that sort of makes sense,

@bnoordhuis
Copy link
Member

Documented behavior and not subject to change; process.env is special and kind of magical.

Use delete process.env.whatever to unset properties.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2023
@piranna
Copy link
Contributor Author

piranna commented Jul 21, 2023

I accept it being magical, and find the automatic casting to strings somewhat a smart trick, but setting undefined and being set to an undefined string, I find it more like a bug... For other values including null is fine for me, but for undefined, what are the reasons behind that behaviour?

@piranna
Copy link
Contributor Author

piranna commented Jul 21, 2023

Ok, current behaviour is properly documented at https://nodejs.org/api/process.html#processenv, but as deprecated. Can we left this issue open for when the current implicit conversion to string gets removed, so in addition of allowing only string, number, or boolean values (although I would be more strict and left just only strings), can be also accepted undefined as a way to remove the entry?

@bnoordhuis
Copy link
Member

That "may result in a thrown error" is a bit of a paper tiger, it was already weakened from "will" to "may" (ref) and I don't expect it to ever happen, it would simply break too much of the ecosystem for no good reason.

can be also accepted undefined as a way to remove the entry?

That was proposed back in 2018 and was shot down for backwards compatibility reasons: #18158

The threshold for breaking changes is really, really high. And it should be.

@piranna
Copy link
Contributor Author

piranna commented Jul 21, 2023

@bnoordhuis, I've read the links you posted, it's sad the decision that was taken, specially by maintaining a broken API for backwards compatibility instead of fixing it :-( But I suppose I would need to deal with it... Luckily it doesn't have a direct impact on me, just found that on mid development and did a proper fix on my code, so this doesn't affect me, but anyway I thoung should notify it.

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

No branches or pull requests

2 participants