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

Fix EBADENGINE error for Node.js 14.13.1 through 14.17.x #6729

Closed
bbrk24 opened this issue Mar 23, 2023 · 12 comments
Closed

Fix EBADENGINE error for Node.js 14.13.1 through 14.17.x #6729

bbrk24 opened this issue Mar 23, 2023 · 12 comments
Labels
status: needs discussion triage needs further discussion
Milestone

Comments

@bbrk24
Copy link

bbrk24 commented Mar 23, 2023

Describe the documentation issue

The package.json says it supports "node": "^14.13.1 || >=16.0.0". However, it also lists as a dependency "supports-hyperlinks": "^3.0.0", which requires "node": ">=14.18". Thus, attempting to install using versions of node older than 14.18 results in EBADENGINE.

What solution would you like to see?

There are three possible solutions:

  • Say that listing the lower bound as 14.13.1 was a mistake, and bump it to 14.18.0 or higher. I think this could be semver-patch.
  • Make supports-hyperlinks an optional dependency, or remove it altogether.
  • Revert to support-hyperlinks 2, which supports earlier versions of node.

I'm not familiar enough with the codebase to say which is the best idea.

@bbrk24
Copy link
Author

bbrk24 commented Mar 23, 2023

I didn't notice this at first, but I double-checked just now and that's not the only dependency that can trigger this: write-file-atomic 5 requires node ^14.17.0 || ^16.13.0 || >=18.0.0.

@ybiquitous
Copy link
Member

@bbrk24 Thanks for the report. Is a workaround of engine-strict=false reasonable?

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label Mar 24, 2023
@bbrk24
Copy link
Author

bbrk24 commented Mar 24, 2023

All that does is demote it from an error to a warning -- and, since it currently isn't specified, it's false by default anyways.

@ybiquitous
Copy link
Member

ybiquitous commented Mar 24, 2023

Do you know how to detect an inconsistency of engines.node? (for example, a tool for that purpose)

@bbrk24
Copy link
Author

bbrk24 commented Mar 24, 2023

No, I'm not aware of any automated tool, but also I haven't looked very hard. I discovered the issue with write-file-atomic by grepping package-lock.json (I was using node 14.17, so only supports-hyperlinks triggered EBADENGINE).

I decided to go back and double-check with the same method, and I found that none of the @csstools packages support node 17 -- they require ^14 || ^16 || >=18. I fully understand the need for such a tool.

@romainmenke
Copy link
Member

romainmenke commented Mar 26, 2023

I found that none of the @csstools packages support node 17

We can relax this if needed.

@bbrk24
Copy link
Author

bbrk24 commented Mar 26, 2023

I just created a simple tool to check this: https://github.com/bbrk24/engine-check. I haven't tested it super thoroughly yet but it seems to work so far.

Sample output:

@csstools/css-parser-algorithms@2.0.1 requires node "^14 || ^16 || >=18".
@csstools/css-tokenizer@2.1.0 requires node "^14 || ^16 || >=18".
@csstools/media-query-list-parser@2.0.1 requires node "^14 || ^16 || >=18".
@csstools/selector-specificity@2.2.0 requires node "^14 || ^16 || >=18".
supports-hyperlinks@3.0.0 requires node ">=14.18".
write-file-atomic@5.0.0 requires node "^14.17.0 || ^16.13.0 || >=18.0.0".

@ybiquitous
Copy link
Member

@bbrk24 Thanks for sharing your tool. I just confirmed it works on my local machine.

For now, the following way seems easier because changes between jamestalmage/supports-hyperlinks@v2.3.0...v3.0.0 do not seem to include significant things:

Revert to support-hyperlinks 2, which supports earlier versions of node.

But I still have a few questions:

  • Do we need to address engines.node strictly in the whole dependency tree?
  • Is not there a best practice or something in the Node.js community to address such a problem?
  • Will we need to check inconsistency all the time in the future?

By the way, note that Node.js v14 will become end-of-life on April 30 this year. So we will drop the support with the next Stylelint major version.

@bbrk24
Copy link
Author

bbrk24 commented Mar 27, 2023

Do we need to address engines.node strictly in the whole dependency tree?

As I mentioned earlier, I use node 14.17, so the others don't affect me directly. However, write-file-atomic does prevent using earlier minor versions of node 14.

I'm not as concerned about @csstools, since node 17 has already reached end of life.

Is not there a best practice or something in the Node.js community to address such a problem?
Will we need to check inconsistency all the time in the future?

I would like to believe there's an existing solution for this, but I'm not aware of one.

By the way, note that Node.js v14 will become end-of-life on April 30 this year. So we will drop the support with the next Stylelint major version.

Thanks for the heads-up. I'll have to change my configuration to use v16 or v18 by default instead of v14. As a side note, I'm not aware of any other language that goes through major versions as quickly as Node does.

@ybiquitous
Copy link
Member

It seems reasonable to wait until Node.js v14 EOL (about a month left) unless people complain about this inconsistency.

For future releases, we might need to consider any way to check engines.node consistency... 🤔

@ybiquitous
Copy link
Member

I just opened an issue to remove Node.js 14 support: #6740

@jeddy3 jeddy3 changed the title Using node 14.13.1 - 14.17.x results in EBADENGINE Fix EBADENGINE error for Node.js 14.13.1 through 14.17.x Jun 24, 2023
@Mouvedia Mouvedia added this to the future-major milestone Jul 22, 2023
@jeddy3
Copy link
Member

jeddy3 commented Aug 14, 2023

Closed by #7020

@jeddy3 jeddy3 closed this as completed Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

5 participants