From e0f520e62adcc9336cd3da174d9f218d2168884f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Connor=20B=C3=A4r?= Date: Thu, 24 Oct 2024 09:02:29 +0200 Subject: [PATCH 1/2] Safely access env variables --- .changeset/chilly-scissors-remember.md | 5 ++ .../circuit-ui/components/Display/Display.tsx | 3 +- .../components/Headline/Headline.tsx | 3 +- packages/circuit-ui/util/env.spec.ts | 52 +++++++++++++++++++ packages/circuit-ui/util/env.ts | 29 +++++++++++ 5 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 .changeset/chilly-scissors-remember.md create mode 100644 packages/circuit-ui/util/env.spec.ts create mode 100644 packages/circuit-ui/util/env.ts diff --git a/.changeset/chilly-scissors-remember.md b/.changeset/chilly-scissors-remember.md new file mode 100644 index 0000000000..16a34568d4 --- /dev/null +++ b/.changeset/chilly-scissors-remember.md @@ -0,0 +1,5 @@ +--- +"@sumup-oss/circuit-ui": patch +--- + +Fixed safely accessing environment variables in environments where the `process` variable is undefined. diff --git a/packages/circuit-ui/components/Display/Display.tsx b/packages/circuit-ui/components/Display/Display.tsx index 43bfb21e0f..c0c1573f92 100644 --- a/packages/circuit-ui/components/Display/Display.tsx +++ b/packages/circuit-ui/components/Display/Display.tsx @@ -18,6 +18,7 @@ import { forwardRef, type HTMLAttributes } from 'react'; import { clsx } from '../../styles/clsx.js'; import { CircuitError } from '../../util/errors.js'; import { deprecate } from '../../util/logger.js'; +import { getEnv } from '../../util/env.js'; import classes from './Display.module.css'; @@ -75,7 +76,7 @@ export const Display = forwardRef( if ( process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test' && - !process?.env?.UNSAFE_DISABLE_ELEMENT_ERRORS && + !getEnv('UNSAFE_DISABLE_ELEMENT_ERRORS') && !as ) { throw new CircuitError('Display', 'The `as` prop is required.'); diff --git a/packages/circuit-ui/components/Headline/Headline.tsx b/packages/circuit-ui/components/Headline/Headline.tsx index a4b0e04064..a3daa64058 100644 --- a/packages/circuit-ui/components/Headline/Headline.tsx +++ b/packages/circuit-ui/components/Headline/Headline.tsx @@ -18,6 +18,7 @@ import { forwardRef, type HTMLAttributes } from 'react'; import { clsx } from '../../styles/clsx.js'; import { CircuitError } from '../../util/errors.js'; import { deprecate } from '../../util/logger.js'; +import { getEnv } from '../../util/env.js'; import classes from './Headline.module.css'; @@ -68,7 +69,7 @@ export const Headline = forwardRef( if ( process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test' && - !process?.env?.UNSAFE_DISABLE_ELEMENT_ERRORS && + !getEnv('UNSAFE_DISABLE_ELEMENT_ERRORS') && !as ) { throw new CircuitError('Headline', 'The `as` prop is required.'); diff --git a/packages/circuit-ui/util/env.spec.ts b/packages/circuit-ui/util/env.spec.ts new file mode 100644 index 0000000000..a939ef4a11 --- /dev/null +++ b/packages/circuit-ui/util/env.spec.ts @@ -0,0 +1,52 @@ +/** + * Copyright 2024, SumUp Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { beforeEach, describe, expect, it } from 'vitest'; + +import { getEnv } from './env.js'; + +describe('env', () => { + describe('getEnvironment', () => { + const originalProcess = process; + + beforeEach(() => { + process = originalProcess; + process.env = {}; + }); + + it('should return the environment variable`', () => { + process.env.FOO = 'foo'; + const actual = getEnv('FOO'); + expect(actual).toBe('foo'); + }); + + it('should return undefined if the environment variable is not defined', () => { + const actual = getEnv('FOO'); + expect(actual).toBeUndefined(); + }); + + it('should return undefined if `process` is not defined', () => { + // @ts-expect-error We're testing for this error + process = undefined; + const actual = getEnv('FOO'); + expect(actual).toBeUndefined(); + }); + + it('should throw an error when used for `NODE_ENV`', () => { + const actual = () => getEnv('NODE_ENV'); + expect(actual).toThrowError(); + }); + }); +}); diff --git a/packages/circuit-ui/util/env.ts b/packages/circuit-ui/util/env.ts new file mode 100644 index 0000000000..6c1fdc2105 --- /dev/null +++ b/packages/circuit-ui/util/env.ts @@ -0,0 +1,29 @@ +/** + * Copyright 2024, SumUp Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export function getEnv(name: string) { + if (name === 'NODE_ENV') { + // Some bundlers have special logic for `process.env.NODE_ENV` which + // relies on it being written as a continuous string. Destructuring or + // dynamically accessing `NODE_ENV` can break this logic. + throw new Error('Do not dynamically access NODE_ENV'); + } + + if (typeof process !== 'undefined') { + return process.env?.[name]; + } + + return undefined; +} From ac3b84714c12dde099897b4103043441600d1e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Connor=20B=C3=A4r?= Date: Thu, 24 Oct 2024 12:53:40 +0200 Subject: [PATCH 2/2] Rename to getEnvVariable --- packages/circuit-ui/components/Display/Display.tsx | 4 ++-- packages/circuit-ui/components/Headline/Headline.tsx | 4 ++-- packages/circuit-ui/util/env.spec.ts | 12 ++++++------ packages/circuit-ui/util/env.ts | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/circuit-ui/components/Display/Display.tsx b/packages/circuit-ui/components/Display/Display.tsx index c0c1573f92..28c03d65e7 100644 --- a/packages/circuit-ui/components/Display/Display.tsx +++ b/packages/circuit-ui/components/Display/Display.tsx @@ -18,7 +18,7 @@ import { forwardRef, type HTMLAttributes } from 'react'; import { clsx } from '../../styles/clsx.js'; import { CircuitError } from '../../util/errors.js'; import { deprecate } from '../../util/logger.js'; -import { getEnv } from '../../util/env.js'; +import { getEnvVariable } from '../../util/env.js'; import classes from './Display.module.css'; @@ -76,7 +76,7 @@ export const Display = forwardRef( if ( process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test' && - !getEnv('UNSAFE_DISABLE_ELEMENT_ERRORS') && + !getEnvVariable('UNSAFE_DISABLE_ELEMENT_ERRORS') && !as ) { throw new CircuitError('Display', 'The `as` prop is required.'); diff --git a/packages/circuit-ui/components/Headline/Headline.tsx b/packages/circuit-ui/components/Headline/Headline.tsx index a3daa64058..1da5d60152 100644 --- a/packages/circuit-ui/components/Headline/Headline.tsx +++ b/packages/circuit-ui/components/Headline/Headline.tsx @@ -18,7 +18,7 @@ import { forwardRef, type HTMLAttributes } from 'react'; import { clsx } from '../../styles/clsx.js'; import { CircuitError } from '../../util/errors.js'; import { deprecate } from '../../util/logger.js'; -import { getEnv } from '../../util/env.js'; +import { getEnvVariable } from '../../util/env.js'; import classes from './Headline.module.css'; @@ -69,7 +69,7 @@ export const Headline = forwardRef( if ( process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test' && - !getEnv('UNSAFE_DISABLE_ELEMENT_ERRORS') && + !getEnvVariable('UNSAFE_DISABLE_ELEMENT_ERRORS') && !as ) { throw new CircuitError('Headline', 'The `as` prop is required.'); diff --git a/packages/circuit-ui/util/env.spec.ts b/packages/circuit-ui/util/env.spec.ts index a939ef4a11..57177f7392 100644 --- a/packages/circuit-ui/util/env.spec.ts +++ b/packages/circuit-ui/util/env.spec.ts @@ -15,10 +15,10 @@ import { beforeEach, describe, expect, it } from 'vitest'; -import { getEnv } from './env.js'; +import { getEnvVariable } from './env.js'; describe('env', () => { - describe('getEnvironment', () => { + describe('getEnvVariable', () => { const originalProcess = process; beforeEach(() => { @@ -28,24 +28,24 @@ describe('env', () => { it('should return the environment variable`', () => { process.env.FOO = 'foo'; - const actual = getEnv('FOO'); + const actual = getEnvVariable('FOO'); expect(actual).toBe('foo'); }); it('should return undefined if the environment variable is not defined', () => { - const actual = getEnv('FOO'); + const actual = getEnvVariable('FOO'); expect(actual).toBeUndefined(); }); it('should return undefined if `process` is not defined', () => { // @ts-expect-error We're testing for this error process = undefined; - const actual = getEnv('FOO'); + const actual = getEnvVariable('FOO'); expect(actual).toBeUndefined(); }); it('should throw an error when used for `NODE_ENV`', () => { - const actual = () => getEnv('NODE_ENV'); + const actual = () => getEnvVariable('NODE_ENV'); expect(actual).toThrowError(); }); }); diff --git a/packages/circuit-ui/util/env.ts b/packages/circuit-ui/util/env.ts index 6c1fdc2105..6740b481c4 100644 --- a/packages/circuit-ui/util/env.ts +++ b/packages/circuit-ui/util/env.ts @@ -13,7 +13,7 @@ * limitations under the License. */ -export function getEnv(name: string) { +export function getEnvVariable(name: string) { if (name === 'NODE_ENV') { // Some bundlers have special logic for `process.env.NODE_ENV` which // relies on it being written as a continuous string. Destructuring or