🚀 Feature: Update glob from 8.1.0 which is no longer supported

[NullPiotrException]

trafficstars

Feature Request Checklist

• [X] I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
• [X] I have searched for related issues and issues with the faq label, but none matched my issue.
• [ ] I want to provide a PR to resolve this

Overview

mocha uses [email protected] which is not supported and also depends on inflight which is also not supported (any version).

$ mkdir mochatest
$ cd mochatest
$ npm install mocha
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
...

I do realise it's been reported in #4988 but I'm not reporting it as security vulnerability but feature request. It would be nice not to have these warnings. Especially that a lot of packages depend on mocha (like appium from which I came).

Suggested Solution

Updating glob dependency to v9.0.0 or maybe even v10.4.1.

Alternatives

Leave glob v8.1.0 which is no longer supported and ignore warnings.

Additional Info

https://www.npmjs.com/package/inflight https://www.npmjs.com/package/glob/v/8.1.0

[JoshuaKGoldberg]

Agreed, this would be great. There's no reason to keep glob -or any other dependency- on an outdated major version - especially one that's now deprecated!

#5114 tracks allowing ^ (minor) version matching on packages. That's a semver-minor for us.

I suspect (but am not wholly convinced) for major version bumps we'll want to play it safe and wait until the next major version of Mocha. cc @mochajs/maintenance-crew: we'd previously talked about how we wanted to get to a new major version, but are also hesitant to make new majors given how entrenched the current v10 major version of Mocha is? For reference, we took that strategy in typescript-eslint recently: typescript-eslint@v7's breaking changes were around major versions, while typescript-eslint@v8's breaking changes are more substantial.

Marking as semver-major and in discussion just in case for now. We'll likely end up filing issues for other dependencies that are on out-of-date majors. The PRs linked in #5114 mention those.

[JoshuaKGoldberg]

We talked internally and will treat this as semver-minor. It's a dependency of the mocha package and not an actual exported part of the API. We'll tackle soon! 🚀

[JoshuaKGoldberg]

Oh. This is unfortunate: glob@9 and glob@10 have more restrictive ranges than our node >= 14:

• glob@9: >=14.17
• glob@10: >=14.18

...so this would be semver-major after all. Which we'd like to do, but need a bit of time before.

[voxpelli]

Apparently 14.18 because of node:path: https://github.com/isaacs/node-glob/commit/57f5551d92d8bc05a903bcc2601045e4f5b7fa5a

14.17 was set here: https://github.com/isaacs/node-glob/commit/03b9bcaa3b42b28f02139c27e15d1610ff9da4f0

Not sure why 14.17 was picked, other than that it was requested here: https://hachyderm.io/@lukekarrys/109932512524686599

[voxpelli]

Its used in two places in mocha:

https://github.com/mochajs/mocha/blob/e16d25db743e6023443a5b1d9f36ac0b26c2e72c/lib/cli/lookup-files.js#L78

And:

https://github.com/mochajs/mocha/blob/e16d25db743e6023443a5b1d9f36ac0b26c2e72c/lib/cli/lookup-files.js#L90-L93

[Mr0grog]

Worth noting here (since I just ran into it in an old package I was updating): glob requires Node.js >=18 as of v10.4.3 (yes, updated engine requirements in a patch release). This also happened in v10.3.1 of lru-cache, which is a transitive dependency of glob (by the same author).

In my case, I was able to handle it by adding a depending on "glob": "^9.3.5", "lru-cache": "<10.3.1" in my package.json, but I only needed to maintain compatibility with Node.js 16, which is not the case here. 😕

I am also a little concerned there may be some mislabeling and these version requirements are not actually compatible, but my tests all pass, at least. Also, glob’s author does not seem to think this is a problem, which tells me I should be a little more careful depending on his packages in the future.

[jakebailey]

Not sure if this is helpful, but tinyglobby (now a dep of vitest) is >=12.0.0. I don't think it has hasMagic, but I'm also not sure if it's needed or why.

[jakebailey]

I tried putting together a change that would work here, but:

• tinyglobby does not pass the windows flag down to picomatch, wheras mocha always calls glob with windowsPathsNoEscape
• The hasMagic function means "this pattern is only matched by a fixed set of strings", i.e. no *, [], negation, etc, but it is allowed to contain {}. I have not found a clean way to do that in picomatch's APIs.

Another popular alternative is fdir, but it does not actually set engines, and they only test Node 16+ in CI.

[NullPiotrException]

@jakebailey Thank you for the update. As for fdir I created an issue asking to set engines here. Will let you know if/when that changes. As for NodeJS 14, I think not supporting it is the right thing to do as it's no longer maintained and supporting it discourages devs from updating to a supported NodeJS version, which is inevitable anyway. So perhaps it's mocha that should bump Node's version in engines. I admit I never maintained such a widely used package as mocha, so I might not be aware of some problems with that bump. Just sharing my perspective.

[voxpelli]

@einsteinsfool We will bump to >=18 real soon as we prepare for a new major release, to avoid issues like these, so no worries on using a dependency that requires node >=18 in a PR (reference this comment if you do)

[voxpelli]

PR about the bump of engine in Mocha 11: https://github.com/mochajs/mocha/pull/5216

[jakebailey]

Bumping mocha's major to get an engine bump is definitely a way to solve the problem, though TS unfortunately still supports Node 14 so I'll just have to stare at the deprecation warning until TS can affort an engines bump for a while if so :\

[voxpelli]

@jakebailey Can you elaborate? If it's a problem that we drop Node 14 and 16, then we might have to reconsider #5216

[jakebailey]

TS uses mocha for its testing, TS currently supports Node 14.17+; if you bump the minimum version, we'd just have to stay on v10 (just like we're stuck on chai v4). I'm certainly not going to try and stop you from bumping to 18+; loads of projects are doing that. The only gotcha is just that while a dep like ESLint can go to v18 no problem (just don't lint on an old version of Node), test deps are less fun to deal with because a test framework dropping a version of Node means being unable to test on that version of Node at all. But again, it's absolutely reasonable to bump your minimum node version in v11.

My main interest in this issue is just to make npm install not complain so much, with the secondary goal of not having like 3 different version of the whole glob dependency chain 😄

(It's just that bumping major to fix this particular issue means not actually fixing it for those on v10, but, that's just the reality of fixes with a project moving forward. I don't want to stop you from making progress!)

[JoshuaKGoldberg]

@jakebailey I asked about TypeScript's plan for Node.js EOL version support in https://discord.com/channels/508357248330760243/640177505915109377/1293239882323591200 & noted it in https://github.com/mochajs/mocha/pull/5216/files#r1792135513.