framework-info icon indicating copy to clipboard operation
framework-info copied to clipboard

Allow frameworks to specify the minimum Node.js version

Open ehmicky opened this issue 4 years ago • 11 comments
trafficstars

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:

  • The value would be a semver range like ^10 || >=12 or >=12.20.0 since some frameworks support Node.js version range that are more complex than just >=version.
  • The minimal version can still be inferred (for example to use as a default Node.js version) using node-semver minVersion() method.

We could use that information to either:

  • Warn users when using the wrong Node.js version, for example by printing a message in the build logs.
  • Choose the default Node.js version run in builds

I would personally favor only warning users and not overriding the Node.js version based on that information because:

  • The logic to decide on the default Node.js version is already quite complicated as mentioned by @ingride
  • Users might not expect that Node.js version unless documented, and documenting it clearly might be difficult if it's framework-specific.
  • When using multiple frameworks, their ranges might be in conflict. This could be fixed by using an intersection, but I am wondering whether that intersection might be empty in some cases.
  • It is consistent with our current strategy for Build plugins. For Build plugins, we not only warn users but fail the build altogether when the Node.js version is incompatible. We do this based on the engines.node field in package.json. This is also consistent with how npm and yarn behave during npm 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!

ehmicky avatar Jun 04 '21 19:06 ehmicky

My concern here is that different versions of frameworks require different versions. Could we read it from the framework's engines.node field, rather than the plugin's?

ascorbic avatar Jun 04 '21 19:06 ascorbic

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

ascorbic avatar Jun 04 '21 19:06 ascorbic

Could we read it from the framework's engines.node field, rather than the plugin's?

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 npmDependencies field, we could use this to extract the engines.node field. Some frameworks like Nuxt are missing that field, so this would require for them to add that field in their package.json, but this might be best practice anyway.

ehmicky avatar Jun 04 '21 19:06 ehmicky

Doesn't npm already prints a warning when you npm install a package that requires a Node.js version that doesn't match the one used? I believe yarn install fails when the version doesn't match (same behavior as npm install --engine-strict).

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.

erezrokah avatar Jun 06 '21 11:06 erezrokah

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.

ascorbic avatar Jun 07 '21 08:06 ascorbic

so we can automatically switch to a supported version

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.

erezrokah avatar Jun 07 '21 10:06 erezrokah

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.

ascorbic avatar Jun 07 '21 11:06 ascorbic

This is a reasonable approach @erezrokah :+1:

One potential gotcha is that some frameworks (like Nuxt) do not have an engines.node field. However, that field is best practice and I do not see a reason why a framework would legitimately not want to add it.

ehmicky avatar Jun 07 '21 12:06 ehmicky

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: https://github.com/netlify/framework-info/pull/553. We do specify an engines.node in @sveltejs/kit's package.json already

benmccann avatar Nov 01 '21 20:11 benmccann

@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)

ascorbic avatar Nov 02 '21 09:11 ascorbic

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.

ascorbic avatar Nov 02 '21 09:11 ascorbic