Skip to content

Migrate all TOML I/O callsites to TomlFile #6944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryancbahan merged 1 commit into from
Mar 11, 2026
+599 −915
Merged

Migrate all TOML I/O callsites to TomlFile #6944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 11 additions & 25 deletions packages/app/src/cli/models/app/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {getOrCreateAppConfigHiddenPath} from '../../utilities/app/config/hidden-
import {ApplicationURLs, generateApplicationURLs} from '../../services/dev/urls.js'
import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
import {fileExists, readFile, glob, findPathUp, fileExistsSync} from '@shopify/cli-kit/node/fs'
import {TomlFile, TomlParseError} from '@shopify/cli-kit/node/toml/toml-file'
import {zod} from '@shopify/cli-kit/node/schema'
import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {
Expand All @@ -49,7 +50,7 @@ import {
} from '@shopify/cli-kit/node/node-package-manager'
import {resolveFramework} from '@shopify/cli-kit/node/framework'
import {hashString} from '@shopify/cli-kit/node/crypto'
import {JsonMapType, decodeToml} from '@shopify/cli-kit/node/toml'
import {JsonMapType} from '@shopify/cli-kit/node/toml'
import {joinPath, dirname, basename, relativePath, relativizePath} from '@shopify/cli-kit/node/path'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputContent, outputDebug, OutputMessage, outputToken} from '@shopify/cli-kit/node/output'
Expand Down Expand Up @@ -82,27 +83,19 @@ const noopAbortOrReport: AbortOrReport = (_errorMessage, fallback, _configuratio
export async function loadConfigurationFileContent(
filepath: string,
abortOrReport: AbortOrReport = abort,
decode: (input: string) => JsonMapType = decodeToml,
): Promise<JsonMapType> {
if (!(await fileExists(filepath))) {
return abortOrReport(outputContent`Couldn't find an app toml file at ${outputToken.path(filepath)}`, {}, filepath)
}

try {
const configurationContent = await readFile(filepath)
return decode(configurationContent)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (err: any) {
// TOML errors have line, pos and col properties
if (err.line !== undefined && err.pos !== undefined && err.col !== undefined) {
return abortOrReport(
outputContent`Fix the following error in ${outputToken.path(filepath)}:\n${err.message}`,
{},
filepath,
)
} else {
throw err
const file = await TomlFile.read(filepath)
return file.content
} catch (err) {
if (err instanceof TomlParseError) {
return abortOrReport(outputContent`${err.message}`, {}, filepath)
}
throw err
}
}

Expand All @@ -115,12 +108,11 @@ export async function parseConfigurationFile<TSchema extends zod.ZodType>(
schema: TSchema,
filepath: string,
abortOrReport: AbortOrReport = abort,
decode: (input: string) => JsonMapType = decodeToml,
preloadedContent?: JsonMapType,
): Promise<zod.TypeOf<TSchema> & {path: string}> {
const fallbackOutput = {} as zod.TypeOf<TSchema>

const configurationObject = preloadedContent ?? (await loadConfigurationFileContent(filepath, abortOrReport, decode))
const configurationObject = preloadedContent ?? (await loadConfigurationFileContent(filepath, abortOrReport))

if (!configurationObject) return fallbackOutput

Expand Down Expand Up @@ -517,13 +509,8 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
)
}

private parseConfigurationFile<TSchema extends zod.ZodType>(
schema: TSchema,
filepath: string,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
decode: (input: any) => any = decodeToml,
) {
return parseConfigurationFile(schema, filepath, this.abortOrReport.bind(this), decode)
private parseConfigurationFile<TSchema extends zod.ZodType>(schema: TSchema, filepath: string) {
return parseConfigurationFile(schema, filepath, this.abortOrReport.bind(this))
}

private validateWebs(webs: Web[]): void {
Expand Down Expand Up @@ -1027,7 +1014,6 @@ async function loadAppConfigurationFromState<
schemaForConfigurationFile,
configState.configurationPath,
abort,
decodeToml,
file,
)) as LoadedAppConfigFromConfigState<TConfig>
const allClientIdsByConfigName = await getAllLinkedConfigClientIds(configState.appDirectory, {
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/prompts/import-extensions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface MigrationChoice {
ext: ExtensionRegistration,
allExtensions: ExtensionRegistration[],
appConfiguration: CurrentAppConfiguration,
) => string
) => object
}

export const allMigrationChoices: MigrationChoice[] = [
Expand Down
104 changes: 64 additions & 40 deletions packages/app/src/cli/services/admin-link/extension-to-toml.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {ExtensionRegistration} from '../../api/graphql/all_app_extension_registr
import {describe, expect, test} from 'vitest'

describe('extension-to-toml', () => {
test('correctly builds a toml string for a app_link extension on a non embedded app', () => {
test('correctly builds a toml object for a app_link extension on a non embedded app', () => {
// Given
const appConfig = {
path: '',
Expand All @@ -28,18 +28,24 @@ describe('extension-to-toml', () => {
const got = buildTomlObject(extension1, [], appConfig)

// Then
expect(got).toEqual(`[[extensions]]
type = "admin_link"
name = "Admin link label"
handle = "admin-link-title"

[[extensions.targeting]]
url = "https://google.es"
target = "admin.collection-details.action.link"
`)
expect(got).toEqual({
extensions: [
{
type: 'admin_link',
name: 'Admin link label',
handle: 'admin-link-title',
targeting: [
{
url: 'https://google.es',
target: 'admin.collection-details.action.link',
},
],
},
],
})
})

test('correctly builds a toml string for bulk_action extension with path in an embedded app', () => {
test('correctly builds a toml object for bulk_action extension with path in an embedded app', () => {
// Given
const appConfig = {
path: '',
Expand All @@ -63,17 +69,23 @@ handle = "admin-link-title"
const got = buildTomlObject(extension1, [], appConfig)

// Then
expect(got).toEqual(`[[extensions]]
type = "admin_link"
name = "Bulk action label"
handle = "bulk-action-title"

[[extensions.targeting]]
url = "app://action/product?product_id=123#hash"
target = "admin.product-index.selection-action.link"
`)
expect(got).toEqual({
extensions: [
{
type: 'admin_link',
name: 'Bulk action label',
handle: 'bulk-action-title',
targeting: [
{
url: 'app://action/product?product_id=123#hash',
target: 'admin.product-index.selection-action.link',
},
],
},
],
})
})
test('correctly builds a toml string for bulk_action extension with no path in an embedded app', () => {
test('correctly builds a toml object for bulk_action extension with no path in an embedded app', () => {
// Given
const appConfig = {
path: '',
Expand All @@ -97,17 +109,23 @@ handle = "bulk-action-title"
const got = buildTomlObject(extension1, [], appConfig)

// Then
expect(got).toEqual(`[[extensions]]
type = "admin_link"
name = "Bulk action label"
handle = "bulk-action-title"

[[extensions.targeting]]
url = "app://"
target = "admin.product-index.selection-action.link"
`)
expect(got).toEqual({
extensions: [
{
type: 'admin_link',
name: 'Bulk action label',
handle: 'bulk-action-title',
targeting: [
{
url: 'app://',
target: 'admin.product-index.selection-action.link',
},
],
},
],
})
})
test('correctly builds a toml string for bulk_action extension with no path but search query in an embedded app', () => {
test('correctly builds a toml object for bulk_action extension with no path but search query in an embedded app', () => {
// Given
const appConfig = {
path: '',
Expand All @@ -131,14 +149,20 @@ handle = "bulk-action-title"
const got = buildTomlObject(extension1, [], appConfig)

// Then
expect(got).toEqual(`[[extensions]]
type = "admin_link"
name = "Bulk action label"
handle = "bulk-action-title"

[[extensions.targeting]]
url = "app://?foo=bar"
target = "admin.product-index.selection-action.link"
`)
expect(got).toEqual({
extensions: [
{
type: 'admin_link',
name: 'Bulk action label',
handle: 'bulk-action-title',
targeting: [
{
url: 'app://?foo=bar',
target: 'admin.product-index.selection-action.link',
},
],
},
],
})
})
})
6 changes: 2 additions & 4 deletions packages/app/src/cli/services/admin-link/extension-to-toml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {contextToTarget} from './utils.js'
import {ExtensionRegistration} from '../../api/graphql/all_app_extension_registrations.js'
import {MAX_EXTENSION_HANDLE_LENGTH} from '../../models/extensions/schemas.js'
import {CurrentAppConfiguration} from '../../models/app/app.js'
import {encodeToml} from '@shopify/cli-kit/node/toml'
import {slugify} from '@shopify/cli-kit/common/string'

interface AdminLinkConfig {
Expand All @@ -17,7 +16,7 @@ export function buildTomlObject(
extension: ExtensionRegistration,
_: ExtensionRegistration[],
appConfiguration: CurrentAppConfiguration,
): string {
): object {
const versionConfig = extension.activeVersion?.config ?? extension.draftVersion?.config
if (!versionConfig) throw new Error('No config found for extension')

Expand Down Expand Up @@ -55,6 +54,5 @@ export function buildTomlObject(
},
],
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return encodeToml(localExtensionRepresentation as any)
return localExtensionRepresentation
}
1 change: 0 additions & 1 deletion packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ describe('addUidToTomlsIfNecessary', () => {
// Then
const updatedContent = await readFile(tomlPath)
expect(updatedContent).toContain('uid = "123"')
expect(updatedContent).toMatch(/uid.*type/s)
})
})

Expand Down
35 changes: 17 additions & 18 deletions packages/app/src/cli/services/app/add-uid-to-extension-toml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {decodeToml} from '@shopify/cli-kit/node/toml'
import {readFile, writeFile} from '@shopify/cli-kit/node/fs'
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
import {getPathValue} from '@shopify/cli-kit/common/object'

export async function addUidToTomlsIfNecessary(
Expand All @@ -20,26 +19,26 @@ export async function addUidToTomlsIfNecessary(
async function addUidToToml(extension: ExtensionInstance) {
if (!extension.isUUIDStrategyExtension || extension.configuration.uid) return

const tomlContents = await readFile(extension.configurationPath)
const extensionConfig = decodeToml(tomlContents)
const extensions = getPathValue(extensionConfig, 'extensions') as ExtensionInstance[]
const file = await TomlFile.read(extension.configurationPath)
const extensionsArray = getPathValue(file.content, 'extensions') as ExtensionInstance[]

if ('uid' in extensionConfig) return
if (extensions) {
const currentExtension = extensions.find((ext) => ext.handle === extension.handle)
if ('uid' in file.content) return
if (extensionsArray) {
const currentExtension = extensionsArray.find((ext) => ext.handle === extension.handle)
if (currentExtension && 'uid' in currentExtension) return
}

let updatedTomlContents = tomlContents
if (extensions?.length > 1) {
// If the TOML has multiple extensions, we look for the correct handle to add the uid below
const regex = new RegExp(`(\\n?(\\s*)handle\\s*=\\s*"${extension.handle}")`)
updatedTomlContents = tomlContents.replace(regex, `$1\n$2uid = "${extension.uid}"`)
if (extensionsArray && extensionsArray.length > 1) {
// Multi-extension TOML: use regex to insert uid after the correct handle.
// updateTomlValues (WASM) doesn't support patching individual array-of-tables entries,
// so transformRaw with positional insertion is the pragmatic choice here.
const handle = extension.handle
await file.transformRaw((raw) => {
const regex = new RegExp(`(\\n?(\\s*)handle\\s*=\\s*"${handle}")`)
return raw.replace(regex, `$1\n$2uid = "${extension.uid}"`)
})
} else {
// If the TOML has only one extension, we add the uid before the type, which is always present
if ('uid' in extensionConfig) return
const regex = /\n?((\s*)type\s*=\s*"\S*")/
updatedTomlContents = tomlContents.replace(regex, `$2\nuid = "${extension.uid}"\n$1`)
// Single extension (or no extensions array): add uid at the top level via WASM patch
await file.patch({uid: extension.uid})
Copy link

@binks-code-reviewer binks-code-reviewer bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-extension path may write uid at top-level instead of inside [[extensions]]

Previously, the single-extension branch inserted uid before the first type = ... occurrence, which typically lives inside [[extensions]]. Now it does:

await file.patch({uid: extension.uid})

This patches uid at the top level rather than inside extensions[0]. Meanwhile, the multi-extension branch inserts uid next to the matching handle inside a specific [[extensions]] table, yielding inconsistent output shape depending on file structure. If downstream expects per-extension [[extensions]].uid, a top-level uid may be ignored or misinterpreted. Additionally, if ('uid' in file.content) return now treats a top-level uid as completed even if per-extension uid is missing.

}
await writeFile(extension.configurationPath, updatedTomlContents)
}
Loading
Loading