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

Allow node 10.19 #912

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Allow node 10.19 #912

merged 1 commit into from
Feb 3, 2021

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 3, 2021

This was bumped in #645
Ubuntu LTS ships node 10.19 by default.
If we can, let's save ppl (aka me) from installing yet another ppa.

@jdreesen
Copy link
Contributor

jdreesen commented Feb 3, 2021

Note that Node 10 EOLs at 2021-04-30: https://nodejs.org/en/about/releases/

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Feb 3, 2021

That's the EOL date from node maintainers. There are other ppl that take over maintenance after them, eg from Ubuntu LTS or RHEL.

@tgalopin
Copy link
Member

tgalopin commented Feb 3, 2021

An indirect dependency of Encore (webpack-manifest-plugin) doesn't support 10.19 anymore, I guess that's the reason behind the bump.

@Kocal
Copy link
Contributor

Kocal commented Feb 3, 2021

I don't think using nodejs (or php, python, ...) binaries from Ubuntu (or other any OS) official repositories is a good idea.
Using a versions managing tool, like nvm, would allow you to use any Node.js versions you want (latest 10, latest 12, etc..).

Also, I'm not a big fan for lowering the version range "just for you", because of your OS constraints. IMO, this is not a problem Encore should have to deal with.

And as @tgalopin said, webpack-manifest-plugin requires at least Node 10.22.1.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Feb 3, 2021

Thanks for spotting the dep that triggered the bump. Here is a PR to fix it too: shellscape/webpack-manifest-plugin#247

We don't have to wait for this other PR to merge this one.

Bumping when there is no technical reason to do so is doing DRM and forcing ppl (not just me this time) to spend more time doing extra work. We can definitely be more friendly towards our communities by relaxing minimum versions too.

@weaverryan
Copy link
Member

Thanks Nicolas!

@weaverryan weaverryan merged commit b8ced11 into main Feb 3, 2021
@tgalopin tgalopin deleted the nicolas-grekas-patch-1 branch February 3, 2021 20:16
@nicolas-grekas
Copy link
Member Author

Unfortunately shellscape/webpack-manifest-plugin#247 has been rejected :'(

@Kocal
Copy link
Contributor

Kocal commented Mar 18, 2021

Should we revert this PR?

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

Successfully merging this pull request may close these issues.

None yet

5 participants