Skip to content

Conversation

@shauns
Copy link
Contributor

@shauns shauns commented May 16, 2025

WHY are these changes introduced?

To improve validation of app configuration files by enforcing stricter checks on unsupported sections in app.toml files. See #5816

Removes the SHOPIFY_CLI_DISABLE_UNSUPPORTED_CONFIG_PROPERTY_CHECKS opt-out.

Copy link
Contributor Author

shauns commented May 16, 2025

@shauns shauns added the #gsd:37045 label May 16, 2025 — with Graphite App
@shauns shauns marked this pull request as ready for review May 16, 2025 12:24
@shauns shauns requested review from a team as code owners May 16, 2025 12:24
@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

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/archiver.d.ts
@@ -20,43 +20,4 @@ interface ZipOptions {
  * @param options - ZipOptions.
  */
 export declare function zip(options: ZipOptions): Promise<void>;
-export interface BrotliOptions {
-    /**
-     * The directory to compress.
-     */
-    inputDirectory: string;
-    /**
-     * The path where the compressed file will be saved.
-     */
-    outputPath: string;
-    /**
-     * An optional glob pattern to match files.
-     */
-    matchFilePattern?: string | string[];
-    /**
-     * Brotli compression level (0-11, default: 11).
-     */
-    level?: number;
-}
-/**
- * Options for decompressing a Brotli compressed tar archive.
- */
-export interface DecompressionOptions {
-    /**
-     * Path to the compressed file.
-     */
-    inputFile: string;
-    /**
-     * Directory where files should be extracted.
-     */
-    outputDirectory: string;
-}
-/**
- * It compresses a directory with Brotli.
- * First creates a tar archive to preserve directory structure,
- * then compresses it with Brotli.
- *
- * @param options - BrotliOptions.
- */
-export declare function brotliCompress(options: BrotliOptions): Promise<void>;
 export {};
\ No newline at end of file
packages/cli-kit/dist/public/node/fs.d.ts
@@ -323,10 +323,4 @@ export interface MatchGlobOptions {
  * @returns true if the key matches the pattern, false otherwise.
  */
 export declare function matchGlob(key: string, pattern: string, options?: MatchGlobOptions): boolean;
-/**
- * Read a directory.
- * @param path - The path to read.
- * @returns A promise that resolves to an array of file names.
- */
-export declare function readdir(path: string): Promise<string[]>;
 export {};
\ No newline at end of file
packages/cli-kit/dist/public/node/git.d.ts
@@ -129,11 +129,4 @@ export declare function ensureIsClean(directory?: string): Promise<void>;
  * @param directory - The directory to check.
  * @returns True is the .git directory is clean.
  */
-export declare function isClean(directory?: string): Promise<boolean>;
-/**
- * Returns the latest tag of a git repository.
- *
- * @param directory - The directory to check.
- * @returns String with the latest tag or undefined if no tags are found.
- */
-export declare function getLatestTag(directory?: string): Promise<string | undefined>;
\ No newline at end of file
+export declare function isClean(directory?: string): Promise<boolean>;
\ No newline at end of file

@github-actions
Copy link
Contributor

github-actions bot commented May 16, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.34% (+0.43% 🔼)
12558/16031
🟡 Branches
72.49% (+0.42% 🔼)
6113/8433
🟡 Functions
78.45% (+0.3% 🔼)
3277/4177
🟡 Lines
78.78% (+0.43% 🔼)
11885/15086
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / provision_shop_access.ts
100% 100% 100% 100%
🟢
... / card_present_payments_app_extension_schema.ts
100% 100% 100% 100%
🟡
... / output.ts
77.78% 66.67% 75% 75%
🟢
... / serial-batch-processor.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app.test-data.ts
92.04% (-0.46% 🔻)
93.75%
82.56% (-0.97% 🔻)
91.44% (-0.49% 🔻)
🟢
... / loader.ts
93.27% (-0.1% 🔻)
85.64% (-0.14% 🔻)
97.2% (-0.03% 🔻)
94.29% (-0.09% 🔻)
🔴
... / payments_app_extension.ts
50% (-4.55% 🔻)
14.29% (-2.38% 🔻)
100%
50% (-4.55% 🔻)
🔴
... / theme.ts
52.27% (-1.89% 🔻)
20% (-15.71% 🔻)
50%
54.76% (-2.06% 🔻)
🟢
... / ui_extension.ts
96.08% (-0.86% 🔻)
88.41% (-0.82% 🔻)
100%
97.96% (-0.98% 🔻)
🔴
... / dev.ts
11.57% (-7.09% 🔻)
12.33% (-1.37% 🔻)
12.12% (-12.88% 🔻)
12.39% (-6.81% 🔻)
🟢
... / fetch.ts
83.33% (-1.11% 🔻)
83.33% (+8.33% 🔼)
75% (-2.78% 🔻)
83.87% (-1.13% 🔻)
🟢
... / handlers.ts
91.94% (-5.36% 🔻)
67.86% (+1.19% 🔼)
89.47% (-10.53% 🔻)
91.8% (-5.42% 🔻)
🟢
... / Dev.tsx
90.59% (-2.35% 🔻)
75% (-3.57% 🔻)
86.36% (-4.55% 🔻)
92.5% (-1.25% 🔻)
🔴
... / partners-client.ts
25.79% (-0.36% 🔻)
30% (-1.58% 🔻)
18.03% (+1.08% 🔼)
25.49% (-0.36% 🔻)
🟢
... / ui.tsx
96.15% (-0.27% 🔻)
93.75% (-0.99% 🔻)
88.89%
96% (-0.15% 🔻)
🟢
... / git.ts
91.92% (+0.91% 🔼)
88% (-1.36% 🔻)
90.48% (+1% 🔼)
92.71% (+0.75% 🔼)
🔴
... / rest-api-throttler.ts
38.6% (-23.77% 🔻)
50%
20% (-26.67% 🔻)
38.6% (-23.77% 🔻)
🟢
... / pull.ts
96.3% (-0.13% 🔻)
69.7% 100%
96.15% (-0.14% 🔻)
🟢
... / theme-fs.ts
81.14% (-1.42% 🔻)
63.27% (-2.69% 🔻)
71.43% (-1.49% 🔻)
83.54% (-1.03% 🔻)
🟡
... / ExtensionServerProvider.tsx
66.67% (-19.05% 🔻)
0%
46.15% (-30.77% 🔻)
68.75% (-25% 🔻)

Test suite run success

2911 tests passing in 1261 suites.

Report generated by 🧪jest coverage report action from e66b6e3

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Ok, now I'm totally confused 😂

Why adding two flags and then removing them in two consecutive PRs? What's the plan?

@shauns shauns force-pushed the shauns/05-16-remove_opt-out_for_improve_config_module_loading branch from 519731c to d9b7e60 Compare May 20, 2025 11:29
@shauns shauns force-pushed the shauns/05-16-use_improved_config_module_loading_by_default branch from 52d1b32 to 2687ba6 Compare May 20, 2025 11:29
@github-actions
Copy link
Contributor

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.
→ If there's no activity within a week, then a bot will automatically close this.
Thanks for helping to improve Shopify's dev tooling and experience.

Copy link
Contributor

I guess the plan is to release the previous PR and then wait 1-2 releases before merging this one :)

@shauns shauns force-pushed the shauns/05-16-use_improved_config_module_loading_by_default branch from 2687ba6 to d691457 Compare June 25, 2025 10:06
@shauns shauns force-pushed the shauns/05-16-remove_opt-out_for_improve_config_module_loading branch from d9b7e60 to e66b6e3 Compare June 25, 2025 10:06
Copy link
Contributor Author

shauns commented Jun 25, 2025

That's right - let 5817 ship and then this can follow up in a subsequent release. This is the PR that fully migrates and removes The Old Way.

Base automatically changed from shauns/05-16-use_improved_config_module_loading_by_default to main June 25, 2025 10:39
@gonzaloriestra
Copy link
Contributor

According to this plan, we can now merge this.

@gonzaloriestra gonzaloriestra added this pull request to the merge queue Jul 16, 2025
Copy link
Contributor Author

shauns commented Jul 16, 2025

Yes 👍

Merged via the queue into main with commit a8f4c35 Jul 16, 2025
1 check passed
@gonzaloriestra gonzaloriestra deleted the shauns/05-16-remove_opt-out_for_improve_config_module_loading branch July 16, 2025 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants