Update heroku/buildpacks-nodejs to v2.0.0

[heroku-linguist]

heroku/nodejs

Added

• Added heroku/nodejs-npm-engine to the buildpack group for npm support (#623)
• Added heroku/nodejs-npm-install to the buildpack group for npm support (#625)
• Added heroku/nodejs-corepack to the buildpack group for npm support (#685)

Changed

• Updated buildpack display name, description and keywords. (#692)
• Updated heroku/nodejs-corepack to 2.0.0.
• Updated heroku/nodejs-engine to 2.0.0.
• Updated heroku/nodejs-npm-engine to 2.0.0.
• Updated heroku/nodejs-npm-install to 2.0.0.
• Updated heroku/nodejs-pnpm-install to 2.0.0.
• Updated heroku/nodejs-yarn to 2.0.0.

Removed

• Removed the deprecated heroku/nodejs-npm from the buildpack group for npm support (#625)
• Removed heroku/procfile, since it's being added directly to the Heroku builder images instead. If you override the Heroku builder images' default buildpack detection order (or use this buildpack with a non-Heroku builder image), you will need to append heroku/procfile to your buildpacks list. (#696)

heroku/nodejs-corepack

Added

• Added support for using corepack to install npm. (#685)

Changed

• Updated buildpack description and keywords. (#692)
• Switched from supporting explicitly named stacks to supporting the wildcard stack. (#693)

heroku/nodejs-engine

Added

• Added Node.js version 21.0.0.

Changed

• Updated buildpack description and keywords. (#692)

Removed

• Dropped support for the end of life io.buildpacks.stacks.bionic stack. (#693)

heroku/nodejs-function

Added

• Added heroku/nodejs-npm-engine (#686)
• Added heroku/nodejs-npm-install (#686)

Removed

• Removed heroku/nodejs-npm (#686)

Changed

• Updated buildpack display name and description. (#692)
• Updated heroku/nodejs-engine to 2.0.0.
• Updated heroku/nodejs-function-invoker to 2.0.0.
• Updated heroku/nodejs-npm-engine to 2.0.0.
• Updated heroku/nodejs-npm-install to 2.0.0.

heroku/nodejs-function-invoker

Changed

• Updated buildpack display name, description and keywords. (#692)

heroku/nodejs-npm

Changed

• Updated buildpack display name, description and keywords. (#692)

Removed

• Removed redundant explicitly named supported stacks. (#693)

heroku/nodejs-npm-engine

Added

• Initial release

heroku/nodejs-npm-install

Added

• Initial release

heroku/nodejs-pnpm-install

Changed

• Updated buildpack description and keywords. (#692)

Removed

• Removed redundant explicitly named supported stacks. (#693)

heroku/nodejs-yarn

Changed

• Updated buildpack description and keywords. (#692)

Removed

• Removed redundant explicitly named supported stacks. (#693)

[edmorley]

@colincasey @joshwlewis CI is failing due to the Node.js getting started guide not passing buildpack detection:
https://github.com/heroku/cnb-builder-images/actions/runs/6632101496/job/18017129692?pr=417#step:6:37

There's no log output explaining why detect failed, but I believe this is because:

• the Node.js getting started guide doesn't have a lockfile: https://github.com/heroku/node-js-getting-started
• the new NPM install buildpack now requires a lockfile: https://github.com/heroku/buildpacks-nodejs/blob/e5a27d41dcf26226bdee996d7a0515c7fe326315/buildpacks/nodejs-npm-install/src/main.rs#L37-L41

Initial thoughts:

• A lockfile probably needs to be added to the getting started guide (whilst that means another thing that has to be kept up to date with Dependabot, if lockfiles are now required, it seems unavoidable?)
• It may be worth adding the getting started guide as an integration test to the Node.js CNB repo, so any issues are caught before the release PR is opened against the builder. (The JVM repo uses Git submodules to achieve this to save having to vendor.)
• Some log output explaining detect fail (added here) would help a lot with debugging. (Similar to what Python and PHP do - example. For bonus points this would be tested via an integration test that has no lockfile.)
• There's no mention of lockfiles now being mandatory in changelogs - perhaps worth adding an entry to the changelog for heroku/nodejs since the composite buildpack is the level at which detect ultimately passes or not?

[colincasey]

A lockfile probably needs to be added to the getting started guide (whilst that means another thing that has to be kept up to date with Dependabot, if lockfiles are now required, it seems unavoidable?)

This is fine. We shouldn't let the guides fall too far behind with dependency updates.

It may be worth adding the getting started guide as an integration test to the Node.js CNB repo, so any issues are caught before the release PR is opened against the builder. (The JVM repo uses Git submodules to achieve this to save having to vendor.)

Agreed. That would be a good check to have earlier in the development process.

Some log output explaining detect fail (added here) would help a lot with debugging. (Similar to what Python and PHP do - example. For bonus points this would be tested via an integration test that has no lockfile.)

This one is a bit tricky due to the way our buildpacks are structured into groups. Or maybe I'm misunderstanding how this will interact with detection. If you're using yarn tooling instead of npm, will the warning about missing a package-lock.json file show up always?

There's no mention of lockfiles now being mandatory in changelogs - perhaps worth adding an entry to the changelog for heroku/nodejs since the composite buildpack is the level at which detect ultimately passes or not?

It's documented in the npm install buildpack README. There is a changelog entry for the replacement of the previous npm buildpack with the new one so it's kind of implied but calling out some of these changes in behavior is probably warranted.

[edmorley]

This one is a bit tricky due to the way our buildpacks are structured into groups.

Yeah I agree this is harder when using micro-buildpacks (it's one of the many reasons I chose the mono-buildpack approach for Python). However, I think we could still output some logs to stdout from each buildpack - they are only shown when using --verbose or when detection fails (which prints them even when not using verbose).

For example, it would look something like:

======== Output: heroku/nodejs-yarn@2.0.0 ========
No Yarn lockfile found (yarn.lock).
======== Results ========
pass: heroku/nodejs-engine@2.0.0
skip: heroku/nodejs-corepack@2.0.0
fail: heroku/nodejs-yarn@2.0.0
======== Output: heroku/nodejs-npm-install@2.0.0 ========
No NPM lockfile found (package-lock.json).
======== Results ========
pass: heroku/nodejs-engine@2.0.0
skip: heroku/nodejs-corepack@2.0.0
skip: heroku/nodejs-npm-engine@2.0.0
fail: heroku/nodejs-npm-install@2.0.0

[edmorley]

@colincasey @joshwlewis Ah so there are two other failures in CI:

1. In the typescript getting started guide (https://github.com/heroku/typescript-getting-started) since it also doesn't have a lockfile
2. In the example JavaScript and TypeScript Salesforce Functions (vendored in this repo), since they don't have lockfiles either

For (1), that guide is really old - perhaps we should just remove it from CI for this repo? We don't test our classic buildpacks against it for example.

For (2), how do you want to proceed? The template functions don't ship with lockfiles (eg: https://github.com/heroku/sf-functions-core/tree/main/tmpl/javascript). It's likely that customers will have had a lockfile be generated when they ran the function locally, and then ended up committing that file - however, I imagine there will be at least some users that haven't - and would need a warning message or something to hint at what's changed.

One option would be to modify the new npm-install buildpack, so it passes detection even if there isn't a lockfile (so long as there is a package.json), and then error during the build instead? Since the NPM group is last, this would still work, although the error message would have to be made slightly generic - ie: when mentioning "Error: A lockfile is required" it would need to talk about Yarn / pnpm lockfiles too and not just NPM.

[colincasey]

For (1), that guide is really old - perhaps we should just remove it from CI for this repo? We don't test our classic buildpacks against it for example.

I wasn't even aware of the Typescript getting started guide. I don't think it was even in the list when we last did a guides & docs refresh. I'm in favour of dropping that from the tests. There's no special treatment of Typescript projects beyond the standard Node.js setup that would require a separate check.

In the example JavaScript and TypeScript Salesforce Functions (vendored in this repo), since they don't have lockfiles either

Let me add those. There's no reason they shouldn't have a lockfile.

The template functions don't ship with lockfiles (eg: https://github.com/heroku/sf-functions-core/tree/main/tmpl/javascript). It's likely that customers will have had a lockfile be generated when they ran the function locally, and then ended up committing that file - however, I imagine there will be at least some users that haven't - and would need a warning message or something to hint at what's changed.

Yes, the templates wouldn't have one but a lockfile will be generated since the customer needs to run npm install before running the function. @joshwlewis and I discussed the possibility of a customer not having a lockfile committed to git and if we should add special logic to handle that case but decided against it because:

• Adding a lockfile is considered best practice. Especially for an application which you'd like to deploy without worrying about surprise dependency updates.
• The workaround if a customer encounters this scenario would be to simply check in their lockfile.

[edmorley]

I've opened #421 to remove the TypeScript getting started guide from CI.

Colin opened #418 for adding lockfiles to the functions test fixtures.