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

Consistent tests against supported NodeJS versions #7

Closed
6 tasks done
evertonfraga opened this issue Dec 4, 2019 · 14 comments
Closed
6 tasks done

Consistent tests against supported NodeJS versions #7

evertonfraga opened this issue Dec 4, 2019 · 14 comments

Comments

@evertonfraga
Copy link
Member

evertonfraga commented Dec 4, 2019

@nivida pointed that NodeJS LTS is reaching its end-of-life in December 31st, 2019.

So I have looked into the several repos we have and came up with a table of node versions we have in our tests, meaning that "we support" and care for.

  • -vm: add 10, 12, 13 PR
  • -account: drop 6, 7; add 13 PR
  • -blockchain: drop: 11; add: 10, 13 PR
  • -block: add: 10, 12, 13 PR
  • -tx: drop: 11; add: 10, 13 PR
  • -common: drop: 6; add: 10, 13 PR

Edit: Removed table with proposed changes.

@alcuadrado
Copy link
Member

It would be great if this config can be somewhat shared, so it only has to be updated once. I think Github Actions lets you share this kind of things. The monorepo migration would make this much easier though.

Also, I think including beta versions would be useful. Currently, we only test against node versions after they are released, and many times things break. This is an issue, as many users update their node installations as soon as a new version is released.

@ricmoo
Copy link
Contributor

ricmoo commented Dec 4, 2019

I've always found the NodeJS support cycle to be far too fast (and loose with the definition of "long"), with only around 6 months of release to entering LTS. As a developer, especially of iOS apps, this has often bit me. Updating an app often takes a lot longer because of this, and often it becomes far too tedious and the app simply dies, because some interstitial dependency stopped working and was too much effort to re-write.

It's important to realize that that node is often a sub-sub-* dependency, and each layer in a stack adds migration time, and many frameworks still in use may have stopped being supported entirely. Stopping support means that those libraries are often unable to update their top dependencies, because of failures in the lower dependencies.

ReactNative, Expo and many of those, even on fairly recent versions still use very old versions of Node, which may be impacted by dropping support for these versions.

I am considering dropping support for Node 6, but do not see the need to be so aggressive. Dropping support for 8 means we are disqualifying any project that began on an Active LTS as late as December 31st, 2018; less than 1 year ago.

At the end of the day, most of our code is TypeScript, which while painful, is possible to target ES3-ish and ESM, and most of us use a CI which makes it trivial to tack on additional test versions.

I plan in ethers v6 to start using ESM more aggressively (mainly WeakMap and Proxy) à la ethers-app, at which point v5 will become the last release for legacy users and there may be non-trivial dropping of node test versions. But in the meantime, there are still plenty of users not on the latest versions for reasons that are not their fault.

Also, at the end of the day, very little has changed (other than ESM) in most versions of Node, so maybe it doesn't matter either way, but it only takes about 15 minutes for the 22K test cases to pass, and it can usually be done in parallel.

Anyways, that was my mash-mash of thoughts...

(P.S. I've already suggested this to a few people, but I recommend everyone create a project using their respective libraries to get a feel for how it works in the "real world"... I'd like to add an extra suggestion to that, maintain that project to see how much effort is required to keep it working. ;))

@ricmoo
Copy link
Contributor

ricmoo commented Dec 4, 2019

(note: I'll add node 13 soon; I've tested locally, but haven't updated the CI; we're working right now on migrating from TravisCI to CircleCI, and is one of the last needed things before releasing v5 as a non-beta)

@evertonfraga
Copy link
Member Author

Thanks for the input, @ricmoo!

@holgerd77
Copy link
Collaborator

I think the argumentation from @ricmoo makes a lot of sense, maybe we should at least keep Node 8 for some more time. Release cycles are indeed fast and the ecosystem usage is very diverse, some people update pretty fast, some need to stay on older versions for various reasons.

@evertonfraga
Copy link
Member Author

Right, I updated the issue description. FYI, @ryanio is keeping node 8 on the PRs he opened.

@alcuadrado
Copy link
Member

I think the situation is a little more complicated, as almost everything Ethereum is pretty sensitive security-wise. Telling our users that it's ok to use unsupported platforms, which won't get security patches, may be a disservice to them.

I don't know what a good policy for this would be, but I think there's a lot of value in defining and documenting one.

@ricmoo
Copy link
Contributor

ricmoo commented Dec 5, 2019

@alcuadrado I half agree with that sentiment; there is no good policy. Just various degrees of terrible ones. :)

But, if there is for example, a critical update in ethers, and they cannot update to it because they run node 6, and the new ethers has dropped support for it, now their only options are to

  1. Update their node and all other dependencies, which may or may themselves support the updated version of node and may require their own updates, plus updating to the new ethers; which may require non-trivial development resources
  2. Do nothing and continue using the old ethers with whatever critical issues there may be

Many projects which are under-staffed (which is basically every project) will just go with option 2. So, we didn't force them to keep up-to-date, we just forced them to not update one more thing.

Again, there is no good solution. I'm just speaking as someone who has had to choose between these options many, many times... And over the years, I've had to follow both courses for various projects.

@nivida
Copy link
Member

nivida commented Dec 9, 2019

@ricmoo Good point!

web3.js will keep the support for version 8 of nodejs for 1.0 and will drop the support for v8 as soon as 2.0 of web3.js is released as a stable version. This because we would have otherwise to release a breaking change within a minor version.

Telling our users that it's ok to use unsupported platforms, which won't get security patches, may be a disservice to them.

@alcuadrado I think this could be solved with a small recommendation in the documentation of our projects. For those who are aware of the security issues is it still possible to use a newer version and those who are stuck with v8 for now still have the possibility to get required security patches from our side.

@evertonfraga
Copy link
Member Author

Log:
Node 8 dropped and Node 14 added to the respective CI files:

  1. ethereumjs-account
  2. ethereumjs-block
  3. ethereumjs-blockchain
  4. ethereumjs-common
  5. ethereumjs-tx
  6. ethereumjs-vm

It would be great if this config can be somewhat shared, so it only has to be updated once.

@alcuadrado I support this idea. Any suggestions on how to share the configuration of node versions to test against? There doesn't seem to have such built-in option in GH Actions. This is as far as I could get:

Usually, Node versions can't be defined as environment variables in the CI. We could have a devDependency like @ethereumjs/config-node-version that modifies all CI files injecting those node version files. It could be bound to a repository hook with husky for added consistency.

@evertonfraga
Copy link
Member Author

evertonfraga commented May 7, 2020

GitLab has yaml includes though: https://docs.gitlab.com/ee/ci/yaml/#using-extends-and-include-together

@alcuadrado
Copy link
Member

@alcuadrado I support this idea. Any suggestions on how to share the configuration of node versions to test against?

I haven't found a way to do this, which is surprising. I'd like something that automatically made my CIs run against every currently supported and beta node.js version.

@holgerd77
Copy link
Collaborator

holgerd77 commented Aug 24, 2020

Will move this over to the EthereumJS organization repo since Ethers and Web3 will likely continue with their own setup here, so we can concentrate to unify/simplify this within the EthereumJS ecosystem, eventually by setting up some shared ethereumjs-config configuration.

@evertonfraga
Copy link
Member Author

@alcuadrado Finally there's a way to do that! 😄

I grab fresh information at build time from NodeJS releases and GitHub's available node images, and output it to a subsequent matrix of jobs. It's being implemented here: ethereumjs/ethereumjs-monorepo#939.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants