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.

Sign up for GitHub

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

Jump to bottom

Allow participating buildpacks to opt-out of Node.js build scripts #928

Merged
merged 9 commits into from
Oct 22, 2024
Merged

Allow participating buildpacks to opt-out of Node.js build scripts #928

merged 9 commits into from
Oct 22, 2024

Conversation

colincasey
Copy link
Contributor

The buildpacks that performs installation for each of the supported package managers (npm, pnpm, Yarn) will execute a preset list of scripts from package.json if present:

  • heroku-prebuild
  • heroku-build or build
  • heroku-postbuild

This PR adds a new Buildpack Plan called node_build_scripts to these package manager installation buildpacks that can be configured by later buildpacks that require it. For example, the scripts can be prevented from running with the following:

[[requires]]
name = "node_build_scripts"

  [requires.metadata]
  enabled = false

@colincasey
Allow participating buildpacks to opt-out of Node.js build scripts
0985640 
The buildpacks that performs installation for each of the supported package managers (npm, pnpm, Yarn) will execute a preset list of scripts from `package.json` if present:
- `heroku-prebuild`
- `heroku-build` or `build`
- `heroku-postbuild`

This PR adds a new Buildpack Plan called `node_build_scripts` to the package manager installation buildpacks that can be configured by later buildpacks that require it with the following:

```toml
[[requires]]
name = "node_build_scripts"

  [requires.metadata]
  enabled = <bool>
```
@colincasey colincasey added the enhancement New feature or request label Sep 25, 2024
@colincasey colincasey self-assigned this Sep 25, 2024
@colincasey colincasey requested a review from mars September 25, 2024 18:56
@colincasey
Allow participating buildpacks to opt-out of Node.js build scripts
c1ea345 
The buildpacks that performs installation for each of the supported package managers (npm, pnpm, Yarn) will execute a preset list of scripts from `package.json` if present:
- `heroku-prebuild`
- `heroku-build` or `build`
- `heroku-postbuild`

This PR adds a new Buildpack Plan called `node_build_scripts` to the package manager installation buildpacks that can be configured by later buildpacks that require it with the following:

```toml
[[requires]]
name = "node_build_scripts"

  [requires.metadata]
  enabled = <bool>
```
@colincasey colincasey marked this pull request as ready for review September 25, 2024 19:36
@colincasey colincasey requested a review from a team as a code owner September 25, 2024 19:36
@colincasey colincasey mentioned this pull request Sep 26, 2024
Merged
mars
mars approved these changes Sep 26, 2024
View reviewed changes
Copy link
Member

@mars mars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the new Build Plan docs! This looks great to me, regardless of the Build Plan precedence decision.

common/nodejs-utils/src/buildplan.rs Show resolved Hide resolved
joshwlewis
joshwlewis reviewed Sep 26, 2024
View reviewed changes
Copy link
Member

@joshwlewis joshwlewis left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this is here to prevent double builds, when other processes run npm build inside of the resulting container. I think I actually prefer the double build scenario -- the npm build artifacts end up as part of the image, which means it will run as customers expect without having to use volumes or run another process.

I don't have any issue with the API, really, other than the non-determinism mentioned in my other comment.

@mars
Copy link
Member

mars commented Sep 26, 2024

means it will run as customers expect

Not necessarily… for Static Web Apps, this can mean that the image defaults to serving the wrong or broken version of the site. Ideally with release-build configured, artifacts will only be generated by release-build.

@colincasey
Merge remote-tracking branch 'origin/main' into node_build_scripts_bu…
60a7d66 
…ildplan

# Conflicts:
#	Cargo.lock
#	buildpacks/nodejs-npm-install/CHANGELOG.md
#	buildpacks/nodejs-pnpm-install/CHANGELOG.md
#	buildpacks/nodejs-yarn/CHANGELOG.md
@colincasey
Update Cargo.lock after merge
a8cf7fc
@colincasey colincasey merged commit 0b9fc8c into main Oct 22, 2024
41 checks passed
@colincasey colincasey deleted the node_build_scripts_buildplan branch October 22, 2024 18:58
This was referenced Oct 22, 2024
Merged
Merged
heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request Oct 23, 2024
@heroku-linguist
Update heroku/buildpacks-nodejs to v3.2.16 (#592)
e514177 
## heroku/nodejs

### Changed

- Updated `heroku/nodejs-corepack` to `3.2.16`.
- Updated `heroku/nodejs-engine` to `3.2.16`.
- Updated `heroku/nodejs-npm-engine` to `3.2.16`.
- Updated `heroku/nodejs-npm-install` to `3.2.16`.
- Updated `heroku/nodejs-pnpm-engine` to `3.2.16`.
- Updated `heroku/nodejs-pnpm-install` to `3.2.16`.
- Updated `heroku/nodejs-yarn` to `3.2.16`.

## heroku/nodejs-corepack

- No changes.

## heroku/nodejs-engine

### Added

- 23.0.0 (linux-amd64, linux-arm64)
- 22.10.0 (linux-amd64, linux-arm64)

## heroku/nodejs-function

### Changed

- Updated `heroku/nodejs-engine` to `3.2.16`.
- Updated `heroku/nodejs-function-invoker` to `3.2.16`.
- Updated `heroku/nodejs-npm` to `3.2.16`.

## heroku/nodejs-function-invoker

- No changes.

## heroku/nodejs-npm

- No changes.

## heroku/nodejs-npm-engine

- No changes.

## heroku/nodejs-npm-install

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([#928](heroku/buildpacks-nodejs#928))

## heroku/nodejs-pnpm-engine

- No changes.

## heroku/nodejs-pnpm-install

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([#928](heroku/buildpacks-nodejs#928))

## heroku/nodejs-yarn

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([#928](heroku/buildpacks-nodejs#928))
- Added Yarn version 4.5.1.
- Added Yarn version 3.8.6.

Co-authored-by: heroku-linguist[bot] <136119646+heroku-linguist[bot]@users.noreply.github.com>
mlarraz pushed a commit to mlarraz/buildpacks-nodejs that referenced this pull request Nov 12, 2024
@heroku-linguist
Prepare release v3.2.16 (heroku#944)
ca94d54 
## heroku/nodejs

### Changed

- Updated `heroku/nodejs-corepack` to `3.2.16`.
- Updated `heroku/nodejs-engine` to `3.2.16`.
- Updated `heroku/nodejs-npm-engine` to `3.2.16`.
- Updated `heroku/nodejs-npm-install` to `3.2.16`.
- Updated `heroku/nodejs-pnpm-engine` to `3.2.16`.
- Updated `heroku/nodejs-pnpm-install` to `3.2.16`.
- Updated `heroku/nodejs-yarn` to `3.2.16`.

## heroku/nodejs-corepack

- No changes.

## heroku/nodejs-engine

### Added

- 23.0.0 (linux-amd64, linux-arm64)
- 22.10.0 (linux-amd64, linux-arm64)

## heroku/nodejs-function

### Changed

- Updated `heroku/nodejs-engine` to `3.2.16`.
- Updated `heroku/nodejs-function-invoker` to `3.2.16`.
- Updated `heroku/nodejs-npm` to `3.2.16`.

## heroku/nodejs-function-invoker

- No changes.

## heroku/nodejs-npm

- No changes.

## heroku/nodejs-npm-engine

- No changes.

## heroku/nodejs-npm-install

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([heroku#928](heroku#928))

## heroku/nodejs-pnpm-engine

- No changes.

## heroku/nodejs-pnpm-install

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([heroku#928](heroku#928))

## heroku/nodejs-yarn

### Added

- Allow configuration of build script behavior through the `node_build_scripts` build plan. ([heroku#928](heroku#928))
- Added Yarn version 4.5.1.
- Added Yarn version 3.8.6.

Co-authored-by: heroku-linguist[bot] <136119646+heroku-linguist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mars mars mars approved these changes

@joshwlewis joshwlewis joshwlewis approved these changes

Assignees

@colincasey colincasey

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@colincasey @mars @joshwlewis