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

Drop support of old Node.js versions #100

Closed
bsautel opened this issue May 2, 2020 · 6 comments
Closed

Drop support of old Node.js versions #100

bsautel opened this issue May 2, 2020 · 6 comments
Milestone

Comments

@bsautel
Copy link
Contributor

bsautel commented May 2, 2020

It sounds like all versions before the 10 LTS are end of life. I think it is no longer necessary to support them.

Removing its support would enable us to simplify the NodeSetupTask and specific cases regarding very old versions (some 4.x or 6.x) for which the zip distribution did not already exist.

We would not have to parse the Node.js version and this would enable versions such as 10.+ to work.

This probably would enable us to fix #95 and #54 more easily.

@bsautel bsautel added this to the 3.0 milestone May 2, 2020
@deepy
Copy link
Member

deepy commented May 4, 2020

What I don't like about enabling things such as 10.+ is that it requires us to be able to reach and parse an index page listing all the versions and if someone uses a mirror this isn't necessarily going to work.

Given that these are end of life I think it'd be perfectly reasonable for us to only support them in the download = false scenario

@bsautel
Copy link
Contributor Author

bsautel commented May 5, 2020

You are right, there is also another downside which is that your build is not reproducible. And how often do you check whether there is a new version of not? You can't look for newer versions at each nodeSetup run, but if you never do it, you no longer run the latest version... We would probably do the same thing as we do for Yarn now: we install the latest version when the yarnSetup task runs and never update it after.

But as you said, dropping support of those very old versions does not imply we support dynamic Node.js versions.

I am going to remove the code that supports setup of this very old versions.

@deepy
Copy link
Member

deepy commented May 5, 2020

I guess we could extract the earlier snippet into a documentation entry for those who want to do it and are ok with coding against an index pattern, I think there's luckily some Gradle caching being done so it's not as awful as it could be, but if someone's idea of fun is risking the build breaking between invocations then I'm not going to stop them :-)
There's probably some very legitimate setups like having a nightly job testing against the latest version of node though.

@bsautel
Copy link
Contributor Author

bsautel commented May 5, 2020

I'm ok to explain how to do that in the documentation and not support it directly (at least for now).

@bsautel bsautel closed this as completed May 5, 2020
@deepy
Copy link
Member

deepy commented May 5, 2020

It has to be better to document how to do this rather than adding API methods for interacting with the IvyRepository right?

@bsautel
Copy link
Contributor Author

bsautel commented May 5, 2020

What do you mean by adding API methods? Adding a kind of DSL that would configure a custom repository and that would also set the distBaseUrlto nullso that the default repository is not added?

stuartraetaylor added a commit to stuartraetaylor/gradle-node-plugin that referenced this issue Nov 24, 2020
3.0.0-rc4 - kotlin rewrite, lazy configuration, api changes, proxy fixes.
* Plugin rewritten in Kotlin (issue node-gradle#17) (thanks mikejhill for the pull request)
* Improved Kotlin DSL support
* Upgraded default Node.js version to 12.18.1 (bundled with npm 6.14.5), the latest LTS version
* Lazy configuration support
* Fix some remaining input/output declaration issues (issue node-gradle#34)
* Gradle 5.6.4+ support (instead of Gradle 5.0.0+ before)
* Node.js 10+ support (issue node-gradle#100)
* Use http:// for both HTTP and HTTPS_PROXY
* No longer configures proxy if there's already proxy settings present
* Support for npm 7+ node-gradle#123

See CHANGELOG for full details and upgrade instructions.
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

2 participants