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

Overly strict node versions specified in engine #774

Closed
Tracked by #642
macsj200 opened this issue Aug 5, 2021 · 18 comments
Closed
Tracked by #642

Overly strict node versions specified in engine #774

macsj200 opened this issue Aug 5, 2021 · 18 comments

Comments

@macsj200
Copy link

macsj200 commented Aug 5, 2021

I'm using node 14.2.0 and encountering the following error:

The engine "node" is incompatible with this module. Expected version "^12.20 || ^14.14.0 || ^16". Got "14.2.0"

I would be surprised if this package only works on 14.14.0. Is there a way we can specify a more permissive node version restriction?

Something like 14.x would probably do the trick.

@brettz9
Copy link
Collaborator

brettz9 commented Aug 5, 2021

At one point, comment-parser only made certain utilities available within submodules. The engines for comment-parser were bumped as a result of the form of its exports not being compatible with earlier Node and we followed suit here. Earlier Node had a problem resolving exports like this:

    "./parser/*": {
      "import": "./es6/parser/*.js",
      "require": "./lib/parser/*.cjs"
    },

However, comment-parser has since made available the utilities which we need in this project (and in @es-joy/jsdoccomment on which we depend) as part of its main export, and since we are already accessing the utilities from the main export, we could now revert to using the lower Node requirements.

However, so long as comment-parser is still using the higher engines threshold, it shouldn't be helpful (or technically accurate) for this plugin to do so. I'm not sure whether he'll want to revert since some utilities may still only be accessible through submodules.

@foray1010
Copy link

@brettz9 comment-parser has reverted to >=12.0.0 in v1.2.4, think we are ok to revert now?
https://github.com/syavorsky/comment-parser/blob/master/CHANGELOG.md#v124

@gajus
Copy link
Owner

gajus commented Aug 25, 2021

🎉 This issue has been resolved in version 36.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Aug 25, 2021
@amed
Copy link

amed commented Oct 6, 2021

The engine "node" is incompatible with this module. Expected version "^12 || ^14 || ^16". Got "15.14.0"

What about 15.x?

@sauloquirino
Copy link

The engine "node" is incompatible with this module. Expected version "^12 || ^14 || ^16". Got "17.0.0"

@elektronik2k5
Copy link

So I encountered this artificial roadblock as well, with node 17. I don't understand why on Earth would a linter configuration care about the runtime. 🤷🏻‍♂️ What exact problem does this solve?

Anyways, I made a fork just to overcome this issue until it is resolved. Here's the required change to use it:

-    "eslint-plugin-jsdoc": "^36.1.1",
+    "eslint-plugin-jsdoc": "elektronik2k5/eslint-plugin-jsdoc#patch-1",

Cheers! 🍻

@brettz9
Copy link
Collaborator

brettz9 commented Oct 22, 2021

@amed : Node 15 is no longer in LTS .

@elektronik2k5 : The problem it solves is that it communicates that the linter has been tested on the given version, and it prompts us to ensure our CI is checking the given engine.

Planning to allow Node 17 as a part of #792 .

@brettz9
Copy link
Collaborator

brettz9 commented Oct 22, 2021

Also note that the engines restriction should only be enforced if the user has opted into the engine-strict flag; see https://docs.npmjs.com/cli/v7/configuring-npm/package-json

@elektronik2k5
Copy link

@brettz9 that's the thing: it doesn't only communicate, but also enforces it, unnecessarily.

I use yarn for many reasons, one of them is better defaults. Specifically, unlike npm, it enforces engines by default - which in my opinion is a good thing. Likewise, when I'm forced to use npm, I always set the non default engine-strict=true flag on the project level via .npmrc. In such a configuration, eslint-config-jsdoc fails to install in any "non supported" version of node, unnecessarily.

In my opinion, there are two completely different concerns here:

  1. eslint-config-jsdoc should be tested in the versions of node which the project maintainers choose to support. That's possible in whichever CI vendor the project uses. This policy is meant for the maintainers of the project and has nothing to do with users.
  2. eslint-config-jsdoc should be installable and run in any compatible runtime. In this sense, JS aims to be a forward compatible language, where one of the language's guiding design principles is backwards compatibility ("don't break the web" and all that). Node of course is much more than just a JS runtime and has semver for good reasons, but I argue that for a linter configuration the chances of this package breaking as a result of a node upgrade are extremely low.

As such, the current use of the engines restriction is misused to enforce an overly strict policy for the wrong audience.

I suggest to follow the practice most node package maintainers have: a minimum version only. This way, users can install and use eslint-config-jsdoc on any technically compatible past, present or future version of node (starting from the earliest version which is capable of running the shipped JS).

In addition, in order to not deal with non supported versions, I suggest to declare (in the README and in the GitHub issue template) which versions of node the project supports and is tested against, instead of enforcing it in any way.
This way, non tested versions are out of support scope, but if a user chooses to ignore the official policy then it can still install and work. If it doesn't, that's the user's problem and not the maintainers'.

But don't prevent people from using your package if they know what they're doing!

@brettz9
Copy link
Collaborator

brettz9 commented Oct 23, 2021

Appreciate your arguments, but I think your two items are in a sense connected for us, in that we become informed of the need to test new versions by our users if we haven't yet updated it ourselves. Rather than allowing users to encounter subtle bugs (or even potentially harmful ones) due to a breaking Node change, I believe it is better to enforce the changes, especially since we are actively maintaining the project and can update in relatively short order. If someone "knows what they're doing", they should be able to use their package manager to opt in to override the safety checks.

As much as some may say that the README is required reading, I tend to believe in more intrinsically safe and agile-friendly approaches. (And there are other projects, including linting projects, which do use engines in this manner as well.)

We've added support for Node 17 so in any case, things should work for now.

@jdforsythe
Copy link

@brettz9 how about Node 19?

@brettz9
Copy link
Collaborator

brettz9 commented Nov 17, 2022

@jdforsythe : The latest version should already be working for Node 19.

@batpurev
Copy link

batpurev commented Dec 26, 2022

is it compatible yet with version 18?

dspace@dspace:~/dspace-angular-dspace-7.4$ yarn install
yarn install v1.22.19
[1/4] Resolving packages...
[2/4] Fetching packages...
error eslint-plugin-jsdoc@38.0.6: The engine "node" is incompatible with this module. Expected version "^12 || ^14 || ^16 || ^17". Got "18.12.1"

@brettz9
Copy link
Collaborator

brettz9 commented Dec 26, 2022

is it compatible yet with version 18?

dspace@dspace:~/dspace-angular-dspace-7.4$ yarn install yarn install v1.22.19 [1/4] Resolving packages... [2/4] Fetching packages... error eslint-plugin-jsdoc@38.0.6: The engine "node" is incompatible with this module. Expected version "^12 || ^14 || ^16 || ^17". Got "18.12.1"

Yes, the latest version, 39.6.4 is compatible with 18 and 19...

@batpurev
Copy link

Hi. thanks for the response. I have yarn.lock file in which we have following:
what should be changed to make it use version 39.6.4.

thanks

eslint-plugin-jsdoc@^38.0.6:
  version "38.0.6"
  resolved "https://registry.yarnpkg.com/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-38.0.6.tgz#b26843bdc445202b9f0e3830bda39ec5aacbfa97"
  integrity sha512-Wvh5ERLUL8zt2yLZ8LLgi8RuF2UkjDvD+ri1/i7yMpbfreK2S29B9b5JC7iBIoFR7KDaEWCLnUPHTqgwcXX1Sg==
  dependencies:
    "@es-joy/jsdoccomment" "~0.22.1"
    comment-parser "1.3.1"
    debug "^4.3.4"
    escape-string-regexp "^4.0.0"
    esquery "^1.4.0"
    regextras "^0.8.0"
    semver "^7.3.5"
    spdx-expression-parse "^3.0.1
```"

@batpurev
Copy link

batpurev commented Dec 27, 2022

39.6.4
sorry, i am noob here. Tried changing it to.

eslint-plugin-jsdoc@^39.6.4:
  version "39.6.4"

but no luck.
same error:

error eslint-plugin-jsdoc@38.1.6: The engine "node" is incompatible with this module. Expected version "^12 || ^14 || ^16 || ^17". Got "18.12.1"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

@batpurev
Copy link

changed following in package.json

"eslint-plugin-jsdoc": "^39.6.4"

but

following warnings and errors.

warning " > @nicky-lenaers/ngx-scroll-to@9.0.0" has incorrect peer dependency "@angular/common@^8.0.0 || ^9.0.0".
warning " > @nicky-lenaers/ngx-scroll-to@9.0.0" has incorrect peer dependency "@angular/core@^8.0.0 || ^9.0.0".
warning " > @nicky-lenaers/ngx-scroll-to@9.0.0" has incorrect peer dependency "tslib@^1.10.0".
warning " > bootstrap@4.3.1" has unmet peer dependency "jquery@1.9.1 - 3".
warning " > bootstrap@4.3.1" has unmet peer dependency "popper.js@^1.14.7".
warning " > ngx-sortablejs@11.1.0" has incorrect peer dependency "@angular/common@^11.0.0".
warning " > ngx-sortablejs@11.1.0" has incorrect peer dependency "@angular/core@^11.0.0".
warning " > ngx-ui-switch@11.0.1" has incorrect peer dependency "@angular/core@>=5.0.0 <12.0.0".
warning " > ngx-ui-switch@11.0.1" has incorrect peer dependency "@angular/common@>=5.0.0 <12.0.0".
warning " > ngx-ui-switch@11.0.1" has incorrect peer dependency "@angular/forms@>=5.0.0 <12.0.0".
error An unexpected error occurred: "ENOSPC: no space left on device, copyfile '/home/dspace/.cache/yarn/v6/npm-typescript-4.6.3-eefeafa6afdd31d725584c67a0eaba80f6fc6c6c-integrity/node_modules/typescript/lib/tsserverlibrary.js' -> '/home/dspace/dspace-angular-dspace-7.4/node_modules/@swc-node/register/node_modules/typescript/lib/tsserverlibrary.js'".
info If you think this is a bug, please open a bug report with the information provided in "/home/dspace/dspace-angular-dspace-7.4/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

@brettz9
Copy link
Collaborator

brettz9 commented Dec 27, 2022

If there is something relevant we can do, please file a new issue pointing to a minimal test repo, but those warnings and error do not look relevant to our package.

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

Successfully merging a pull request may close this issue.

9 participants