Merge pull request #4750 from Shopify/fix-dot-env-files-overwriting-c…

…onetnt Safely patch env files on deploy
Shopify · Oct 31, 2024 · fae0225 · fae0225
2 parents c8914f9 + 066bcb1
commit fae0225
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/packages/app/src/cli/models/app/identifiers.test.ts b/packages/app/src/cli/models/app/identifiers.test.ts
@@ -6,7 +6,7 @@ import {fileExists, inTemporaryDirectory, readFile, writeFile} from '@shopify/cl
 import {joinPath} from '@shopify/cli-kit/node/path'
 
 describe('updateAppIdentifiers', () => {
-  test('persists the ids that are not env variables when deploying', async () => {
+  test('persists the ids that are not env variables when deploying, creating a new file', async () => {
     await inTemporaryDirectory(async (tmpDir: string) => {
       // Given
       const uiExtension = await testUIExtension()
@@ -37,9 +37,11 @@ describe('updateAppIdentifiers', () => {
     })
   })
 
-  test('persists the ids in the config-specific env file when deploying', async () => {
+  test('persists the ids in the config-specific env file when deploying, updating the existing file', async () => {
     await inTemporaryDirectory(async (tmpDir: string) => {
       // Given
+      const dotEnvFilePath = joinPath(tmpDir, '.env.staging')
+      await writeFile(dotEnvFilePath, '#comment\nEXISTING_VAR=value\nSHOPIFY_MY_EXTENSION_ID=OLDID\n#anothercomment')
       const uiExtension = await testUIExtension()
       const app = testAppWithConfig({
         app: {
@@ -65,7 +67,12 @@ describe('updateAppIdentifiers', () => {
       })
 
       // Then
-      const dotEnvFile = await readAndParseDotEnv(joinPath(tmpDir, '.env.staging'))
+      const dotEnvFileContent = await readFile(dotEnvFilePath)
+      const dotEnvFile = await readAndParseDotEnv(dotEnvFilePath)
+      expect(dotEnvFileContent).toEqual(
+        '#comment\nEXISTING_VAR=value\nSHOPIFY_MY_EXTENSION_ID=BAR\n#anothercomment\nSHOPIFY_API_KEY=FOO',
+      )
+      expect(dotEnvFile.variables.EXISTING_VAR).toEqual('value')
       expect(dotEnvFile.variables.SHOPIFY_API_KEY).toEqual('FOO')
       expect(dotEnvFile.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('BAR')
       expect(gotApp.dotenv?.variables.SHOPIFY_API_KEY).toEqual('FOO')

diff --git a/packages/app/src/cli/models/app/identifiers.ts b/packages/app/src/cli/models/app/identifiers.ts
@@ -1,11 +1,11 @@
 import {getDotEnvFileName} from './loader.js'
 import {ExtensionInstance} from '../extensions/extension-instance.js'
 import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
-import {writeDotEnv} from '@shopify/cli-kit/node/dot-env'
+import {patchEnvFile} from '@shopify/cli-kit/node/dot-env'
 import {constantize} from '@shopify/cli-kit/common/string'
 import {joinPath} from '@shopify/cli-kit/node/path'
-import {readFile, writeFile} from '@shopify/cli-kit/node/fs'
-import {getPathValue} from '@shopify/cli-kit/common/object'
+import {fileExists, readFile, writeFile} from '@shopify/cli-kit/node/fs'
+import {deepCompare, getPathValue} from '@shopify/cli-kit/common/object'
 import {decodeToml} from '@shopify/cli-kit/node/toml'
 import type {AppInterface} from './app.js'
 
@@ -79,12 +79,15 @@ export async function updateAppIdentifiers(
     }
   })
 
-  const write =
-    JSON.stringify(dotenvFile.variables) !== JSON.stringify(updatedVariables) &&
-    (command === 'deploy' || command === 'release')
+  const contentIsEqual = deepCompare(dotenvFile.variables, updatedVariables)
+  const writeToFile = !contentIsEqual && (command === 'deploy' || command === 'release')
   dotenvFile.variables = updatedVariables
-  if (write) {
-    await writeDotEnv(dotenvFile)
+
+  if (writeToFile) {
+    const dotEnvFileExists = await fileExists(dotenvFile.path)
+    const envFileContent = dotEnvFileExists ? await readFile(dotenvFile.path) : ''
+    const updatedEnvFileContent = patchEnvFile(envFileContent, updatedVariables)
+    await writeFile(dotenvFile.path, updatedEnvFileContent)
   }
 
   // eslint-disable-next-line require-atomic-updates