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

Update Node engine version in package.json to >=16.14 #133

Closed
hingew opened this issue Jul 6, 2023 · 6 comments
Closed

Update Node engine version in package.json to >=16.14 #133

hingew opened this issue Jul 6, 2023 · 6 comments

Comments

@hingew
Copy link

hingew commented Jul 6, 2023

Description

The minimal node version should be 16.14.0. In a ci pipline with a lower version we got the following error:

$ npm ci --cache .npm --prefer-offline
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'lru-cache@10.0.0',
npm WARN EBADENGINE   required: { node: '14 || >=16.14' },
npm WARN EBADENGINE   current: { node: 'v16.13.1', npm: '8.1.2' }
npm WARN EBADENGINE }

Dependency tree for lru-cache

-> npm ls lru-cache
├─┬ elm-review@2.10.2
│ └─┬ rimraf@5.0.1
│   └─┬ glob@10.3.1
│     └─┬ path-scurry@1.10.0
│       └── lru-cache@10.0.0

According the docs from lru-cache (https://github.com/isaacs/node-lru-cache#breaking-changes-in-version-8) the engine version should be 16.14.0 or higher.

Tnks for elm-review ❤️.

@lishaduck
Copy link
Contributor

"lru-cache": "^9.0.0",

https://github.com/isaacs/node-lru-cache/blob/v9.0.0/package.json#L80-L82

On my Windows machine (my main Elm machine), I only have 16.12, so I've just had to ignore it for (two?) years. Luckily, it works, but npm complains mightily. I never thought to check if it what caused it, otherwise I would have noted it sooner :)

Originally posted by @lishaduck in #147 (comment)

@lishaduck
Copy link
Contributor

lishaduck commented Jun 15, 2024

Sorry for all the noise, this issue can be marked as resolved.

Old issue

I did some more investigation:

  • lru-cache v10 re-adds support for Node 14, but, as @hingew noted, this path-scurry requires v9.
  • Latest path-scurry uses lru-cache v10, and latest glob uses latest path-scurry. So that all seems fine, what's pulling in glob?
  • It's rimraf, the latest of which supports engines >=14.18, so that too seems fine, and elm-review uses `^5.0.0 , so there shouldn't be an issue.

It looks like this got fixed sometime in the last year, so maybe we should just refresh our lockfiles and pray it gets fixed. It seems fine, so it's either stale or a bug in npm.

EDIT: I regenerated my lockfile, and while I still get this error, it's due to node-elm-compiler (through elm-optimize-level-2), elm-doc-preview, and elm-pages@2. Hurrah! elm-review for Node 14 (well, 16, but same difference)!

I might suggest regenerating your lockfile as well. It's quite out of date.

@jfmengels
Copy link
Owner

Sorry for the delay in replying to this issue. Yes, older Node.js support broke underneath my feet, even more with the last release (mostly because of glob) if I'm not mistaken. I think in the future we should add a tool in the test runner to verify that we didn't break support for a Node.js version we ought to support (such as https://www.npmjs.com/package/ls-engines)

@lishaduck I'm sorry, I'm not sure what you mean by "this issue can be marked as resolved.", do you mean this issue #133? Can support for v10/12 been restored somehow? 🤔

@hingew
Copy link
Author

hingew commented Jun 17, 2024

@jfmengels i can take a look at this.

@jfmengels
Copy link
Owner

Fixed by #157

@lishaduck
Copy link
Contributor

I meant that latest lru-cache supports >=14.
I'd forgotten that elm-review supports node 10 (despite having come here from that discussion 🤣)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants