Skip to content

Extract path from AppConfiguration into App.configPath#6949

Open
ryancbahan wants to merge 2 commits intomainfrom
rcb/config-model-extract-path
Open

Extract path from AppConfiguration into App.configPath#6949
ryancbahan wants to merge 2 commits intomainfrom
rcb/config-model-extract-path

Conversation

@ryancbahan
Copy link
Contributor

@ryancbahan ryancbahan commented Mar 6, 2026

WHY are these changes introduced?

Some light cleanup to make maintenance and coming refactors simpler. Today, AppConfiguration carries a path field that was never part of any TOML schema. It was added by parseConfigurationFile and then worked around in many places: Omit<..., 'path'> on schema types, delete file.path before re-validation, destructuring to strip it in type guards, a blocklist to prevent writing it back to disk.

This PR establishes a cleaner separation: AppConfiguration describes only what's in the TOML file. The file path is metadata about the app, not part of its configuration.

WHAT is this pull request doing?

Moves path from the configuration type to the app model:

  • App.configPath: string — path is metadata about the app, not part of its config (same pattern ExtensionInstance uses with configurationPath)
  • AppConfiguration — removes & {path: string} intersection from all 3 config types
  • AppConfigurationWithoutPath — deleted (no longer needed), 8 usages replaced with AppConfiguration
  • SchemaForConfig — simplified to ZodObjectOf<TConfig> (no more Omit)
  • parseConfigurationFile — no longer appends {path: filepath} to return value
  • writeAppConfigurationFile — takes explicit configPath parameter
  • Type guards — pass objects directly instead of destructuring out path
  • ~24 callsites migrated from app.configuration.pathapp.configPath
  • 10 workarounds removed (delete, Omit, destructure, blocklist)

Behaviorally, this is a no-op.

How to test your changes?

cd packages/app && npx tsc --noEmit   # zero type errors
npx vitest run packages/app           # all tests pass

Config pipeline snapshot tests confirm no change in TOML output.

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.25% 14570/18861
🟡 Branches 70.9% 7231/10199
🟡 Functions 76.21% 3701/4856
🟡 Lines 78.73% 13771/17491

Test suite run success

3806 tests passing in 1462 suites.

Report generated by 🧪jest coverage report action from d965024

@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

@ryancbahan ryancbahan force-pushed the rcb/config-model-snapshot-tests branch from 8ae41eb to a29ba87 Compare March 11, 2026 22:13
@ryancbahan ryancbahan force-pushed the rcb/config-model-extract-path branch from 5028d72 to 49d9789 Compare March 11, 2026 22:13
@ryancbahan ryancbahan force-pushed the rcb/config-model-snapshot-tests branch from a29ba87 to dab1a61 Compare March 12, 2026 14:38
@ryancbahan ryancbahan force-pushed the rcb/config-model-extract-path branch from 49d9789 to 29d8b4d Compare March 12, 2026 14:53
@ryancbahan ryancbahan force-pushed the rcb/config-model-snapshot-tests branch from dab1a61 to 2378a7f Compare March 12, 2026 14:53
Base automatically changed from rcb/config-model-snapshot-tests to main March 12, 2026 15:55
Path is not data — it's metadata about where the config file lives,
not part of what the TOML file contains. This is the first step toward
making AppConfiguration a trustworthy, file-shaped type.

- Add `configPath: string` to App, AppInterface, AppConfigurationInterface
- Remove `& {path: string}` from AppConfiguration, BasicAppConfigurationWithoutModules, LegacyAppConfiguration
- Delete AppConfigurationWithoutPath (replaced by AppConfiguration)
- Simplify SchemaForConfig (no more Omit<..., 'path'>)
- parseConfigurationFile no longer bolts path onto return value
- writeAppConfigurationFile takes explicit configPath parameter
- Type guards pass objects directly (no destructuring to strip path)
- Migrate ~24 callsites from app.configuration.path → app.configPath
- Remove 10 workarounds (delete file.path, blocklist, Omit wrappers)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

# Conflicts:
#	packages/app/src/cli/models/app/app.test-data.ts
#	packages/app/src/cli/models/app/app.test.ts
#	packages/app/src/cli/models/app/app.ts
#	packages/app/src/cli/models/app/loader.test.ts
#	packages/app/src/cli/models/app/loader.ts
#	packages/app/src/cli/services/app-context.test.ts
#	packages/app/src/cli/services/app/config/link.test.ts
#	packages/app/src/cli/services/app/config/use.test.ts
#	packages/app/src/cli/services/app/env/pull.test.ts
#	packages/app/src/cli/services/app/env/show.test.ts
#	packages/app/src/cli/services/context.ts
#	packages/app/src/cli/services/context/identifiers-extensions.test.ts
#	packages/app/src/cli/services/dev.ts
#	packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts
#	packages/app/src/cli/services/dev/app-events/file-watcher.test.ts
#	packages/app/src/cli/services/dev/select-app.test.ts
#	packages/app/src/cli/services/info.test.ts
#	packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts
@ryancbahan ryancbahan force-pushed the rcb/config-model-extract-path branch from 29d8b4d to 0af7623 Compare March 13, 2026 18:29
@ryancbahan ryancbahan marked this pull request as ready for review March 13, 2026 18:31
@ryancbahan ryancbahan requested a review from a team as a code owner March 13, 2026 18:31
@github-actions
Copy link
Contributor

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.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan force-pushed the rcb/config-model-extract-path branch from 0af7623 to d965024 Compare March 13, 2026 18:34
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.

1 participant