-
Notifications
You must be signed in to change notification settings - Fork 21
Allow frameworks to specify the minimum Node.js version #288
Comments
My concern here is that different versions of frameworks require different versions. Could we read it from the framework's |
I guess we need to know before we install it, but we could just grab the framework's package.json before installing the right node version |
Yes, this makes lots of sense. Please note that I meant the above to be specific about frameworks, not plugins. I.e. I meant it to be part of the following files. Since we declare the npm package names using the |
Doesn't What is the additional gain in implementing the logic ourselves? The benefit I can see is during site creation so we can recommend users which Node.js version to use. However this might be better solved by always using Node.js LTS as the default Node.js version. |
As I understand it, we'd do it so we can automatically switch to a supported version. This is why we'd want to know the supported version before running the install. |
Thanks for clarifying @ascorbic. Will using Node.js LTS by default solve this? My suggestion only works if we are OK with assuming all frameworks support Node.js LTS (I think it's quite safe to assume it). That way we won't have to maintain additional logic. If we do automatically chose a Node.js version based on the framework's engines field we would also need to communicate that to users property. |
That sounds like the simplest solution. I can't think of a scenario where a new site wouldn't work with LTS, but if there were I think it would be rare enough to say that they could set it manually. |
This is a reasonable approach @erezrokah 👍 One potential gotcha is that some frameworks (like Nuxt) do not have an |
Using Node LTS by default sounds great. SvelteKit just dropped support for Node 12 and we're getting a number of complaints that deploys are failing on Netlify now. I sent a PR that I hope fixes this just for SvelteKit, but I'm not quite sure if it does: #553. We do specify an |
@benmccann I made the same mistake, but unfortunately setting the env vars in framework-info doesn't work. With the change of the default build image to Focal, we are at least now defaulting to newer Node for builds. Unfortunately that doesn't apply to the lambda runtime, which still defaults to 12. We have an internal issue which covers this, but I think that right now we should default the lambda runtime to 14 (and hope that Amazon does a new release with 16 soon) |
I've highlighted this again internally, as I think this is a growing problem. As well as SvelteKit it now affects Gatsby and in some cases Next.js. |
All Node.js-based frameworks have a specific minimum Node.js version.
At the moment, if a user runs a framework on Netlify with a Node.js version that's too old, the framework will most likely fail at
require()
-time with some cryptic error message due to some unsupported JavaScript feature or missing Node.js core method. Users might be confused and think this is a bug in Netlify.Since we detect frameworks, we could use that knowledge to infer the minimum Node.js version. We could do this by adding a new
nodeVersionRange
field to the frameworks declarations:^10 || >=12
or>=12.20.0
since some frameworks support Node.js version range that are more complex than just>=version
.node-semver
minVersion()
method.We could use that information to either:
I would personally favor only warning users and not overriding the Node.js version based on that information because:
engines.node
field inpackage.json
. This is also consistent with hownpm
andyarn
behave duringnpm install
(although one warns only while the other fails).That being said, I could also see an argument being made that Netlify users might expect the platform to just use the right Node.js version so things "just work" with their chosen framework.
Note: this discussion was brought up due to Nuxt supporting only Node.js
>=12.20.0
.I would be very curious to learn more about what you all think about this idea, especially @ingride @JGAntunes @erezrokah @verythorough @lukeocodes @ascorbic. Thanks!
The text was updated successfully, but these errors were encountered: