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

Added nodejs-npm-install Buildpack #625

Merged
merged 36 commits into from
Oct 17, 2023
Merged

Added nodejs-npm-install Buildpack #625

merged 36 commits into from
Oct 17, 2023

Conversation

colincasey
Copy link
Contributor

@colincasey colincasey commented Aug 16, 2023 •
edited
Loading

Initial commit of nodejs-npm-install npm buildpack which replaces partial functionality from the existing bash-based npm buildpack.

The nodejs-npm-install buildpack adds support for Heroku's installing and caching an applications node modules. The nodejs-engine buildpack has also been modified to provide npm as a default system version.

Example Buildpack Output

# Heroku npm Install Buildpack

- Installing node modules
  - Using npm version `6.14.18`
  - Creating npm cache
  - Configuring npm cache directory
  - Running `npm ci "--production=false"`

      added 4 packages in 0.866s
            
  - Done (1.245s)
- Running scripts
  - Running `npm run -s heroku-prebuild`

      executed heroku-prebuild
            
  - Done (0.184s)
  - Running `npm run -s build`
     
      executed build
            
  - Done (0.180s)
  - Running `npm run -s heroku-postbuild`

      executed heroku-postbuild
            
  - Done (0.226s)
- Configuring default processes
  - Adding default web process for `npm start`
- Done (finished in 2.310s)

@colincasey
Added nodejs-npm-install Buildpack
d436eb2 
Initial commit of `nodejs-npm-install` npm buildpack which replaces partial functionality from the existing bash-based npm buildpack.

The `nodejs-npm-install` buildpack adds support for Heroku's installing and caching an applications node modules. The `nodejs-engine` buildpack has also been modified to provide npm as a default system version.
@colincasey colincasey added the enhancement New feature or request label Aug 16, 2023
@colincasey colincasey self-assigned this Aug 16, 2023
@colincasey colincasey mentioned this pull request Aug 16, 2023
Merged
joshwlewis
joshwlewis reviewed Aug 17, 2023
View reviewed changes
Copy link
Member

@joshwlewis joshwlewis left a comment

Choose a reason for hiding this comment

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

Looking good so far. Left some questions - mostly food for thought at this point.

buildpacks/nodejs-npm-install/src/main.rs Outdated Show resolved Hide resolved
buildpacks/nodejs-npm-install/src/main.rs Outdated Show resolved Hide resolved
buildpacks/nodejs-npm-install/src/errors.rs Outdated Show resolved Hide resolved
@colincasey
Merge branch 'main' into npm-buildpacks/nodejs-npm-install
88d4903 
* main:
  Bump chrono from 0.4.26 to 0.4.28 (#642)
  Bump serde from 1.0.183 to 1.0.188 (#643)
  Bump url from 2.4.0 to 2.4.1 (#644)
  Bump regex from 1.9.3 to 1.9.4 (#645)
  Skip `mirror-distribution` job if there are no versions (#628)
  Bump Swatinem/rust-cache from 2.5.1 to 2.6.2 (#640)
  Update to libcnb 0.14.0 (#638)
  Update Procfile CNB to v2.0.1 (#639)
  Bump buildpacks/github-actions from 5.3.0 to 5.4.0 (#636)
  Bump regex from 1.9.1 to 1.9.3 (#635)
  Bump serde_json from 1.0.104 to 1.0.105 (#634)
  Bump thiserror from 1.0.44 to 1.0.47 (#633)
  Bump toml from 0.7.5 to 0.7.6 (#632)
  Bump anyhow from 1.0.71 to 1.0.75 (#631)
  Bump tempfile from 3.7.0 to 3.7.1 (#630)
  Bump serde from 1.0.166 to 1.0.168 (#629)
  Ignore yarn 2.4.3 (#627)
  Fix inventory update order (#621)

# Conflicts:
#	Cargo.lock
#	Cargo.toml
@colincasey
Refactoring + output logging and error messages
3597967
@colincasey
Refactoring + output logging and error messages
bbf371c
@colincasey
Refactoring + output logging and error messages
5496b14
@colincasey
Added README.md
40dc2bc
@colincasey
Added new line to buildpack.toml
aca3682
@colincasey
Added ending newline to several files
99c3765
@colincasey
Fix link
2728a94
@colincasey
Fix typo
73ef5ad
@colincasey
Merge branch 'main' into npm-buildpacks/nodejs-npm-install
27473cf 
* main: (31 commits)
  Add `npm` to `nodejs-engine` build plan (#622)
  Remove cutlass job from CI (#675)
  Update Inventory for heroku/nodejs-npm-engine (#624)
  Remove cutlass integration tests (#674)
  Update Inventory for heroku/nodejs-engine (#671)
  Update Inventory for heroku/nodejs-yarn (#670)
  Prepare integration tests to run under a single builder (#663)
  Bump Swatinem/rust-cache from 2.6.2 to 2.7.0 (#665)
  Bump actions/checkout from 3 to 4 (#664)
  Bump regex from 1.9.5 to 1.9.6 (#666)
  Bump thiserror from 1.0.48 to 1.0.49 (#667)
  Bump ureq from 2.7.1 to 2.8.0 (#668)
  Bump serde_json from 1.0.105 to 1.0.107 (#669)
  Fix CNB builder repo URL (#672)
  Fix non-existent builder Docker image name (#673)
  Updates `libcnb` to 0.15.0 (#662)
  Switch tests from `heroku/buildpacks:20` to `heroku/builder:20` (#660)
  Prepare release v1.1.6 (#658)
  Bump regex from 1.9.4 to 1.9.5 (#656)
  Bump indoc from 2.0.3 to 2.0.4 (#657)
  ...

# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	meta-buildpacks/nodejs/buildpack.toml
#	test_support/Cargo.toml
#	test_support/src/lib.rs
@colincasey
Add npm install w/ no package-lock.json
d7b8993
@colincasey
Fix lint error
4b05d74
@colincasey
Don't use -s flag when executing scripts
9912f70
@colincasey
Fix typo in multiple lockfile error and include link
f141c2b
@colincasey
Include text from CX error review
39e20f1
@colincasey
Include text from CX error review
3049052
@colincasey
Merge branch 'main' into npm-buildpacks/nodejs-npm-install
f3a9ff5 
* main:
  Update Inventory for heroku/nodejs-engine (#681)
  Update Inventory for heroku/nodejs-npm-engine (#678)
  Bump the rust-dependencies group with 2 updates (#680)
  Group minor/patch version Rust Dependabot updates into one PR (#679)

# Conflicts:
#	Cargo.lock
@colincasey
Include text from CX error review
cee513a
@colincasey
Fix tests
fe58644
@colincasey colincasey marked this pull request as ready for review October 11, 2023 22:08
@colincasey colincasey requested a review from joshwlewis October 17, 2023 13:22
@colincasey
Merge branch 'main' into npm-buildpacks/nodejs-npm-install
07a67c8
@colincasey
Update buildpack.toml
3588650
@colincasey
Merge branch 'main' into npm-buildpacks/nodejs-npm-install
0099bef 
* main:
  Added `nodejs-npm-engine` Buildpack (#623)

# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	README.md
#	meta-buildpacks/nodejs/buildpack.toml
#	meta-buildpacks/nodejs/package.toml
@colincasey colincasey enabled auto-merge (squash) October 17, 2023 19:48
@colincasey colincasey merged commit fa4b44c into main Oct 17, 2023
21 checks passed
@colincasey colincasey deleted the npm-buildpacks/nodejs-npm-install branch October 17, 2023 20:33
colincasey added a commit that referenced this pull request Oct 18, 2023
@colincasey
Merge branch 'main' into relocate_fixtures
1becb21 
* main:
  Added `nodejs-npm-install` Buildpack (#625)
  Added `nodejs-npm-engine` Buildpack (#623)
  Prepare release v1.1.7 (#683)
  Update Inventory for heroku/nodejs-engine (#682)
  Update Inventory for heroku/nodejs-engine (#681)
  Update Inventory for heroku/nodejs-npm-engine (#678)
  Bump the rust-dependencies group with 2 updates (#680)
  Group minor/patch version Rust Dependabot updates into one PR (#679)
  Add `npm` to `nodejs-engine` build plan (#622)

# Conflicts:
#	buildpacks/nodejs-engine/tests/fixtures/node-with-indexjs/package-lock.json
#	buildpacks/nodejs-engine/tests/fixtures/node-with-serverjs/package-lock.json
#	buildpacks/nodejs-engine/tests/integration_test.rs
@edmorley
Copy link
Member

@colincasey I happened to notice whilst looking at #686 that there are a number of CI jobs not marked as a required in the branch protection status checks list - these are the new CI jobs for the buildpack added here and in #623.

@colincasey
Copy link
Contributor Author

thanks @edmorley. added the 4 new jobs to the required status checks list.

@edmorley edmorley mentioned this pull request Oct 24, 2023
Merged
colincasey added a commit that referenced this pull request Oct 24, 2023
@colincasey
Added missing changelog entries for npm buildpacks
278942d 
The composite buildpack `heroku/nodejs` should have had entries added for the new npm buildpacks merged in PRs:
- #623
- #625
- #685
@colincasey colincasey mentioned this pull request Oct 24, 2023
Merged
colincasey added a commit that referenced this pull request Oct 24, 2023
@colincasey
Added missing changelog entries for npm buildpacks
f126641 
The composite buildpack `heroku/nodejs` should have had entries added for the new npm buildpacks merged in PRs:
- #623
- #625
- #685
colincasey added a commit that referenced this pull request Oct 24, 2023
@colincasey @joshwlewis @edmorley
Added missing changelog entries for npm buildpacks (#697)
ee87816 
* Added missing changelog entries for npm buildpacks

The composite buildpack `heroku/nodejs` should have had entries added for the new npm buildpacks merged in PRs:
- #623
- #625
- #685

* Drop extra hyphen

Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>

---------

Co-authored-by: Josh W Lewis <josh.lewis@salesforce.com>
Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>
@heroku-linguist heroku-linguist bot mentioned this pull request Oct 24, 2023
Merged
heroku-linguist bot added a commit that referenced this pull request Oct 24, 2023
@heroku-linguist @edmorley
Prepare release v2.0.0 (#700)
e5a27d4 
* Prepare release v2.0.0

## heroku/nodejs

### Added

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

### Changed

- Updated buildpack display name, description and keywords. ([#692](#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](#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](#696))

## heroku/nodejs-corepack

### Added

- Added support for using corepack to install npm. ([#685](#685))

### Changed

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

## heroku/nodejs-engine

### Added

- Added Node.js version 21.0.0.

### Changed

- Updated buildpack description and keywords. ([#692](#692))

### Removed

- Dropped support for the end of life `io.buildpacks.stacks.bionic` stack. ([#693](#693))

## heroku/nodejs-function

### Added

- Added `heroku/nodejs-npm-engine` ([#686](#686))
- Added `heroku/nodejs-npm-install` ([#686](#686))

### Removed

- Removed `heroku/nodejs-npm` ([#686](#686))

### Changed

- Updated buildpack display name and description. ([#692](#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](#692))

## heroku/nodejs-npm

### Changed

- Updated buildpack display name, description and keywords. ([#692](#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](#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](#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](#693))

## heroku/nodejs-yarn

### Changed

- Updated buildpack description and keywords. ([#692](#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](#693))

* Replace non-breaking spaces with normal spaces

Yey copy-paste from GitHub PR diffs.

---------

Co-authored-by: heroku-linguist[bot] <136119646+heroku-linguist[bot]@users.noreply.github.com>
Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com>
@heroku-linguist heroku-linguist bot mentioned this pull request Oct 24, 2023
Merged
edmorley
edmorley reviewed Oct 24, 2023
View reviewed changes
buildpacks/nodejs-npm-install/src/main.rs
Comment on lines +37 to +39
let package_json_exists = context
.app_dir
.join(PackageManager::Npm.lockfile())
Copy link
Member

Choose a reason for hiding this comment

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

The variable name is package_json_exists, however the file being checked instead seems to be the lockfile? (PackageManager::Npm.lockfile()). I presume the latter is correct, so the variable name is what needs changing?

heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request Oct 25, 2023
@heroku-linguist @edmorley
Update heroku/buildpacks-nodejs to v2.0.0 (#417)
cd7be2d 
* Update heroku/buildpacks-nodejs to v2.0.0

## heroku/nodejs

### Added

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

### Changed

- Updated buildpack display name, description and keywords. ([#692](heroku/buildpacks-nodejs#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](heroku/buildpacks-nodejs#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/buildpacks-nodejs#696))

## heroku/nodejs-corepack

### Added

- Added support for using corepack to install npm. ([#685](heroku/buildpacks-nodejs#685))

### Changed

- Updated buildpack description and keywords. ([#692](heroku/buildpacks-nodejs#692))
- Switched from supporting explicitly named stacks to supporting the wildcard stack. ([#693](heroku/buildpacks-nodejs#693))

## heroku/nodejs-engine

### Added

- Added Node.js version 21.0.0.

### Changed

- Updated buildpack description and keywords. ([#692](heroku/buildpacks-nodejs#692))

### Removed

- Dropped support for the end of life `io.buildpacks.stacks.bionic` stack. ([#693](heroku/buildpacks-nodejs#693))

## heroku/nodejs-function

### Added

- Added `heroku/nodejs-npm-engine` ([#686](heroku/buildpacks-nodejs#686))
- Added `heroku/nodejs-npm-install` ([#686](heroku/buildpacks-nodejs#686))

### Removed

- Removed `heroku/nodejs-npm` ([#686](heroku/buildpacks-nodejs#686))

### Changed

- Updated buildpack display name and description. ([#692](heroku/buildpacks-nodejs#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/buildpacks-nodejs#692))

## heroku/nodejs-npm

### Changed

- Updated buildpack display name, description and keywords. ([#692](heroku/buildpacks-nodejs#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](heroku/buildpacks-nodejs#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](heroku/buildpacks-nodejs#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](heroku/buildpacks-nodejs#693))

## heroku/nodejs-yarn

### Changed

- Updated buildpack description and keywords. ([#692](heroku/buildpacks-nodejs#692))

### Removed

- Removed redundant explicitly named supported stacks. ([#693](heroku/buildpacks-nodejs#693))

* Add `heroku/procfile` to the `heroku/nodejs` order groups

Since the Procfile CNB is no longer included in the `heroku/nodejs` as of:
heroku/buildpacks-nodejs#696

---------

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

@edmorley edmorley edmorley left review comments

@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 @edmorley @joshwlewis