Update the asset ignore module to be backward-compatible

[karreiro]

WHY are these changes introduced?

Fixes https://github.com/Shopify/cli/issues/4496

WHAT is this pull request doing?

This PR updates the ignore module (--only/--ignore/.shopifyignore) to be backward compatible with (Ruby) Shopify CLI 2. The infrastructure code is no longer opinionated about minimatch defaults, and the themes module passes matchBase=true and noglobstar=true.

How to test your changes?

• Run shopify theme init test
• Run cd test
• Create .shopifyignore file with templates/**/*.json
• Run shopify theme push -d --verbose
• Run shopify theme push --legacy --verbose
• Notice the ignored files matches now

Notice: in the original report the user reports this issue using the --password flag instead of --legacy. Both flags activate the legacy implementation, but --legacy is more straightforward because it doesn't require any password. The reason the --password flag uses the legacy implementation is that this flag is designed to be used in CI/CD environments, so we decided to keep running the legacy implementation there in the first release of commands.

Post-release steps

N/A

Measuring impact

How do we know this change was effective? Please choose one:

• [ ] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
• [x] Existing analytics will cater for this addition
• [ ] PR includes analytics changes to measure impact

Checklist

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

[github-actions[bot]]

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

• Themes: @shopify/advanced-edits
• UI extensions: @shopify/ui-extensions-cli
  • Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
• Hydrogen: @shopify/hydrogen
• Other: @shopify/app-inner-loop

[github-actions[bot]]

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/fs.d.ts

@@ -283,6 +283,7 @@ export declare function pathToFileURL(path: string): URL;
 export declare function findPathUp(matcher: OverloadParameters<typeof internalFindUp>[0], options: OverloadParameters<typeof internalFindUp>[1]): ReturnType<typeof internalFindUp>;
 export interface MatchGlobOptions {
     matchBase: boolean;
+    noglobstar: boolean;
 }
 /**
  * Matches a key against a glob pattern.

[github-actions[bot]]

Coverage report

St.:grey_question:	Category	Percentage	Covered / Total
🟡	Statements	72.61% (+0.01% 🔼)	8522/11737
🟡	Branches	69.55% (+0.03% 🔼)	4178/6007
🟡	Functions	71.61%	2205/3079
🟡	Lines	72.92% (+0.01% 🔼)	8064/11058

Test suite run success

1939 tests passing in 873 suites.

Report generated by 🧪jest coverage report action from d9fff2c30ce1b0fcc32afab8b743a85f43567053

[jamesmengo]

I think this behaves differently when there are > 1 levels such as templates/customers/test/file.liquid.

Not a valid directory structure in today's world, though maybe one day it will be

[rkumorek]

Hey @karreiro , I have a question in regards to this change.

In 3.68.1 templates/**/*.json only ignores nested files (customers) which caught me off guard when I upgraded.

If I add templates/*.json it ignores all .json templates but shows the backwards compatibility rewrite message.

This is quite confusing as it now does the opposite to previous versions. Is this expected?

Thanks.

PS: Wasn't sure if I should create issue for this.

[karreiro]

👋 Hey @rkumorek,

Thanks for sharing that context. I've answered your question here because I've noticed that other folks had a similar concern.

Wasn't sure if I should create issue for this.

Please feel free to always create an issue. They are really, really welcome and help us make the CLI better.