Document breaking changes for glob@8

[s100]

It looks like glob@8 has been released, congratulations! We would love to upgrade, but before that, would it be possible to document the changes, particularly the breaking changes, for this release, in the change log? Thank you in advance.

[j0k3r]

Looks like it's only Node >= 12
3d6c4cd

[sircharlo]

Looks like it's only Node >= 12 3d6c4cd

More than that, I believe... v7.2.1...v8.0.1

[XhmikosR]

Yeah, and also isaacs/minimatch@v4.2.1...v5.0.1#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

[isaacs]

The minimatch 5 breaking change is not a breaking change for glob, because it's always coerced all slashes to /. It's only a breaking change if you're using minimatch directly. (And if you're relying on glob's minimatch behaving a particular way, and don't have a dependency on it yourself, well... that's not wise.)

But sure, I'll add a changelog.

[sircharlo]

Weird, my project completely broke with version 8. I must have misunderstood something.

[ath0mas]

I also had to change my patterns sent to glob when updating from v7.2 to v8.0, to not get empty array instead of correct list results.

On Windows, and mixing or joining path values with "child" glob patterns, it works quite fine directly with v7.2, while with v8.0 you are required to replace backslashes with forward-slashes (basic replace, or like using normalize-path or unixify packages).

• does it come from the minimatch v3 to v5 update? (or v3.0 to v3.1 in unpublished 7.2.1 yet?)
  if so, it should be added to the changelog.md I think
• even if the README mentions the "backslashes vs. forward-slashes" it in its Windows section, maybe it should be more explicit about how to manipulate path values and meet the expected glob pattern "format" and/or the "tools" to help do so. add a warning in the changelog telling it may have worked before but it is not the case anymore, requiring to review and possibly update your patterns.

Sorry for the ref, but fast-glob helped me understand with its

Use the normalize-path or the unixify package to convert Windows-style path to a Unix-style path.

[sircharlo]

I'm trying upath as we speak to see if that would work in my case. For sure a heads up would've been nice, as it worked before and is broken now for "\\" (read: non-UNIX) style paths.

[sircharlo]

I'm trying upath as we speak to see if that would work in my case. For sure a heads up would've been nice, as it worked before and is broken now for "\" (read: non-UNIX) style paths.

Confirmed, using upath as a drop-in replacement for path works great along with glob.

[larshp]

Also breaks stuff for me, probably due to windows paths as noted above.

[isaacs]

This caveat has been in the readme for a a decade now (added in 386f7e8). But if someone has a repro case showing something that worked in v7 and doesn't work in v8, I'd be happy to take a look.

[larshp]

thanks

suggest adding a note about the caveat in the changelog, as the behavior changes from 7.2.0 to 8.0.0? My issue occurs in https://github.com/abaplint/abaplint/blob/main/packages/cli/src/file_operations.ts, however I have not taken the caveat into account, and also not debugged it, so its probably an issue in my library.

[domoritz]

The update broke widows support for me. Here is a repro case: vega/ts-json-schema-generator#1221.

[icebob]

Same issue here. On Windows, version 8.x.x doesn't find files in folder that works on 7.2.0

[juergba]

Upgrade to v8.0.1 breaks Mocha as well.

[nicolas377]

Using backslashes was never explicitly supported by glob, see the windows portion of the README. My current solution is to resolve paths and then normalize them to forward slashes. See this example:

// assuming the pattern is stored in string variable pattern, and that node's path module has been imported
const normalizedPattern = path.resolve(pattern).split(path.sep).join("/");
glob.sync(normalizedPattern);

The workaround will most likely change soon, assuming the implementation of #468 lands.

[JustinBeckwith]

Not to pile on, but this broke linkinator as well:
JustinBeckwith/linkinator#389

Very specifically it changed something with paths on windows. I'll dig in and try to get a more minimal repo.

[isaacs]

Just to repeat what was already said above:

• This is a change that occurred in a semver major. It should not have broken anyone unexpectedly, unless you are depending on * or >= or something foolish like that.
• It is dropping support for a use case that has not been supported, explicitly in the readme, for 10 years.
• It fixed the use of escape characters on Windows platforms (which was broken before).

A v8 minor will come with the ability to explicitly choose to use \ as a path separator and not an escape character in #468.

Since cwd and root are always paths, and not patterns, we can add /-coercion to those automatically.

[JustinBeckwith]

Apologies, I used "broke" a tad flippantly :) The PR to update the dependency caused CI to fail in a way that wasn't obvious in the changelog. Thanks for the detailed explanations!

[isaacs]

@JustinBeckwith No worries! I definitely knew what you meant. More just trying to forestall the next dozen comments complaining about glob breaking their programs 😅

[SimenB]

More just trying to forestall the next dozen comments complaining about glob breaking their programs

I'd just like to say that's the most relatable comment I've ever seen on GitHub! 😅

Also - I appreciate the work you've done for the community, node and npm through the years. A major version breaking assumptions is not a huge issue.

Thank you! ♥️

[s100]

• This is a change that occurred in a semver major. It should not have broken anyone unexpectedly, unless you are depending on * or >= or something foolish like that.
• It is dropping support for a use case that has not been supported, explicitly in the readme, for 10 years.
• It fixed the use of escape characters on Windows platforms (which was broken before).

Would it be possible to document these changes in the change log?

[ronenteva]

Just to repeat what was already said above:

• This is a change that occurred in a semver major. It should not have broken anyone unexpectedly, unless you are depending on * or >= or something foolish like that.
• It is dropping support for a use case that has not been supported, explicitly in the readme, for 10 years.
• It fixed the use of escape characters on Windows platforms (which was broken before).

A v8 minor will come with the ability to explicitly choose to use \ as a path separator and not an escape character in #468.

Since cwd and root are always paths, and not patterns, we can add /-coercion to those automatically.

You are 100% right, but why not mention it in the changelog?

[swrdfish]

Same issue came up on Windows for version 7.2.2 also. Was working fine with 7.2.0

[nicolas377]

please upgrade to 7.2.3

[swrdfish]

@nicolas377 thanks! That worked.