-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,6 +168,29 @@ describe('when getting default env', () => { | |
}) | ||
}) | ||
|
||
describe('and both DCL_DEFAULT_ENV and VITE_DCL_DEFAULT_ENV are defined', () => { | ||
describe('and both have the same value', () => { | ||
it('should return that value as default env', () => { | ||
expect( | ||
getDefaultEnv({ | ||
DCL_DEFAULT_ENV: 'dev', | ||
VITE_DCL_DEFAULT_ENV: 'dev', | ||
}) | ||
).toBe(Env.DEVELOPMENT) | ||
}) | ||
}) | ||
describe('and they have different values', () => { | ||
it('should throw', () => { | ||
expect(() => | ||
getDefaultEnv({ | ||
DCL_DEFAULT_ENV: 'dev', | ||
VITE_DCL_DEFAULT_ENV: 'prod', | ||
}) | ||
).toThrow() | ||
}) | ||
}) | ||
}) | ||
|
||
describe('and both REACT_APP_DCL_DEFAULT_ENV and GATSBY_DCL_DEFAULT_ENV are defined', () => { | ||
describe('and both have the same value', () => { | ||
it('should return that value as default env', () => { | ||
|
@@ -194,15 +217,17 @@ describe('when getting default env', () => { | |
|
||
describe('when getting env', () => { | ||
let windowSpy: jest.SpyInstance<Window & typeof globalThis, []> | ||
const { env } = process | ||
const prevProcess = process | ||
const { env: prevEnv } = process | ||
|
||
beforeEach(() => { | ||
windowSpy = jest.spyOn(window, 'window', 'get') | ||
}) | ||
|
||
afterEach(() => { | ||
windowSpy.mockRestore() | ||
process.env = env | ||
process = prevProcess | ||
process.env = prevEnv | ||
}) | ||
|
||
function mockLocation(mock: Partial<Location>) { | ||
|
@@ -215,7 +240,7 @@ describe('when getting env', () => { | |
) | ||
} | ||
|
||
function mockProcess(mock: Record<string, string | undefined>) { | ||
function mockProcessEnv(mock: Record<string, string | undefined>) { | ||
const prev = { ...process.env } | ||
process.env = { ...prev, ...mock } | ||
} | ||
|
@@ -235,7 +260,7 @@ describe('when getting env', () => { | |
|
||
describe('and the system env variable is "dev"', () => { | ||
it('should return Env.DEVELOPMENT', () => { | ||
mockProcess({ DCL_DEFAULT_ENV: 'dev' }) | ||
mockProcessEnv({ DCL_DEFAULT_ENV: 'dev' }) | ||
expect(getEnv()).toBe(Env.DEVELOPMENT) | ||
}) | ||
}) | ||
|
@@ -249,10 +274,25 @@ describe('when getting env', () => { | |
describe('and search param is "stg" and the system env variable is "dev"', () => { | ||
it('should return Env.STAGING', () => { | ||
mockLocation({ search: '?env=stg' }) | ||
mockProcess({ DCL_DEFAULT_ENV: 'dev' }) | ||
mockProcessEnv({ DCL_DEFAULT_ENV: 'dev' }) | ||
expect(getEnv()).toBe(Env.STAGING) | ||
}) | ||
}) | ||
|
||
describe('and process is not defined', () => { | ||
describe('and no system env variable is defined', () => { | ||
it('should return Env.PRODUCTION', () => { | ||
process = undefined as any | ||
expect(getEnv()).toBe(Env.PRODUCTION) | ||
}) | ||
}) | ||
describe('and a system env variable is defined', () => { | ||
it('should return the system env variable as Env', () => { | ||
process = undefined as any | ||
expect(getEnv({ DCL_DEFAULT_ENV: 'dev' })).toBe(Env.DEVELOPMENT) | ||
}) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This tests reproduces the issue |
||
}) | ||
}) | ||
|
||
describe('and host is "builder.decentraland.zone"', () => { | ||
|
@@ -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 commentThe reason will be displayed to describe this comment to others. Learn more. This was just a typo in the test name |
||
it('should return Env.PRODUCTION', () => { | ||
mockLocation({ host: 'builder.decentraland.org' }) | ||
expect(getEnv()).toBe(Env.PRODUCTION) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,9 +118,7 @@ export function getDefaultEnv( | |
* Returns the Env to be used | ||
* @returns Env | ||
*/ | ||
export function getEnv( | ||
systemEnvVariables: EnvironmentVariables = process.env | ||
): Env { | ||
export function getEnv(systemEnvVariables?: EnvironmentVariables): Env { | ||
if (typeof window !== 'undefined') { | ||
const envFromQueryParam = getEnvFromQueryParam(window.location) | ||
if (envFromQueryParam) { | ||
|
@@ -133,5 +131,12 @@ export function getEnv( | |
} | ||
} | ||
|
||
return getDefaultEnv(systemEnvVariables) | ||
const envVars = | ||
typeof systemEnvVariables === 'undefined' && | ||
typeof process === 'object' && | ||
typeof process.env === 'object' | ||
? process.env | ||
: systemEnvVariables | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should keep the same behaviour for projects relying on the If no param is sent to the helper, and All the existing tests are working without having to make any changes on the tests themselves. |
||
|
||
return getDefaultEnv(envVars) | ||
} |
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.