Make "process.env" case insensitive in Windows#5465
Conversation
| const BLACKLIST = new Set(['mainModule', '_events']); | ||
| const BLACKLIST = new Set(['env', 'mainModule', '_events']); | ||
|
|
||
| function createProcessEnv() { |
There was a problem hiding this comment.
I think it will be helpful to point to the process.env docs somewhere here (e.g. I wasn't aware of this, TIL): https://nodejs.org/api/process.html#process_process_env
|
CI failures are completely unrelated, what the heck CircleCI? |
|
It's trying to use what it seems to be a corrupted cache layer ("Found a cache from build 13420 at |
|
I'm going to merge this. CircleCI looks corrupted, but tests passed in AppVeyor. |
| expect(require('process') === process).toBe(true); | ||
| }); | ||
|
|
||
| it('checks that process.env works as expected on Linux platforms', () => { |
There was a problem hiding this comment.
could you add a test for delete process.env.PROP as well?
|
Change looks great 🙂 Missing changelog entry 😱 |
|
@SimenB Good catch on the |
|
@mjesun It is case insensitive:
|
|
What about |
|
Yup.
|
|
Thanks! I will implement that behavior as well 🙂 |
|
And that is the story of how we started using Proxies in production… :D |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
When we fully sandboxed
process, we removed the ability of theenvobject to perform case-insensitive matches when used in Windows. The proposed solution uses aProxyinstance over a shared prototype object, and emulates the behavior by using a secondary lookup object.Another behavior that
process.envhas is that all values are always stored as strings, regardless of their value (numbers, booleans...). This behavior has also been mimicked in the new implementation.Tests were also added to ensure that both behaviors are well respected in different platforms. Fixes #5322.