From 63d172665cf97fae62629f8019d9b2dad29c7d40 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Tue, 18 Jul 2023 15:59:49 +0200 Subject: [PATCH] Reload environment variables in development (#997) * Load env vars in parallel to initial build; Do not throw on env var errors * Reload env variables when project is linked to storefront * Wrap all calls in try-catch * Resolve variables in parallel and avoid crashing on fetch fail * Add .env to Remix watcher in templates * Changesets --- .changeset/smart-cooks-divide.md | 10 ++ .changeset/tricky-pears-dress.md | 5 + package-lock.json | 16 +-- packages/cli/package.json | 2 +- packages/cli/src/commands/hydrogen/dev.ts | 45 +++++---- .../src/lib/combined-environment-variables.ts | 76 -------------- ....test.ts => environment-variables.test.ts} | 44 +++++++-- packages/cli/src/lib/environment-variables.ts | 98 +++++++++++++++++++ packages/cli/src/lib/mini-oxygen.ts | 33 ++++--- templates/demo-store/remix.config.js | 2 +- templates/hello-world/remix.config.js | 2 +- templates/skeleton/remix.config.js | 2 +- 12 files changed, 207 insertions(+), 128 deletions(-) create mode 100644 .changeset/smart-cooks-divide.md create mode 100644 .changeset/tricky-pears-dress.md delete mode 100644 packages/cli/src/lib/combined-environment-variables.ts rename packages/cli/src/lib/{combined-environment-variables.test.ts => environment-variables.test.ts} (75%) create mode 100644 packages/cli/src/lib/environment-variables.ts diff --git a/.changeset/smart-cooks-divide.md b/.changeset/smart-cooks-divide.md new file mode 100644 index 0000000000..48036be0ce --- /dev/null +++ b/.changeset/smart-cooks-divide.md @@ -0,0 +1,10 @@ +--- +'demo-store': patch +--- + +Add `.env` file to Remix watcher to allow reloading environment variables on file save. In `remix.config.js`: + +```diff +-watchPaths: ['./public'], ++watchPaths: ['./public', './.env'], +``` diff --git a/.changeset/tricky-pears-dress.md b/.changeset/tricky-pears-dress.md new file mode 100644 index 0000000000..6c18792de7 --- /dev/null +++ b/.changeset/tricky-pears-dress.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-hydrogen': minor +--- + +Reload environment variables in the development server when `.env` file is updated. Show injected variables when project is not linked to any storefront. diff --git a/package-lock.json b/package-lock.json index 43e0aaa929..b86437788f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8879,9 +8879,9 @@ "link": true }, "node_modules/@shopify/mini-oxygen": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/@shopify/mini-oxygen/-/mini-oxygen-1.6.0.tgz", - "integrity": "sha512-oghOIDqA5Lt1n32irR6fZiBhz4vGc5rdb1xnj121dJRU17VsLezWhKfCs99EestC/vO988ktVJXw3cEEPTIKwg==", + "version": "1.7.0", + "resolved": "https://registry.npmjs.org/@shopify/mini-oxygen/-/mini-oxygen-1.7.0.tgz", + "integrity": "sha512-uU9d4OhwepTAtNFTDKXijD5cgGlUQN0cX9n4rY7I8UIg4kNKaV+TvBZB3UwyNloOPOfhaOJf33zRnzHKwHsXEg==", "dependencies": { "@miniflare/cache": "^2.14.0", "@miniflare/core": "^2.14.0", @@ -32940,7 +32940,7 @@ "@remix-run/dev": "1.17.1", "@shopify/cli-kit": "3.47.0", "@shopify/hydrogen-codegen": "^0.0.2", - "@shopify/mini-oxygen": "^1.6.0", + "@shopify/mini-oxygen": "^1.7.0", "ansi-escapes": "^6.2.0", "diff": "^5.1.0", "fast-glob": "^3.2.12", @@ -41490,7 +41490,7 @@ "@remix-run/dev": "1.17.1", "@shopify/cli-kit": "3.47.0", "@shopify/hydrogen-codegen": "^0.0.2", - "@shopify/mini-oxygen": "^1.6.0", + "@shopify/mini-oxygen": "^1.7.0", "@types/diff": "^5.0.2", "@types/fs-extra": "^11.0.1", "@types/gunzip-maybe": "^1.4.0", @@ -44537,9 +44537,9 @@ } }, "@shopify/mini-oxygen": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/@shopify/mini-oxygen/-/mini-oxygen-1.6.0.tgz", - "integrity": "sha512-oghOIDqA5Lt1n32irR6fZiBhz4vGc5rdb1xnj121dJRU17VsLezWhKfCs99EestC/vO988ktVJXw3cEEPTIKwg==", + "version": "1.7.0", + "resolved": "https://registry.npmjs.org/@shopify/mini-oxygen/-/mini-oxygen-1.7.0.tgz", + "integrity": "sha512-uU9d4OhwepTAtNFTDKXijD5cgGlUQN0cX9n4rY7I8UIg4kNKaV+TvBZB3UwyNloOPOfhaOJf33zRnzHKwHsXEg==", "requires": { "@miniflare/cache": "^2.14.0", "@miniflare/core": "^2.14.0", diff --git a/packages/cli/package.json b/packages/cli/package.json index 91b211710e..24cd136750 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -39,7 +39,7 @@ "@remix-run/dev": "1.17.1", "@shopify/cli-kit": "3.47.0", "@shopify/hydrogen-codegen": "^0.0.2", - "@shopify/mini-oxygen": "^1.6.0", + "@shopify/mini-oxygen": "^1.7.0", "ansi-escapes": "^6.2.0", "diff": "^5.1.0", "fast-glob": "^3.2.12", diff --git a/packages/cli/src/commands/hydrogen/dev.ts b/packages/cli/src/commands/hydrogen/dev.ts index 34a5cdfeda..3ea27eb536 100644 --- a/packages/cli/src/commands/hydrogen/dev.ts +++ b/packages/cli/src/commands/hydrogen/dev.ts @@ -2,7 +2,7 @@ import path from 'path'; import fs from 'fs/promises'; import {outputDebug, outputInfo} from '@shopify/cli-kit/node/output'; import {fileExists} from '@shopify/cli-kit/node/fs'; -import {renderFatalError} from '@shopify/cli-kit/node/ui'; +import {renderFatalError, renderWarning} from '@shopify/cli-kit/node/ui'; import colors from '@shopify/cli-kit/node/colors'; import {copyPublicFiles} from './build.js'; import { @@ -14,11 +14,11 @@ import {muteDevLogs, warnOnce} from '../../lib/log.js'; import {deprecated, commonFlags, flagsToCamelObject} from '../../lib/flags.js'; import Command from '@shopify/cli-kit/node/base-command'; import {Flags} from '@oclif/core'; -import {startMiniOxygen} from '../../lib/mini-oxygen.js'; +import {type MiniOxygen, startMiniOxygen} from '../../lib/mini-oxygen.js'; import {checkHydrogenVersion} from '../../lib/check-version.js'; import {addVirtualRoutes} from '../../lib/virtual-routes.js'; import {spawnCodegenProcess} from '../../lib/codegen.js'; -import {combinedEnvironmentVariables} from '../../lib/combined-environment-variables.js'; +import {getAllEnvironmentVariables} from '../../lib/environment-variables.js'; import {getConfig} from '../../lib/shopify-config.js'; const LOG_REBUILDING = 'šŸ§± Rebuilding...'; @@ -121,10 +121,8 @@ async function runDev({ const serverBundleExists = () => fileExists(buildPathWorkerFile); const {shop, storefront} = await getConfig(root); - const environmentVariables = - !!shop && !!storefront?.id - ? await combinedEnvironmentVariables({root, shop, envBranch}) - : undefined; + const fetchRemote = !!shop && !!storefront?.id; + const envPromise = getAllEnvironmentVariables({root, fetchRemote, envBranch}); const [{watch}, {createFileWatchCache}] = await Promise.all([ import('@remix-run/dev/dist/compiler/watch.js'), @@ -135,21 +133,19 @@ async function runDev({ let initialBuildDurationMs = 0; let initialBuildStartTimeMs = Date.now(); - let isMiniOxygenStarted = false; + let miniOxygen: MiniOxygen; async function safeStartMiniOxygen() { - if (isMiniOxygenStarted) return; + if (miniOxygen) return; - const miniOxygen = await startMiniOxygen({ + miniOxygen = await startMiniOxygen({ root, port, watch: true, buildPathWorkerFile, buildPathClient, - environmentVariables, + env: await envPromise, }); - isMiniOxygenStarted = true; - miniOxygen.showBanner({ appName: storefront ? colors.cyan(storefront?.title) : undefined, headlinePrefix: @@ -174,6 +170,7 @@ async function runDev({ } const fileWatchCache = createFileWatchCache(); + let skipRebuildLogs = false; await watch( { @@ -188,9 +185,9 @@ async function runDev({ { reloadConfig, onBuildStart() { - if (!isInitialBuild) { - console.time(LOG_REBUILT); + if (!isInitialBuild && !skipRebuildLogs) { outputInfo(LOG_REBUILDING); + console.time(LOG_REBUILT); } }, async onBuildFinish() { @@ -198,12 +195,13 @@ async function runDev({ await copyingFiles; initialBuildDurationMs = Date.now() - initialBuildStartTimeMs; isInitialBuild = false; - } else { + } else if (!skipRebuildLogs) { + skipRebuildLogs = false; console.timeEnd(LOG_REBUILT); - if (!isMiniOxygenStarted) console.log(''); // New line + if (!miniOxygen) console.log(''); // New line } - if (!isMiniOxygenStarted) { + if (!miniOxygen) { if (!(await serverBundleExists())) { return renderFatalError({ name: 'BuildError', @@ -235,6 +233,17 @@ async function runDev({ const [relative, absolute] = getFilePaths(file); outputInfo(`\nšŸ“„ File changed: ${relative}`); + if (relative.endsWith('.env')) { + skipRebuildLogs = true; + await miniOxygen.reload({ + env: await getAllEnvironmentVariables({ + root, + fetchRemote, + envBranch, + }), + }); + } + if (absolute.startsWith(publicPath)) { await copyPublicFiles( absolute, diff --git a/packages/cli/src/lib/combined-environment-variables.ts b/packages/cli/src/lib/combined-environment-variables.ts deleted file mode 100644 index 204adcf34b..0000000000 --- a/packages/cli/src/lib/combined-environment-variables.ts +++ /dev/null @@ -1,76 +0,0 @@ -import {fileExists} from '@shopify/cli-kit/node/fs'; -import {resolvePath} from '@shopify/cli-kit/node/path'; -import {linesToColumns} from '@shopify/cli-kit/common/string'; -import {outputInfo} from '@shopify/cli-kit/node/output'; -import {readAndParseDotEnv} from '@shopify/cli-kit/node/dot-env'; -import colors from '@shopify/cli-kit/node/colors'; -import {getStorefrontEnvVariables} from './graphql/admin/pull-variables.js'; -import {login} from './auth.js'; - -interface Arguments { - envBranch?: string; - root: string; - shop: string; -} -export async function combinedEnvironmentVariables({ - envBranch, - root, - shop, -}: Arguments) { - const {session, config} = await login(root, shop); - - const remoteVariables = - (await getStorefrontEnvVariables(session, config.storefront!.id, envBranch)) - ?.environmentVariables || []; - - const formattedRemoteVariables = remoteVariables?.reduce( - (a, v) => ({...a, [v.key]: v.value}), - {}, - ); - - const dotEnvPath = resolvePath(root, '.env'); - const localEnvironmentVariables = (await fileExists(dotEnvPath)) - ? (await readAndParseDotEnv(dotEnvPath)).variables - : {}; - - const remoteKeys = new Set(remoteVariables.map((variable) => variable.key)); - - const localKeys = new Set(Object.keys(localEnvironmentVariables)); - - if ([...remoteKeys, ...localKeys].length) { - outputInfo('\nEnvironment variables injected into MiniOxygen:\n'); - } - - let rows: [string, string][] = []; - - remoteVariables - .filter(({isSecret}) => !isSecret) - .forEach(({key}) => { - if (!localKeys.has(key)) { - rows.push([key, 'from Oxygen']); - } - }); - - localKeys.forEach((key) => { - rows.push([key, 'from local .env']); - }); - - // Ensure secret variables always get added to the bottom of the list - remoteVariables - .filter(({isSecret}) => isSecret) - .forEach(({key}) => { - if (!localKeys.has(key)) { - rows.push([ - colors.dim(key), - colors.dim(`from Oxygen (Marked as secret)`), - ]); - } - }); - - outputInfo(linesToColumns(rows)); - - return { - ...formattedRemoteVariables, - ...localEnvironmentVariables, - }; -} diff --git a/packages/cli/src/lib/combined-environment-variables.test.ts b/packages/cli/src/lib/environment-variables.test.ts similarity index 75% rename from packages/cli/src/lib/combined-environment-variables.test.ts rename to packages/cli/src/lib/environment-variables.test.ts index abc313a136..fd15804398 100644 --- a/packages/cli/src/lib/combined-environment-variables.test.ts +++ b/packages/cli/src/lib/environment-variables.test.ts @@ -3,14 +3,14 @@ import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'; import {joinPath} from '@shopify/cli-kit/node/path'; import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'; -import {combinedEnvironmentVariables} from './combined-environment-variables.js'; +import {getAllEnvironmentVariables} from './environment-variables.js'; import {getStorefrontEnvVariables} from './graphql/admin/pull-variables.js'; import {login} from './auth.js'; vi.mock('./auth.js'); vi.mock('./graphql/admin/pull-variables.js'); -describe('combinedEnvironmentVariables()', () => { +describe('getAllEnvironmentVariables()', () => { const ADMIN_SESSION = { token: 'abc123', storeFqdn: 'my-shop', @@ -52,10 +52,9 @@ describe('combinedEnvironmentVariables()', () => { it('calls pullRemoteEnvironmentVariables', async () => { await inTemporaryDirectory(async (tmpDir) => { - await combinedEnvironmentVariables({ + await getAllEnvironmentVariables({ envBranch: 'main', root: tmpDir, - shop: 'my-shop', }); expect(getStorefrontEnvVariables).toHaveBeenCalledWith( @@ -66,11 +65,23 @@ describe('combinedEnvironmentVariables()', () => { }); }); + it('does not call pullRemoteEnvironmentVariables when indicated', async () => { + await inTemporaryDirectory(async (tmpDir) => { + await getAllEnvironmentVariables({ + envBranch: 'main', + root: tmpDir, + fetchRemote: false, + }); + + expect(getStorefrontEnvVariables).not.toHaveBeenCalled(); + }); + }); + it('renders a message about injection', async () => { await inTemporaryDirectory(async (tmpDir) => { const outputMock = mockAndCaptureOutput(); - await combinedEnvironmentVariables({root: tmpDir, shop: 'my-shop'}); + await getAllEnvironmentVariables({root: tmpDir}); expect(outputMock.info()).toMatch( /Environment variables injected into MiniOxygen:/, @@ -82,12 +93,27 @@ describe('combinedEnvironmentVariables()', () => { await inTemporaryDirectory(async (tmpDir) => { const outputMock = mockAndCaptureOutput(); - await combinedEnvironmentVariables({root: tmpDir, shop: 'my-shop'}); + await getAllEnvironmentVariables({root: tmpDir}); expect(outputMock.info()).toMatch(/PUBLIC_API_TOKEN\s+from Oxygen/); }); }); + it('doest not fail on network errors', async () => { + await inTemporaryDirectory(async (tmpDir) => { + vi.mocked(getStorefrontEnvVariables).mockRejectedValue( + new Error('Network error'), + ); + + const outputMock = mockAndCaptureOutput(); + + await getAllEnvironmentVariables({root: tmpDir}); + + expect(outputMock.info()).not.toMatch(/PUBLIC_API_TOKEN\s+from Oxygen/); + expect(outputMock.warn()).toMatch(/failed to load/i); + }); + }); + describe('when one of the variables is a secret', () => { beforeEach(() => { vi.mocked(getStorefrontEnvVariables).mockResolvedValue({ @@ -107,7 +133,7 @@ describe('combinedEnvironmentVariables()', () => { await inTemporaryDirectory(async (tmpDir) => { const outputMock = mockAndCaptureOutput(); - await combinedEnvironmentVariables({root: tmpDir, shop: 'my-shop'}); + await getAllEnvironmentVariables({root: tmpDir}); expect(outputMock.info()).toMatch( /PUBLIC_API_TOKEN\s+from Oxygen \(Marked as secret\)/, @@ -124,7 +150,7 @@ describe('combinedEnvironmentVariables()', () => { const outputMock = mockAndCaptureOutput(); - await combinedEnvironmentVariables({root: tmpDir, shop: 'my-shop'}); + await getAllEnvironmentVariables({root: tmpDir}); expect(outputMock.info()).toMatch(/LOCAL_TOKEN\s+from local \.env/); }); @@ -138,7 +164,7 @@ describe('combinedEnvironmentVariables()', () => { const outputMock = mockAndCaptureOutput(); - await combinedEnvironmentVariables({root: tmpDir, shop: 'my-shop'}); + await getAllEnvironmentVariables({root: tmpDir}); expect(outputMock.info()).not.toMatch( /PUBLIC_API_TOKEN\s+from Oxygen/, diff --git a/packages/cli/src/lib/environment-variables.ts b/packages/cli/src/lib/environment-variables.ts new file mode 100644 index 0000000000..69beb9deea --- /dev/null +++ b/packages/cli/src/lib/environment-variables.ts @@ -0,0 +1,98 @@ +import {fileExists} from '@shopify/cli-kit/node/fs'; +import {resolvePath} from '@shopify/cli-kit/node/path'; +import {linesToColumns} from '@shopify/cli-kit/common/string'; +import {outputInfo} from '@shopify/cli-kit/node/output'; +import {readAndParseDotEnv} from '@shopify/cli-kit/node/dot-env'; +import {renderWarning} from '@shopify/cli-kit/node/ui'; +import colors from '@shopify/cli-kit/node/colors'; +import {getStorefrontEnvVariables} from './graphql/admin/pull-variables.js'; +import {login} from './auth.js'; + +interface Arguments { + envBranch?: string; + root: string; + fetchRemote?: boolean; +} + +type EnvMap = Record; + +export async function getAllEnvironmentVariables({ + root, + envBranch, + fetchRemote = true, +}: Arguments) { + const dotEnvPath = resolvePath(root, '.env'); + + const [{remoteVariables, remoteSecrets}, {variables: localVariables}] = + await Promise.all([ + // Get remote vars + fetchRemote + ? getRemoteVariables(root, envBranch) + : {remoteVariables: {} as EnvMap, remoteSecrets: {} as EnvMap}, + // Get local vars + fileExists(dotEnvPath).then((exists) => + exists ? readAndParseDotEnv(dotEnvPath) : {variables: {} as EnvMap}, + ), + ]); + + const remoteSecretKeys = Object.keys(remoteSecrets); + const remotePublicKeys = Object.keys(remoteVariables); + const localKeys = Object.keys(localVariables); + + if ( + localKeys.length > 0 || + remotePublicKeys.length + remoteSecretKeys.length > 0 + ) { + outputInfo('\nEnvironment variables injected into MiniOxygen:\n'); + + outputInfo( + linesToColumns([ + ...remotePublicKeys + .filter((key) => !localKeys.includes(key)) + .map((key) => [key, 'from Oxygen']), + ...localKeys.map((key) => [key, 'from local .env']), + // Ensure secret variables always get added to the bottom of the list + ...remoteSecretKeys + .filter((key) => !localKeys.includes(key)) + .map((key) => [ + colors.dim(key), + colors.dim('from Oxygen (Marked as secret)'), + ]), + ]), + ); + } + + return { + ...remoteSecrets, + ...remoteVariables, + ...localVariables, + }; +} + +async function getRemoteVariables(root: string, envBranch?: string) { + const {session, config} = await login(root); + + const envVariables = + ( + await getStorefrontEnvVariables( + session, + config.storefront!.id, + envBranch, + ).catch((error) => { + renderWarning({ + headline: `Failed to load environment variables. The development server will still start, but the following error occurred:`, + body: error?.stack ?? error?.message ?? error, + }); + }) + )?.environmentVariables || []; + + const remoteVariables: EnvMap = {}; + const remoteSecrets: EnvMap = {}; + + for (const {key, value, isSecret} of envVariables) { + if (isSecret) remoteSecrets[key] = value; + else remoteVariables[key] = value; + } + + return {remoteVariables, remoteSecrets}; +} diff --git a/packages/cli/src/lib/mini-oxygen.ts b/packages/cli/src/lib/mini-oxygen.ts index 691cfebac0..b6da3f6827 100644 --- a/packages/cli/src/lib/mini-oxygen.ts +++ b/packages/cli/src/lib/mini-oxygen.ts @@ -14,24 +14,27 @@ type MiniOxygenOptions = { watch?: boolean; buildPathClient: string; buildPathWorkerFile: string; - environmentVariables?: {[key: string]: string}; + env?: {[key: string]: string}; }; +export type MiniOxygen = Awaited>; + export async function startMiniOxygen({ root, port = 3000, watch = false, buildPathWorkerFile, buildPathClient, - environmentVariables = {}, + env, }: MiniOxygenOptions) { - const {default: miniOxygen} = await import('@shopify/mini-oxygen'); + const {default: miniOxygenImport} = await import('@shopify/mini-oxygen'); const miniOxygenPreview = - miniOxygen.default ?? (miniOxygen as unknown as typeof miniOxygen.default); + miniOxygenImport.default ?? + (miniOxygenImport as unknown as typeof miniOxygenImport.default); const dotenvPath = resolvePath(root, '.env'); - const {port: actualPort} = await miniOxygenPreview({ + const miniOxygen = await miniOxygenPreview({ workerFile: buildPathWorkerFile, assetsDir: buildPathClient, publicPath: '', @@ -40,14 +43,10 @@ export async function startMiniOxygen({ autoReload: watch, modules: true, env: { - ...environmentVariables, + ...env, ...process.env, }, - envPath: - !Object.keys(environmentVariables).length && - (await fileExists(dotenvPath)) - ? dotenvPath - : undefined, + envPath: !env && (await fileExists(dotenvPath)) ? dotenvPath : undefined, log: () => {}, buildWatchPaths: watch ? [resolvePath(root, buildPathWorkerFile)] @@ -61,11 +60,19 @@ export async function startMiniOxygen({ ), }); - const listeningAt = `http://localhost:${actualPort}`; + const listeningAt = `http://localhost:${miniOxygen.port}`; return { listeningAt, - port: actualPort, + port: miniOxygen.port, + reload(nextOptions?: Partial>) { + return miniOxygen.reload({ + env: { + ...(nextOptions?.env ?? env), + ...process.env, + }, + }); + }, showBanner(options?: { mode?: string; headlinePrefix?: string; diff --git a/templates/demo-store/remix.config.js b/templates/demo-store/remix.config.js index 238318bcbb..4ed517aa7d 100644 --- a/templates/demo-store/remix.config.js +++ b/templates/demo-store/remix.config.js @@ -2,7 +2,7 @@ module.exports = { appDirectory: 'app', ignoredRouteFiles: ['**/.*'], - watchPaths: ['./public'], + watchPaths: ['./public', './.env'], server: './server.ts', /** * The following settings are required to deploy Hydrogen apps to Oxygen: diff --git a/templates/hello-world/remix.config.js b/templates/hello-world/remix.config.js index 79db25f665..8d313a3be9 100644 --- a/templates/hello-world/remix.config.js +++ b/templates/hello-world/remix.config.js @@ -2,7 +2,7 @@ module.exports = { appDirectory: 'app', ignoredRouteFiles: ['**/.*'], - watchPaths: ['./public'], + watchPaths: ['./public', './.env'], server: './server.ts', /** * The following settings are required to deploy Hydrogen apps to Oxygen: diff --git a/templates/skeleton/remix.config.js b/templates/skeleton/remix.config.js index 79db25f665..8d313a3be9 100644 --- a/templates/skeleton/remix.config.js +++ b/templates/skeleton/remix.config.js @@ -2,7 +2,7 @@ module.exports = { appDirectory: 'app', ignoredRouteFiles: ['**/.*'], - watchPaths: ['./public'], + watchPaths: ['./public', './.env'], server: './server.ts', /** * The following settings are required to deploy Hydrogen apps to Oxygen: