Merge pull request #5720 from Shopify/shauns/04-29-remove_shopify_cli_disable_wasm_toml_patch_opt-out

Remove SHOPIFY_CLI_DISABLE_WASM_TOML_PATCH opt-out

Author: MitchDickinson

.changeset/grumpy-doors-march.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': minor
+---
+
+Remove opt-out of TOML patching with SHOPIFY_CLI_DISABLE_WASM_TOML_PATCH

packages/app/src/cli/constants.ts

@@ -7,7 +7,6 @@ export const environmentVariableNames = {
   mkcertBinaryPath: 'SHOPIFY_CLI_MKCERT_BINARY',
   disableUnsupportedConfigPropertyChecks: 'SHOPIFY_CLI_DISABLE_UNSUPPORTED_CONFIG_PROPERTY_CHECKS',
   disableMinificationOnDev: 'SHOPIFY_CLI_DISABLE_MINIFICATION_ON_DEV',
-  disableWasmTomlPatch: 'SHOPIFY_CLI_DISABLE_WASM_TOML_PATCH',
 }
 
 export const configurationFileNames = {

packages/app/src/cli/services/app/patch-app-configuration-file.test.ts

@@ -4,12 +4,9 @@ import {
   unsetAppConfigValue,
   setManyAppConfigValues,
 } from './patch-app-configuration-file.js'
-import {getAppVersionedSchema} from '../../models/app/app.js'
-import {loadLocalExtensionsSpecifications} from '../../models/extensions/load-specifications.js'
-import {environmentVariableNames} from '../../constants.js'
 import {readFile, writeFileSync, inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
-import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
+import {describe, expect, test} from 'vitest'
 
 const defaultToml = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration
 client_id = "12345"
@@ -31,8 +28,6 @@ redirect_urls = [
 api_version = "2023-04"
 `
 
-const schema = getAppVersionedSchema(await loadLocalExtensionsSpecifications(), false)
-
 function writeDefaulToml(tmpDir: string) {
   const configPath = joinPath(tmpDir, 'shopify.app.toml')
   writeFileSync(configPath, defaultToml)
@@ -120,20 +115,12 @@ describe('patchAppHiddenConfigFile', () => {
   })
 })
 
-describe.each([false, true])('setAppConfigValue (disableWasmTomlPatch: %s)', (disableWasmTomlPatch) => {
-  beforeEach(() => {
-    vi.stubEnv(environmentVariableNames.disableWasmTomlPatch, disableWasmTomlPatch.toString())
-  })
-
-  afterEach(() => {
-    vi.unstubAllEnvs()
-  })
-
+describe('setAppConfigValue', () => {
   test('sets a top-level value in the configuration', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setAppConfigValue(configPath, 'name', 'Updated App Name', schema)
+      await setAppConfigValue(configPath, 'name', 'Updated App Name')
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('name = "Updated App Name"')
@@ -144,7 +131,7 @@ describe.each([false, true])('setAppConfigValue (disableWasmTomlPatch: %s)', (di
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com', schema)
+      await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com')
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('[build]')
@@ -156,7 +143,7 @@ describe.each([false, true])('setAppConfigValue (disableWasmTomlPatch: %s)', (di
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setAppConfigValue(configPath, 'build.auth.settings', true, schema)
+      await setAppConfigValue(configPath, 'build.auth.settings', true)
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('[build.auth]')
@@ -165,20 +152,12 @@ describe.each([false, true])('setAppConfigValue (disableWasmTomlPatch: %s)', (di
   })
 })
 
-describe.each([false, true])('unsetAppConfigValue (disableWasmTomlPatch: %s)', (disableWasmTomlPatch) => {
-  beforeEach(() => {
-    vi.stubEnv(environmentVariableNames.disableWasmTomlPatch, disableWasmTomlPatch.toString())
-  })
-
-  afterEach(() => {
-    vi.unstubAllEnvs()
-  })
-
+describe('unsetAppConfigValue', () => {
   test('unsets a top-level value in the configuration', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await unsetAppConfigValue(configPath, 'name', schema)
+      await unsetAppConfigValue(configPath, 'name')
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).not.toContain('name = "app1"')
@@ -190,10 +169,10 @@ describe.each([false, true])('unsetAppConfigValue (disableWasmTomlPatch: %s)', (
       const configPath = writeDefaulToml(tmpDir)
 
       // Add a value first
-      await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com', schema)
+      await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com')
 
       // Now unset it
-      await unsetAppConfigValue(configPath, 'build.dev_store_url', schema)
+      await unsetAppConfigValue(configPath, 'build.dev_store_url')
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('[build]')
@@ -202,27 +181,15 @@ describe.each([false, true])('unsetAppConfigValue (disableWasmTomlPatch: %s)', (
   })
 })
 
-describe.each([false, true])('setManyAppConfigValues (disableWasmTomlPatch: %s)', (disableWasmTomlPatch) => {
-  beforeEach(() => {
-    vi.stubEnv(environmentVariableNames.disableWasmTomlPatch, disableWasmTomlPatch.toString())
-  })
-
-  afterEach(() => {
-    vi.unstubAllEnvs()
-  })
-
+describe('setManyAppConfigValues', () => {
   test('sets multiple top-level values in the configuration', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setManyAppConfigValues(
-        configPath,
-        [
-          {keyPath: 'name', value: 'Updated App Name'},
-          {keyPath: 'client_id', value: '67890'},
-        ],
-        schema,
-      )
+      await setManyAppConfigValues(configPath, [
+        {keyPath: 'name', value: 'Updated App Name'},
+        {keyPath: 'client_id', value: '67890'},
+      ])
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('name = "Updated App Name"')
@@ -234,14 +201,10 @@ describe.each([false, true])('setManyAppConfigValues (disableWasmTomlPatch: %s)'
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setManyAppConfigValues(
-        configPath,
-        [
-          {keyPath: 'name', value: 'Updated App Name'},
-          {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'},
-        ],
-        schema,
-      )
+      await setManyAppConfigValues(configPath, [
+        {keyPath: 'name', value: 'Updated App Name'},
+        {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'},
+      ])
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('name = "Updated App Name"')
@@ -254,11 +217,9 @@ describe.each([false, true])('setManyAppConfigValues (disableWasmTomlPatch: %s)'
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setManyAppConfigValues(
-        configPath,
-        [{keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']}],
-        schema,
-      )
+      await setManyAppConfigValues(configPath, [
+        {keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']},
+      ])
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('[auth]')
@@ -274,14 +235,10 @@ describe.each([false, true])('setManyAppConfigValues (disableWasmTomlPatch: %s)'
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setManyAppConfigValues(
-        configPath,
-        [
-          {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'},
-          {keyPath: 'build.automatically_update_urls_on_dev', value: true},
-        ],
-        schema,
-      )
+      await setManyAppConfigValues(configPath, [
+        {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'},
+        {keyPath: 'build.automatically_update_urls_on_dev', value: true},
+      ])
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('[build]')
@@ -294,16 +251,12 @@ describe.each([false, true])('setManyAppConfigValues (disableWasmTomlPatch: %s)'
     await inTemporaryDirectory(async (tmpDir) => {
       const configPath = writeDefaulToml(tmpDir)
 
-      await setManyAppConfigValues(
-        configPath,
-        [
-          {keyPath: 'name', value: 'Updated App Name'},
-          {keyPath: 'application_url', value: 'https://example.com'},
-          {keyPath: 'access_scopes.use_legacy_install_flow', value: false},
-          {keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']},
-        ],
-        schema,
-      )
+      await setManyAppConfigValues(configPath, [
+        {keyPath: 'name', value: 'Updated App Name'},
+        {keyPath: 'application_url', value: 'https://example.com'},
+        {keyPath: 'access_scopes.use_legacy_install_flow', value: false},
+        {keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']},
+      ])
 
       const updatedTomlFile = await readFile(configPath)
       expect(updatedTomlFile).toContain('name = "Updated App Name"')

packages/app/src/cli/services/app/patch-app-configuration-file.ts

@@ -1,11 +1,7 @@
-import {addDefaultCommentsToToml} from './write-app-configuration-file.js'
 import {AppHiddenConfig} from '../../models/app/app.js'
-import {environmentVariableNames} from '../../constants.js'
 import {deepMergeObjects} from '@shopify/cli-kit/common/object'
 import {readFile, writeFile} from '@shopify/cli-kit/node/fs'
 import {zod} from '@shopify/cli-kit/node/schema'
-import {decodeToml, encodeToml} from '@shopify/cli-kit/node/toml'
-import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
 import {updateTomlValues} from '@shopify/toml-patch'
 
 export interface PatchTomlOptions {
@@ -16,41 +12,6 @@ export interface PatchTomlOptions {
 
 type TomlPatchValue = string | number | boolean | undefined | string[]
 
-function shouldUseWasmTomlPatch(env = process.env): boolean {
-  return !isTruthy(env[environmentVariableNames.disableWasmTomlPatch])
-}
-
-/**
- * Updates an app configuration file with the given patch.
- *
- * Only updates the given fields in the patch and leaves the rest of the file unchanged.
- * Keeps the same order of the keys as the original file.
- *
- * New keys are always added at the end of the file.
- *
- * @param path - The path to the app configuration file.
- * @param patch - The patch to apply to the app configuration file.
- * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated.
- * @internal Internal function, use setAppConfigValue, unsetAppConfigValue, or setManyAppConfigValues instead
- */
-async function patchAppConfigurationFile({path, patch, schema}: PatchTomlOptions) {
-  const tomlContents = await readFile(path)
-  const configuration = decodeToml(tomlContents)
-
-  // Deep merge the configuration with the patch.
-  // Use replaceArrayStrategy to replace the destination array with the source array. (Arrays are not merged)
-  const updatedConfig = deepMergeObjects(configuration, patch, replaceArrayStrategy)
-
-  // Re-parse the config with the schema to validate the patch
-  // Make every field optional to not crash on tomls that are missing fields.
-  const validSchema = schema ?? zod.object({}).passthrough()
-  validSchema.partial().parse(updatedConfig)
-
-  let encodedString = encodeToml(updatedConfig)
-  encodedString = addDefaultCommentsToToml(encodedString)
-  await writeFile(path, encodedString)
-}
-
 async function patchAppConfigurationFileWithWasm(
   path: string,
   configValues: {keyPath: string; value: TomlPatchValue}[],
@@ -69,98 +30,40 @@ async function patchAppConfigurationFileWithWasm(
  * @param path - The path to the app configuration file.
  * @param keyPath - The dotted key path to set the value at (e.g. 'build.dev_store_url')
  * @param value - The value to set
- * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated.
  */
-export async function setAppConfigValue(
-  path: string,
-  keyPath: string,
-  value: TomlPatchValue,
-  schema?: zod.AnyZodObject,
-) {
-  if (shouldUseWasmTomlPatch()) {
-    return patchAppConfigurationFileWithWasm(path, [{keyPath, value}])
-  }
-  const patch = createPatchFromDottedPath(keyPath, value)
-  await patchAppConfigurationFile({path, patch, schema})
+export async function setAppConfigValue(path: string, keyPath: string, value: TomlPatchValue) {
+  return patchAppConfigurationFileWithWasm(path, [{keyPath, value}])
 }
 
 /**
  * Sets multiple values in the app configuration file.
  *
  * @param path - The path to the app configuration file
  * @param configValues - Array of keyPath and value pairs to set
- * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated.
  *
  * @example
  * ```ts
  * await setManyAppConfigValues('shopify.app.toml', [
  *   { keyPath: 'application_url', value: 'https://example.com' },
  *   { keyPath: 'auth.redirect_urls', value: ['https://example.com/callback'] }
- * ], schema)
+ * ])
  * ```
  */
-export async function setManyAppConfigValues(
-  path: string,
-  configValues: {keyPath: string; value: TomlPatchValue}[],
-  schema?: zod.AnyZodObject,
-) {
-  if (shouldUseWasmTomlPatch()) {
-    return patchAppConfigurationFileWithWasm(path, configValues)
-  }
-  const patch = configValues.reduce((acc, {keyPath, value}) => {
-    const valuePatch = createPatchFromDottedPath(keyPath, value)
-    return deepMergeObjects(acc, valuePatch, replaceArrayStrategy)
-  }, {})
-
-  await patchAppConfigurationFile({path, patch, schema})
+export async function setManyAppConfigValues(path: string, configValues: {keyPath: string; value: TomlPatchValue}[]) {
+  return patchAppConfigurationFileWithWasm(path, configValues)
 }
 
 /**
  * Unsets a value in the app configuration file based on a dotted key path.
  *
  * @param path - The path to the app configuration file.
  * @param keyPath - The dotted key path to unset (e.g. 'build.include_config_on_deploy')
- * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated.
  */
-export async function unsetAppConfigValue(path: string, keyPath: string, schema?: zod.AnyZodObject) {
-  if (shouldUseWasmTomlPatch()) {
-    return patchAppConfigurationFileWithWasm(path, [{keyPath, value: undefined}])
-  }
-  const patch = createPatchFromDottedPath(keyPath, undefined)
-  await patchAppConfigurationFile({path, patch, schema})
-}
-
-/**
- * Creates a patch object from a dotted key path and a value
- * For example, 'build.dev_store_url' with value 'example.myshopify.com'
- * will create \{ build: \{ dev_store_url: 'example.myshopify.com' \} \}
- */
-function createPatchFromDottedPath(keyPath: string, value: unknown): {[key: string]: unknown} {
-  const keys = keyPath.split('.')
-  if (keys.length === 1) {
-    return {[keyPath]: value}
-  }
-
-  const obj: {[key: string]: unknown} = {}
-  let currentObj = obj
-
-  for (let i = 0; i < keys.length - 1; i++) {
-    const key = keys[i]
-    if (key) {
-      currentObj[key] = {}
-      currentObj = currentObj[key] as {[key: string]: unknown}
-    }
-  }
-
-  const lastKey = keys[keys.length - 1]
-  if (lastKey) {
-    currentObj[lastKey] = value
-  }
-
-  return obj
+export async function unsetAppConfigValue(path: string, keyPath: string) {
+  return patchAppConfigurationFileWithWasm(path, [{keyPath, value: undefined}])
 }
 
-export function replaceArrayStrategy(_: unknown[], newArray: unknown[]): unknown[] {
+function replaceArrayStrategy(_: unknown[], newArray: unknown[]): unknown[] {
   return newArray
 }
 

packages/app/src/cli/services/app/write-app-configuration-file.ts

@@ -68,7 +68,7 @@ export const rewriteConfiguration = <T extends zod.ZodTypeAny>(schema: T, config
   return config
 }
 
-export function addDefaultCommentsToToml(fileString: string) {
+function addDefaultCommentsToToml(fileString: string) {
   const appTomlInitialComment = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration\n`
   const appTomlScopesComment = `\n# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes`