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

last release should be major version - breaks compatibility with pre ECMAScript 2015 #2774

Closed
eandersonpgi opened this issue Sep 20, 2017 · 16 comments

Comments

@eandersonpgi
Copy link

Summary

#2751 breaks compatibility, the resulting hawk version should have incremented the major version number, not the patch number (2.82.0 to 2.82.1) in 6f1b51e

I updated hawk from 3.1.3 to 6.0.2 (still a ~ for version managing)

This jump in versions changed the which version(s) of NodeJS it is compatible with and should have resulted in a major version change. Only the patch version was updated (not even the minor version!), consequently libraries which depend on version 2.x of request automatically pick up a breaking change.

Simplest Example to Reproduce

run/compile in code that requires/uses pre- ECMAScript 2015

Expected Behavior

compiling an application that has transitive dependencies on request 5 days ago and today should continue to work, especially when the transitive dependency specifies a locked major version.

Current Behavior

building a project fails due to the new patch version.

/node_modules/grunt-juice-email/node_modules/juice/node_modules/jsdom/node_modules/request/node_modules/hawk/node_modules/boom/lib/index.js:5
const Hoek = require('hoek');
^^^^^

Loading "juice.js" tasks...ERROR
>> SyntaxError: Use of const in strict mode.

Possible Solution

remove version 2.82.1, update to 3.0.0 to account for the breaking change

Context

Builds that worked on the 15th no longer work, code work, deploys that are in-progress are halted. We're looking at having to create a snowflake fork of this library just to keep our application working, since upgrading our NodeJS across the board is not a short-term option.

Your Environment

software version
request 2.x (via jsdom )
node 0.10.26
npm
Operating System Debian 7.11
@mikeproeng37
Copy link

+1 here. This is breaking all of our builds because a lot of the libraries we use depend on this. Please either revert or fix!

@kwalker3690
Copy link

+1 Please revert or fix immediately, it will be a lot of work to go through and fix this in our own projects rather than fixing it in this repo!

@lostip
Copy link

lostip commented Sep 20, 2017

I don't think this is an issue...

The package.json has the engine properly set:

   "engines": {
     "node": ">= 4"
   },

but npm has a very unfortunate default, where this is interpreted as a suggestion sort of, unless you use the engine-strict flag.
See: https://docs.npmjs.com/files/package.json#engines

@mikeal
Copy link
Member

mikeal commented Sep 20, 2017

Nope.

Read thread #2772 (comment)

I also recorded some audio w/ @watson about it https://www.patreon.com/posts/thomas-watson-on-14473075

If you are still on a version of Node.js no longer supported by the project, which we removed support for 11 months ago, and have been ignoring the warnings npm generates that entire time, it's on you to go in and lock the version of request your rely on to an earlier version.

@mikeal mikeal closed this as completed Sep 20, 2017
@mikeproeng37
Copy link

If we are using libraries that aren't locked into the earlier version of request and those libraries have children dependencies, how do we solve for that?

@mikeal
Copy link
Member

mikeal commented Sep 20, 2017

@mikeng13 shrinkwrap on older npm, package-lock.json on newer npm.

@eandersonpgi
Copy link
Author

eandersonpgi commented Sep 20, 2017

Understood, but that does not make the change any less of a breaking change, which should be indicated with a major version increment, not a patch version. http://semver.org/

@mikeproeng37
Copy link

Got it, we can look at that. Thanks

@mikeal
Copy link
Member

mikeal commented Sep 20, 2017

@eandersonpgi you can read the thread to more insight, but the tradeoffs just aren't worth a major version update. too much of the ecosystem that is doing the right thing and staying on Node.js versions that aren't EOL would stop getting fixes.

@mikeal
Copy link
Member

mikeal commented Sep 20, 2017 via email

@lostip
Copy link

lostip commented Sep 20, 2017

@mikeal sorry, misread the user name... I'll get new glasses :)

@sanimalp
Copy link

It is crazy this was closed. Pretending old versions of frameworks don't exist is not an excuse for ignoring SemVer.

@mikeal
Copy link
Member

mikeal commented Sep 20, 2017

@sanimalp old versions of frameworks will work with old versions of request, they just need to lock in the dep version.

why should the burden of manual intervention be on people who are using secure versions of Node.js instead of those insisting on running EOL versions of Node.js?

@sanimalp
Copy link

You can do whatever you want.. It is your project, after all. I am just pointing out that lots of work will have to be done by lots of people because you choose not to respect SemVer.

Using this as an opportunity to grandstand about EOL versions shows a real lack of compassion, or perhaps understanding, about software lifecycles as well.

@MarkSG93
Copy link

You can't just bump a breaking change a minor version bump. We have projects that have been ticking over for many years because we can't build them anymore. We also can't pin down the previous version of request because it's not a direct dependency but one that is 3 levels deep.

Not following semver because you

don't think a major version is worth it

isn't a good reason.

@tylik1
Copy link

tylik1 commented Sep 26, 2017

For me the same, it breaks our builds, and we can't update node.js verson on system that easily. Please redo the release and bump it to the later version! Also lock won't help as the script evolved and different builds use different commits

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

8 participants