Reload environment variables in development (Shopify#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

@@ -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'],
+```

.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.

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",

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,22 +185,23 @@ async function runDev({
     {
       reloadConfig,
       onBuildStart() {
-        if (!isInitialBuild) {
-          console.time(LOG_REBUILT);
+        if (!isInitialBuild && !skipRebuildLogs) {
           outputInfo(LOG_REBUILDING);
+          console.time(LOG_REBUILT);
         }
       },
       async onBuildFinish() {
         if (isInitialBuild) {
           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,

packages/cli/src/lib/combined-environment-variables.test.ts → 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/,