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

Removing node-4 support is a breaking change and should be done in a major release #141

Closed
daprahamian opened this issue Nov 2, 2018 · 5 comments

Comments

@daprahamian
Copy link

daprahamian commented Nov 2, 2018

This is in response to 97a4e64 and ee41b4c

There are multiple packages relying on graceful-fs that still need to run in node 4. It is perfectly acceptable to drop support for node 4, but it must be done in a major version bump so as to not retroactively break current versions of packages.

@isaacs
Copy link
Owner

isaacs commented Nov 2, 2018

Still works fine in Node 4, just won't be actively tested there. (So the Buffer.from in the test is fine.) It's 6 months out of LTS. You aren't getting security updates. It is dangerous to actively develop on unsupported platforms, don't do it.

@isaacs isaacs closed this as completed Nov 2, 2018
@daprahamian
Copy link
Author

I am not advocating for new development on Node 4. I'm saying that there are two consequences for this:

  • packages that previously worked may now suddenly stop working from a change on this package. IMO it's is bad practice to break something that previously worked fine, and it will erode trust in semver.
  • changing the engine version will break anyone who uses yarn or uses strict engine checks on node 4.

Again, I have no problem with dropping support for node 4, and I actively encourage it. I am merely saying that these changes are inherently breaking changes, and as such should be done under a major version change.

@isaacs
Copy link
Owner

isaacs commented Nov 4, 2018

Strict engine checks seem like a bad idea. Does yarn enforce this by default??

@daprahamian
Copy link
Author

Yes, yarn explicitly will error on install if the engine field does not match. You have to pass --ignore-engines to bypass it. The feature has been around since yarn was created, and is considered by some a major benefit / selling point over NPM. See the discussion here.

Also, should we move this discussion to #146 ?

@isaacs
Copy link
Owner

isaacs commented Nov 4, 2018

Wait a second, so, you're telling me, yarn will go ahead and choose a module version with an engines field that isn't allowed, and then crash after the fact?

That's... that's not good.

Whatever. I don't care enough. I removed the engines field in 4.1.15. I figured that if it was there, I may as well broadcast the versions that are actively supported, but I can't be a part of such a profoundly bad UX choice. Use at your own risk, as always, I guess.

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

2 participants