Skip to content

Rename extension-to-toml → extension-config-builder#6945

Merged
ryancbahan merged 2 commits intomainfrom
rcb/toml-rename-builders
Mar 11, 2026
Merged

Rename extension-to-toml → extension-config-builder#6945
ryancbahan merged 2 commits intomainfrom
rcb/toml-rename-builders

Conversation

@ryancbahan
Copy link
Contributor

@ryancbahan ryancbahan commented Mar 6, 2026

Summary

Renames files and functions to reflect that extension builders now return config objects, not TOML strings:

  • extension-to-toml.tsextension-config-builder.ts (all 5 extension types)
  • buildTomlObject()buildExtensionConfig() across all callsites

Test plan

  • All affected tests pass

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 78.86% 14504/18391
🟡 Branches 73.18% 7198/9836
🟡 Functions 79.06% 3697/4676
🟡 Lines 79.19% 13704/17305

Test suite run success

3802 tests passing in 1453 suites.

Report generated by 🧪jest coverage report action from eefb2a9

@ryancbahan ryancbahan changed the title Rename extension-to-toml → extension-config-builder, buildTomlObject → buildExtensionConfig 3/3: Rename extension-to-toml → extension-config-builder Mar 6, 2026
@ryancbahan ryancbahan force-pushed the rcb/toml-rename-builders branch from b9ff79a to 86e7f0c Compare March 6, 2026 17:45
@ryancbahan ryancbahan force-pushed the rcb/toml-migrate-callsites branch from f9b265f to 7e9554f Compare March 6, 2026 17:45
@ryancbahan ryancbahan changed the title 3/3: Rename extension-to-toml → extension-config-builder Rename extension-to-toml → extension-config-builder Mar 6, 2026
@ryancbahan ryancbahan marked this pull request as ready for review March 6, 2026 18:03
@ryancbahan ryancbahan requested a review from a team as a code owner March 6, 2026 18:03
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@binks-code-reviewer
Copy link

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

Complete - No issues

📋 History

✅ No issues

Copy link
Contributor

@dmerand dmerand left a comment

Choose a reason for hiding this comment

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

This looks good! AI and I had the same nits about naming/conventions.


Review assisted by pair-review

Copy link
Contributor

Choose a reason for hiding this comment

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

Praise: The renaming from buildTomlObject to buildExtensionConfig has been applied consistently across function definitions, interface properties, and all callsites throughout the codebase. The new import aliases (buildPaymentsConfig, buildFlowConfig, etc.) are more concise and clearly convey that these functions build configuration objects. This is solid refactoring work.

Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 Code Style: Multiple test descriptions in this file state 'correctly builds a toml object' (lines 6, 48, 88, 128). The function being tested now builds and returns a configuration object, not TOML. The test descriptions should be updated to say 'correctly builds a config object' or 'correctly builds an extension config' to align with the rename from buildTomlObject to buildExtensionConfig.

Suggestion: Update all test descriptions from 'correctly builds a toml object for...' to 'correctly builds a config object for...' to maintain consistency with the refactoring's stated goal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mirroring existing behavior -- happy to change as separate pr, but I'm trying to reduce surface area given this is a large change. the syntax here is pre-existing pattern

@@ -31,7 +31,7 @@ interface FlowWebhookConfig {
* Given a flow extension config file, convert it to toml
Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 Code Style: The comment states 'convert it to toml' but this function returns a configuration object. The TOML conversion is handled by the caller via TomlFile, not by this function. The comment should align with the refactored function naming.

Suggestion:

Suggested change
* Given a flow extension config file, convert it to toml
* Given a flow extension config file, builds the extension configuration object

Copy link
Contributor

Choose a reason for hiding this comment

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

🎨 Code Style: Multiple test descriptions in this file state 'correctly builds a toml object' (lines 57, 153, 200, 255, 309, 367, 420). The function being tested returns a configuration object, not TOML. These descriptions should be updated to align with the renamed function and the PR's stated purpose.

Suggestion: Update all test descriptions from 'correctly builds a toml object for...' to 'correctly builds a config object for...' to maintain terminology consistency throughout the test suite.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Ref Line 107) 💡 Improvement: The internal helper function is still named buildPaymentsToml despite the exported function being renamed to buildExtensionConfig. For consistency with the PR's goal of reflecting that builders return config objects (not TOML), this helper should also be renamed. This will require updating all 6 callsites (lines 65, 71, 77, 83, 89, 95).

Suggestion:

Suggested change
function buildPaymentsConfig<T extends BasePaymentsAppExtensionDeployConfigType>(

@ryancbahan ryancbahan force-pushed the rcb/toml-migrate-callsites branch from 7749fae to 80d65bc Compare March 11, 2026 14:08
@ryancbahan ryancbahan force-pushed the rcb/toml-rename-builders branch from 5d78400 to 5b5210d Compare March 11, 2026 14:09
@github-actions
Copy link
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/public/node/toml/codec.d.ts
import { JsonMap } from '../../../private/common/json.js';
export type JsonMapType = JsonMap;
/**
 * Given a TOML string, it returns a JSON object.
 *
 * @param input - TOML string.
 * @returns JSON object.
 */
export declare function decodeToml(input: string): JsonMapType;
/**
 * Given a JSON object, it returns a TOML string.
 *
 * @param content - JSON object.
 * @returns TOML string.
 */
export declare function encodeToml(content: JsonMap | object): string;
packages/cli-kit/dist/public/node/toml/index.d.ts
export type { JsonMapType } from './codec.js';
packages/cli-kit/dist/public/node/toml/toml-file.d.ts
import { JsonMapType } from './codec.js';
/**
 * Thrown when a TOML file cannot be parsed. Includes the file path for context.
 */
export declare class TomlParseError extends Error {
    readonly path: string;
    constructor(path: string, cause: Error);
}
/**
 * General-purpose TOML file abstraction.
 *
 * Provides a unified interface for reading, patching, removing keys from, and replacing
 * the content of TOML files on disk.
 *
 * - `read` populates content from disk
 * - `patch` does surgical WASM-based edits (preserves comments and formatting)
 * - `remove` deletes a key by dotted path (preserves comments and formatting)
 * - `replace` does a full re-serialization (comments and formatting are NOT preserved).
 * - `transformRaw` applies a function to the raw TOML string on disk.
 */
export declare class TomlFile {
    /**
     * Read and parse a TOML file from disk. Throws if the file doesn't exist or contains invalid TOML.
     * Parse errors are wrapped in {@link TomlParseError} with the file path for context.
     *
     * @param path - Absolute path to the TOML file.
     * @returns A TomlFile instance with parsed content.
     */
    static read(path: string): Promise<TomlFile>;
    readonly path: string;
    content: JsonMapType;
    constructor(path: string, content: JsonMapType);
    /**
     * Surgically patch values in the TOML file, preserving comments and formatting.
     *
     * Accepts a nested object whose leaf values are set in the TOML. Intermediate tables are
     * created automatically. Setting a leaf to `undefined` removes it (use `remove()` for a
     * clearer API when deleting keys).
     *
     * @example
     * ```ts
     * await file.patch({build: {dev_store_url: 'my-store.myshopify.com'}})
     * await file.patch({application_url: 'https://example.com', auth: {redirect_urls: ['...']}})
     * ```
     */
    patch(changes: {
        [key: string]: unknown;
    }): Promise<void>;
    /**
     * Remove a key from the TOML file by dotted path, preserving comments and formatting.
     *
     * @param keyPath - Dotted key path to remove (e.g. 'build.include_config_on_deploy').
     * @example
     * ```ts
     * await file.remove('build.include_config_on_deploy')
     * ```
     */
    remove(keyPath: string): Promise<void>;
    /**
     * Replace the entire file content. The file is fully re-serialized — comments and formatting
     * are NOT preserved.
     *
     * @param content - The new content to write.
     * @example
     * ```ts
     * await file.replace({client_id: 'abc', name: 'My App'})
     * ```
     */
    replace(content: JsonMapType): Promise<void>;
    /**
     * Transform the raw TOML string on disk. Reads the file, applies the transform function
     * to the raw text, writes back, and re-parses to keep `content` in sync.
     *
     * Use this for text-level operations that can't be expressed as structured edits —
     * e.g. Injecting comments or positional insertion of keys in arrays-of-tables.
     * Subsequent `patch()` calls will preserve any comments added this way.
     *
     * @param transform - A function that receives the raw TOML string and returns the modified string.
     * @example
     * ```ts
     * await file.transformRaw((raw) => `# Header comment\n${raw}`)
     * ```
     */
    transformRaw(transform: (raw: string) => string): Promise<void>;
    private decode;
}

Existing type declarations

We found no diffs with existing type declarations

Base automatically changed from rcb/toml-migrate-callsites to main March 11, 2026 15:14
@ryancbahan ryancbahan added this pull request to the merge queue Mar 11, 2026
@ryancbahan ryancbahan removed this pull request from the merge queue due to a manual request Mar 11, 2026
ryancbahan and others added 2 commits March 11, 2026 09:23
…→ buildExtensionConfig

These functions now return config objects instead of TOML strings,
so the names should reflect that.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns test descriptions with the rename from buildTomlObject to
buildExtensionConfig — these functions return config objects, not TOML.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan force-pushed the rcb/toml-rename-builders branch from 5b5210d to eefb2a9 Compare March 11, 2026 15:23
@ryancbahan ryancbahan added this pull request to the merge queue Mar 11, 2026
Merged via the queue into main with commit f6e2e09 Mar 11, 2026
25 checks passed
@ryancbahan ryancbahan deleted the rcb/toml-rename-builders branch March 11, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants