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

No longer compatible with Node 4 #1315

Closed
virkt25 opened this issue May 7, 2018 · 7 comments
Closed

No longer compatible with Node 4 #1315

virkt25 opened this issue May 7, 2018 · 7 comments

Comments

@virkt25
Copy link
Contributor

virkt25 commented May 7, 2018

Q&A (please complete the following information)

  • OS: macOS
  • Environment: Node 4.8.6
  • Method of installation: npm
  • Swagger-Client version: 3.8.1
  • Swagger/OpenAPI version: [e.g. Swagger 2.0, OpenAPI 3.0]

Content & configuration

This is a dependency of a dependency so not sure of the implementation.

Describe the bug you're encountering

/loopback/strong-globalize/node_modules/g11n-pipeline/node_modules/swagger-client/node_modules/object-assign-deep/objectAssignDeep.js:77
function executeDeepMerge (target, _objects = [], _options = {}) {

swagger-js is no longer compatible with Node 4 due to object-assign-deep. Are there plans to fix it ... if not the README should be updated to reflect Node 6 and higher support

To reproduce...

object-assign-deep is Node >= 6 only.

Expected behavior

Expected this module to work on Node 4 / README should be updated to reflect the supported versions.

Screenshots

Additional context or thoughts

Looks like object-assign-deep was added recently in #1309

@gzurbach
Copy link

gzurbach commented May 7, 2018

We're experiencing the same issue. This is breaking all our builds still relying on Node 4 (over a dozen APIs). This is a breaking change and should have been published under a new major version (i.e. swagger-js@4.0.0) by semver specs.

Our apps include swagger-js@3.x very deep in the dependency tree (we use loopback) so we cannot easily lock the version numbers.

The ideal solution would be to publish a 3.8.2 that reverts the latest changes and re-publish the latest version (3.8.1) as 4.0.0 while indicating the requirement for Node 6+.

@shockey
Copy link
Contributor

shockey commented May 7, 2018

@virkt25 + @gzurbach - sorry for the rough ride with this dependency, it looks like the module is written by hand (see saikojosh/Object-Assign-Deep#4) and uses const, which breaks in Node 4.

It just so happens that we dropped Node 4 from our tests in anticipation of that version reaching End-of-Life this month (though it looks like that's April 30th, to be exact), so this wasn't caught before release.

I'm going to fix this, by either forking the problem module or using something else entirely, and publish a patch version for swagger-client.


However, going forward from April, if you stay on Node 4 we might break your implementation without bumping the major. I'm working off of @mikeal's view of version bumping vs leaving EOL users behind (emphasis mine):

Everything is a tradeoff. I understand that people stuck on old versions don't like this tradeoff, but you are in the minority, and we only have the resources to support a single major line of development, so the choice was between breaking you or opting the rest of the community out of automatic updates. This will not be the last time you are inconvenienced by changes like this in dependencies so it's a good idea for you to lock down your deps with shrinkwrap and stop updating them. Or, better yet, update to a supported version of Node.js.

For package authors: it's up to you whether or not to bump the major version, but supporting EOL versions of Node.js is not a good practice and encourages irresponsible/insecure behavior of your users.

request/request#2772 (comment)

So in short: I'm fixing this now, but please upgrade your Node versions!

@gzurbach
Copy link

gzurbach commented May 7, 2018

@shockey Thanks a lot for the quick reply. I've ben wanting to update Node in all those projects for months but my boss wants me to write new shiny toys instead. I'm sure you've heard this before 😄

What happened today will give me leverage to convince him to add this work in future sprints!

@virkt25
Copy link
Contributor Author

virkt25 commented May 8, 2018

Thanks for the update @shockey ! While I totally agree that EOL versions shouldn't be supported for all the reasons stated above and in the linked comment - it's a little easier said than done when enterprise customers are a bit slow to migrate and expect support.

That said, we're working on a plan to drop Node 4 and your comment helps with that. :)

@shockey
Copy link
Contributor

shockey commented May 8, 2018

v3.8.2 should hit npm in a few moments, so an npm update should fix this issue on your end.

(Edit: Wow, it is May already. v4 is dead. Time flies 🤦‍♂️ )

Cheers!

@gzurbach
Copy link

gzurbach commented May 8, 2018

Thanks for publishing v3.8.2 it will buy us some time until we have converted all our APIs.

FYI - I spent the last 10 hours updating some of our apps to Node 6 and I think I got it all working now. 2016 here I am, woohoo!

@virkt25
Copy link
Contributor Author

virkt25 commented May 8, 2018

Thanks for the fix @shockey Appreciate it and definitely gives us some time to come up with a plan to drop support for Node 4. And haha yea it's been May for a week now so you were in the right to drop support.

@gzurbach You might get extra life if you moved on to Node 8 / 10 instead as Node 6 will be End of Life this time next year.

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

3 participants