Skip to content
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.

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

Breaking changes in v3.3.0 #405

Open
jaydenseric opened this issue Jul 11, 2023 · 1 comment
Open

Breaking changes in v3.3.0 #405

jaydenseric opened this issue Jul 11, 2023 · 1 comment

Comments

@jaydenseric
Copy link

With v3.2.12 the glob **/{!(*.d).mts,!(*.d).cts,*.{mjs,cjs,js}} used to select these files:

[
  ".cjs",
  ".cts",
  ".js",
  ".mjs",
  ".mts",
  "a.mjs",
  "ad.cts",
  "ad.mts",
  "a/.cjs",
  "a/.cts",
  "a/.js",
  "a/.mjs",
  "a/.mts",
  "a/a.mjs",
  "a/ad.cts",
  "a/ad.mts",
]

But now with v3.3.0 it only selects these files:

[
  ".cts",
  ".mts",
  "a.mjs",
  "ad.cts",
  "ad.mts",
  "a/.cts",
  "a/.mts",
  "a/a.mjs",
  "a/ad.cts",
  "a/ad.mts",
]

This is breaking this test assertion in find-unused-exports, which uses fast-glob indirectly as a dependency of a dependency (globby):

https://github.com/jaydenseric/find-unused-exports/blob/dd74e679e819c84e606d683f79d9ada1181a20b9/MODULE_GLOB.test.mjs#L15-L41

When using a package.json field overrides I was able to determine that the specific cause for the changed behavior of the glob was the fast-glob update from v3.2.12 to v3.3.0:

  {
    "overrides": {
-     "fast-glob": "3.2.12"
+     "fast-glob": "3.3.0"
    }
  }

It took me a lot of work to figure out the glob originally, and I'm not really sure what I can do in find-unused-exports to workaround this unexpected issue. I can't pin a particular version of fast-glob that still works because it's a dependency of a dependency (globby). It would be fantastic if this could be resolved in a speedy fast-glob patch release, assuming I'm not doing something blatantly incorrect with in my glob I could update to a more correct version.

Environment

  • OS Version: macOS v13.4
  • Node.js Version: v20.4.0
@mrmlnc mrmlnc added this to the 3.3.1 milestone Jul 14, 2023
@mrmlnc
Copy link
Owner

mrmlnc commented Jul 14, 2023

It looks like both behaviours are incorrect.

[
  ".cjs", // matched by **/*.cjs when the dot option is true
  ".cts", // matched by **/!(*.d).cts when the dot option is true
  ".js", // matched by **/*.js when the dot option is true
  ".mjs", // matched by **/*.mjs when the dot option is true
  ".mts", // matched by **/!(*.d).mts when the dot option is true
  "a.mjs", // matched by **/*.mjs
  "ad.cts", // matched by **/*.cjs
  "ad.mts", // matched by **/!(*.d).mts
  "a/.cjs", // matched by **/*.cjs when the dot option is true
  "a/.cts", // matched by **/!(*.d).cts when the dot option is true
  "a/.js", // matched by **/*.js when the dot option is true
  "a/.mjs", // matched by **/*.mjs when the dot option is true
  "a/.mts", // matched by **/!(*.d).mts when the dot option is true
  "a/a.mjs", // matched by **/*.mjs
  "a/ad.cts", // matched by **/!(*.d).cts
  "a/ad.mts", // matched by **/!(*.d).mts
]

Correct result:

// echo **/{!(*.d).mts,!(*.d).cts,*.{mjs,cjs,js}}

[
  "a.mjs",
  "a/a.mjs",
  "a/ad.cts",
  "a/ad.mts",
  "ad.cts",
  "ad.mts"
]

In the new version we are closer to the correct result, but it is still incorrect.

Probably related issues:

Yeap… the root issue is here:

https://github.com/micromatch/picomatch/blob/13efdf0c7d0e07ee30bf78d0079184aa4a0ec7d4/lib/parse.js#L258

Here the rest variable contains .mts and then the parser returns \\.mts for the **/!(*.d).mts.

@mrmlnc mrmlnc removed this from the 3.3.1 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants