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

Builds should test on Node X.0.0 as opposed to Node X@latest #54

Closed
stephenplusplus opened this issue Mar 5, 2019 · 10 comments
Closed
Assignees
Labels
type: question Request for information or clarification. Not an issue.

Comments

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Mar 5, 2019

We just ran into an issue where require('url').URL was a feature added in Node 6.13. Our tests seem to build on 6@latest, so all was assumed to be well.

@JustinBeckwith
Copy link
Contributor

In my mind, this is by design! Technically the node foundation only supports the latest version of any node.js release in the line. Also, node doesn't go LTS until a release after x.0.0, so it doesn't make sense to require something less.

Curious about how @ofrobots and @MylesBorins think about this.

@stephenplusplus
Copy link
Contributor Author

Quick mention that we communicate through package.json that we support >6.0.0, e.g. https://github.com/googleapis/nodejs-common/blob/791b40273ae07eff93a6ebc0261cc62178614ad2/package.json#L8

@MylesBorins
Copy link

From an LTS perspective we only support the latest version of an LTS line. This includes the Semver-Minor additions. Part of this is due to security releases, which sometimes themselves have breaking changes.

While project could test against x.0.0 that is generally a bad idea... especially since that release is not guaranteed to be stable (many initial cuts have lots of regressions). A conservative approach could be testing with the initial LTS release... but TBH I think it is very reasonable to have a support contract for the latest Semver-Minor version of an LTS release line.

The Node.js project does a lot of work to ensure that LTS releases do not have regressions... and running on an arbitrary older version of an LTS line not only limits features but is insecure. As mentioned above we could limit ourselves to the features introduced in the initial LTS release, but I don't believe that is necessarily the best limitation. If people need to run in specific Semver-Minor versions of maintenance LTS they are already giving up all sorts of bug fixes and security releases.

Re: >6.0.0, we could in theory bump that as we introduce new features if we know about them... or at the very least pin it to the initial LTS version

v6.10.0 is over 2 years old FWIW

@alexander-fenster
Copy link
Contributor

@MylesBorins That all sounds great but here the comments in #44 show that, just for example, users of Amazon Lambda can either use Node.js v8.10 or v6.10, neither of those are the latest versions in the corresponding branches.

https://docs.aws.amazon.com/lambda/latest/dg/programming-model.html

image

@JustinBeckwith
Copy link
Contributor

Realistically - this library will drop support for node.js 6 the day it goes EOL. It may be worth considering testing Node 8.10, but there's no way we're going back to an older version of 6 at this stage.

@attaboy
Copy link

attaboy commented Mar 15, 2019

Just wanted to chime in on this ticket since the underlying problem affected me in a bad way — I have a project that runs code on AWS Lambda, still using v6.10 and which uses the googleapis NPM package for a customer solution. (For perspective, AWS only added support for v8.10 in April 2018 and our node runtime version has not been a primary concern until now.)

The surprise to me was not so much that a major new version of googleapis stopped working (v37 which switched to gaxios), but that even going back many versions, it still won't work on a fresh npm install because it depends on google-auth-library which, going back many many versions, depends on gtoken@^2.3.0.

gtoken v2.3.1 switched to gaxios, and so every version of googleapis that indirectly has ^2.3.0 as a dependency will also depend on gaxios — even versions released before the end of node 6's LTS.

@alexander-fenster
Copy link
Contributor

@attaboy From experience, since Node 6 is so close to EOL, we'll soon see a bunch of other dependencies stopping supporting it, breaking in some unusual way, and so on. At least that is what happened when Node 4 was EOLed a year ago.

Regardless if this particular package is made to support Node 6 or not, feel free to add my quick and dirty fix for missing URL constructor from this comment. And think of migrating to Node v8 since AWS supports it.

@attaboy
Copy link

attaboy commented Mar 15, 2019

@alexander-fenster Understood, and I've already done the first thing and started the migration. 🙂

Again though, it's not unexpected that new versions of a library would stop supporting old versions of Node — but I would urge that in future, such changes be reserved for major version releases rather than patch releases so that they don't have "retroactive" effects given the way the npm tool respects semver.

(That is, this is more a complaint about the decision to switch libraries in a patch release of gtoken, not a complaint about gaxios itself… happy to file a ticket on that project with that suggestion, but I suspect the same folks are already here. 🙂)

@alexander-fenster
Copy link
Contributor

Agreed - the fact that it got broken after a set of patch releases looks like a mistake. Good to know that you were able to resolve that in your project.

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels May 1, 2019
@JustinBeckwith JustinBeckwith added type: question Request for information or clarification. Not an issue. and removed 🚨 This issue needs some love. question triage me I really want to be triaged. labels May 13, 2019
@JustinBeckwith
Copy link
Contributor

For now, I don't think we're planning to make a change here. If we do, it's something we should probably do consistently, and discuss in google-cloud-node.

@JustinBeckwith JustinBeckwith self-assigned this Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

6 participants