-
Notifications
You must be signed in to change notification settings - Fork 1
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: support undefined process #10
Conversation
e5f8142
to
8a4a81f
Compare
it('should return Env.PRODUCTION', () => { | ||
process = undefined as any | ||
expect(getEnv()).toBe(Env.PRODUCTION) | ||
}) |
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.
This tests reproduces the issue
typeof process === 'object' && | ||
typeof process.env === 'object' | ||
? process.env | ||
: systemEnvVariables |
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.
This should keep the same behaviour for projects relying on the process.env
and passing no param to the getEnv
helper.
If no param is sent to the helper, and process.env
exists, it uses that as the envVars
, otherwise if something custom is passed as param, it will use the custom param.
All the existing tests are working without having to make any changes on the tests themselves.
).toThrow() | ||
}) | ||
}) | ||
}) |
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.
This test is not related to this PR, it was just missing and causing the coverage to be less than 100% so I added it.
@@ -269,7 +309,7 @@ describe('when getting env', () => { | |||
}) | |||
}) | |||
|
|||
describe('and host is "market.decentraland.org"', () => { | |||
describe('and host is "builder.decentraland.org"', () => { |
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.
This was just a typo in the test name
There are builds where
process
is not defined and is making the library to throw on runtime with the following error:This PR adds a test that reproduces the issue, and then applies a fix for it.