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

v1.2.4 breaks compatibility with Node.js 6 #83

Closed
AndreyBelym opened this issue Sep 27, 2018 · 8 comments
Closed

v1.2.4 breaks compatibility with Node.js 6 #83

AndreyBelym opened this issue Sep 27, 2018 · 8 comments

Comments

@AndreyBelym
Copy link
Contributor

The crypto.randomFillSync was added in the Node.js v7.10.0. Node.js 6.x is in the Long Term Support status until April 2019. Requirement of crypto.randomFillSync is a breaking change for Node.js 6.x users.

Could you please republish v1.2.3 as v1.2.5, and v1.2.4 as v2.0.0 to be more SemVer-friendly, or at least specify the correct engines field in the package.json?

@ai
Copy link
Owner

ai commented Sep 27, 2018

Could you please republish v1.2.3 as v1.2.5, and v1.2.4 as v2.0.0 to be more SemVer-friendly, or at least specify the correct engines field in the package.json?

Node.js 6 is still LTS. I care about my users and do not remove active LTS support.

We just need to add a check and use different API for different Node.js versions.

  1. Do you want to send PR?
  2. Do you want to check tests, why they didn’t fall on Travis CI?

@ai
Copy link
Owner

ai commented Sep 27, 2018

I found a reason. Node.js 6 backported randomFill https://nodejs.org/en/blog/release/v6.13.0/

@ai
Copy link
Owner

ai commented Sep 27, 2018

Do we need to fix it in this way? Or we can just ask to update Node.js 6.x to latest minor version?

@AndreyBelym
Copy link
Contributor Author

AndreyBelym commented Sep 27, 2018

I've created this issue because another issue was opened today in the TestCafe repository: DevExpress/testcafe#2905. So after some discussion with the TestCafe team, we've decided to keep support of older Node.js 6.x versions (<6.13.0), because it's easier to e.g. freeze the specific nanoid version than explain to the users that they have to upgrade to the latest minor version.

I can make a PR if you decide to support older LTS versions, but if you don't plan to do it, I can just use v1.2.3 until April 2019 😄.

@ai
Copy link
Owner

ai commented Sep 27, 2018

@AndreyBelym nope, let’s support <6.13.0. If somebody has a problem, we should take care of it.

I am going sleep right now. If you want to make it as quick as possible, make PR with node <6.13.0 to Travis CI and if case with both API.

@victheone
Copy link

victheone commented Sep 27, 2018

Wanted to pipe in and let everyone know that this is also an issue in Node 7.9.0. Apparently crypto.randomFillSync is the problem here as well; it wasn't added until Node 7.10.X.

@danyfedorov
Copy link

@ai
Copy link
Owner

ai commented Sep 29, 2018

Fixed in 1.2.5 by @AndreyBelym

@ai ai closed this as completed Sep 29, 2018
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

4 participants