Yarn: How to handle invalid engines

[kevinastone]

Similar to #1003

Do you want to request a _feature_ or report a _bug_?

Bug

Fix: Add atom to:
https://github.com/yarnpkg/yarn/blob/b5087a2dde4e2b17121eb4cb5d5e1ca877c3e41f/src/package-compatibility.js#L53

[sebmck]

I don't think we should suppress this. Atom is quite new so existing libraries should update themselves to remove the engine as we're correctly enforcing this. In the future this warning may be a hard error.

[kevinastone]

I'm confused. Atom uses the engines field to specify the compatible version of Atom for a given package. Is Atom not supposed to use the engines field? Is there an issue on Atom to change this?

[sebmck]

I guess this calls into question about why we report on invalid versions. cc @samccone

[samccone]

This seems like an abuse of engines according to npm docs.

You can specify the version of node that your stuff works on:

{ "engines" : { "node" : ">=0.10.3 <0.12" } }
And, like with dependencies, if you don't specify the version (or if you specify "*" as the version), then any version of node will do.

If you specify an "engines" field, then npm will require that "node" be somewhere on that list. If "engines" is omitted, then npm will just assume that it works on node.

You can also use the "engines" field to specify which versions of npm are capable of properly installing your program.

[kevinastone]

Seems to be pretty baked into Atom:
https://github.com/atom/package-generator/blob/master/package.json#L20-L22

[jdalton]

Via the npm/doc/misc/npm-developers.md

engines: Specify the versions of node (or whatever else) that your program runs on. The node API changes a lot, and there may be bugs or new functionality that you depend on. Be explicit.

That wording was added in 2010 when the documentation was written.
Given that, the current level of validation seems unnecessary for this field.

[mgol]

Since npm not only doesn't enforce engines by default but also has removed the possibility to enforce it in package.json via "enginesStrict": true, package owners have been used to being able to use engines as a way to say "we've tested the project on those versions, we don't guarantee working on others". This doesn't mean the project won't work in other versions and it often does. One example of a package that uses engines like that is karma.

In the world where yarn is the only CLI client to the npm registry making engines enforced might work. However, the npm registry is primarily accessed via npm so if yarn is more strict than npm in a way that is contrary to how npm itself advises to use the engines field, that creates an incompatibility with the whole ecosystem. I don't think that's a good idea.

EDIT: Note that this also means that basically any project that enforces Node.js version somehow (even with a loose "node": ">=4") will not work on Node.js pre-releases as they don't meet this check. This is pretty invasive.

[hutson]

For projects I'm responsible for, I use the engines property as a way to indicate which versions of Node I actively support, and on which versions of Node that particular version of the package should be capable of running on for eternity. As soon as a version of Node reaches end-of-life, I remove it from the engines property and release a new major version of the package. When a new version of Node enters LTS, I add it to the engines property (because I actively support LTS versions).

Users of those package may use use it with newer, unsupported, versions of Node if they wish. In the case of npm they'll just receive a warning that the package doesn't indicate support for that particular version of Node, so they need to accept a little risk with the decision. That's usually fine, and people can move forward with their work.

However, yarn's behavior makes unsupported Node versions a hard failure. Any attempt to use an unsupported Node version fails the yarn install process.

There's the hammer approach of using ignore-engines, but that provides no warning to the user that one or more dependencies are not officially supported on their version of Node.

[hutson]

Perhaps we can just implement a pushWarning, similar to pushError (https://github.com/yarnpkg/yarn/blob/93fa0f7c2857b5e9301d554b2b4c362d20988324/src/package-compatibility.js#L123) to call reporter.warn with the incompatibly, instead of erroring out.

[jkrems]

As I mentioned in earlier discussions on this: There's also "I install this package because I want to bundle it into my client-side app. Why would I care that it wouldn't run in the version of node I happen to use to download the tar"?

I think even adding a help text for resolving the issue ("add the following to the .yarnrc file in this project to disable this check") would go a long way.

[hutson]

Why would I care that it wouldn't run in the version of node I happen to use to download the tar"?

That's an excellent point, and as the JavaScript community consumes more npm packages for client-side applications, it's a point that more will want to raise.

add the following to the .yarnrc file in this project to disable this check

Would that suggestion apply to both dependencies and dev dependencies?

[gsklee]

@bestander close; non-issue

[hutson]

It should be noted that this issue was closed as the current Yarn behavior is working as designed.

Please see - https://github.com/yarnpkg/rfcs/pull/69#issuecomment-308567694

[gsklee]

Yep, and another reason is that this issue was repurposed into a discussion which yielded no clear result. If anyone would like to continue the discussion of this topic, please continue inside the RFC @hbetts has linked. Thanks for the linking 😄