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

Speed up npm ci by caching node_modules #45932

Merged
merged 8 commits into from
Mar 16, 2023
Merged

Speed up npm ci by caching node_modules #45932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f4ec7b3
Try custom setup-node composite action for aggressive caching
kevin940726 Nov 21, 2022
239050c
Add comment about GHA bug
kevin940726 Nov 21, 2022
2eff8c8
Try without rebuild and ignore-scripts
kevin940726 Dec 7, 2022
38bca97
Include npm version to the cache key
kevin940726 Dec 7, 2022
4014266
Try reverting the change of graceful-fs
kevin940726 Dec 7, 2022
65efe55
Update step name
kevin940726 Dec 7, 2022
1c9ca27
Code review
kevin940726 Mar 13, 2023
e437750
Add some comments
kevin940726 Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .github/setup-node/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: 'Setup Node.js and install npm dependencies'
description: 'Configure Node.js and install npm dependencies while managing all aspects of caching.'
inputs:
node-version:
description: 'Optional. The Node.js version to use. When not specified, the version specified in .nvmrc will be used.'
required: false
type: string

runs:
using: 'composite'
steps:
- name: Use desired version of Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
node-version: ${{ inputs.node-version }}
cache: npm

- name: Get Node.js and npm version
id: node-version
run: |
echo "NODE_VERSION=$(node -v)" >> $GITHUB_OUTPUT
shell: bash

- name: Cache node_modules
id: cache-node_modules
uses: actions/cache@88522ab9f39a2ea568f7027eddc7d8d8bc9d59c8 # v3.3.1
with:
path: '**/node_modules'
key: node_modules-${{ runner.os }}-${{ steps.node-version.outputs.NODE_VERSION }}-${{ hashFiles('package-lock.json') }}
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved

- name: Install npm dependencies
if: ${{ steps.cache-node_modules.outputs.cache-hit != 'true' }}
run: npm ci
shell: bash
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved

# On cache hit, we run the post-install script to match the native `npm ci` behavior.
# An example of this is to patch `node_modules` using patch-package.
- name: Post-install
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
if: ${{ steps.cache-node_modules.outputs.cache-hit == 'true' }}
run: |
# Run the post-install script for the root project.
npm run postinstall
# Run the post-install scripts for workspaces.
npx lerna run postinstall
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between running postinstall for npm and npx lerna? Do we need both?

If so, I'm wondering if we should split into two steps. That would make it more clear which of the two fails should either encounter a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added some comments explaining this ❤️ .

I don't think we need to split it into two steps though, as they have the same goal.

shell: bash
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions .github/workflows/build-plugin-zip.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ jobs:
with:
ref: ${{ needs.bump-version.outputs.release_branch || github.ref }}

- name: Use desired version of NodeJS
- name: Use desired version of Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
Expand Down Expand Up @@ -326,7 +326,7 @@ jobs:
git config user.name "Gutenberg Repository Automation"
git config user.email [email protected]

- name: Setup Node (for CLI)
- name: Setup Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: 'main/.nvmrc'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bundle-size.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
with:
fetch-depth: 1

- name: Use desired version of NodeJS
- name: Use desired version of Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
Expand Down
13 changes: 5 additions & 8 deletions .github/workflows/create-block.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,17 @@ jobs:
strategy:
fail-fast: false
matrix:
node: [14]
node: ['14']
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why this is needed. Does this cause a failure in the composite workflow because the type is defined as string?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think either works, but everywhere else uses string. Just want to keep it consistent.

os: [macos-latest, ubuntu-latest, windows-latest]

steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node
with:
node-version: ${{ matrix.node }}
cache: npm

- name: npm install, build, format and lint
- name: Create block
shell: bash
run: |
npm ci
bash ./bin/test-create-block.sh
run: bash ./bin/test-create-block.sh
39 changes: 12 additions & 27 deletions .github/workflows/end2end-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,11 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Npm install and build
run: |
npm ci
npm run build
- name: Npm build
Copy link
Contributor

Choose a reason for hiding this comment

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

We're running npm run build, but it's not just an npm build. Maybe "Run build scripts" would be more general and accurate. If we change this, it should get changed everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, let's do this in another PR though.

run: npm run build

- name: Install WordPress
run: |
Expand Down Expand Up @@ -78,16 +73,11 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Npm install and build
run: |
npm ci
npm run build
- name: Npm build
run: npm run build

- name: Install Playwright dependencies
run: |
Expand Down Expand Up @@ -137,19 +127,14 @@ jobs:
name: flaky-tests-report
path: flaky-tests

- name: Use desired version of NodeJS
- name: Setup Node.js and install dependencies
if: ${{ steps.download_artifact.outcome == 'success' }}
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm
uses: ./.github/setup-node

- name: Npm install and build
- name: Npm build
if: ${{ steps.download_artifact.outcome == 'success' }}
# TODO: We don't have to build the entire project, just the action itself.
run: |
npm ci
npm run build:packages
run: npm run build:packages

- name: Report flaky tests
if: ${{ steps.download_artifact.outcome == 'success' }}
Expand Down
11 changes: 2 additions & 9 deletions .github/workflows/performance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,8 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm

- name: Npm install
run: |
npm ci
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Install specific versions of the themes used in tests
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-npm-packages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
git config user.name "Gutenberg Repository Automation"
git config user.email [email protected]

- name: Setup Node (for CLI)
- name: Setup Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: 'main/.nvmrc'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pull-request-automation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
with:
ref: trunk

- name: Use desired version of NodeJS
- name: Use desired version of Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
desrosj marked this conversation as resolved.
Show resolved Hide resolved
with:
node-version: ${{ matrix.node }}
Expand Down
9 changes: 2 additions & 7 deletions .github/workflows/rnmobile-android-runner.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,8 @@ jobs:
distribution: 'temurin'
java-version: '11'

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm

- run: npm ci
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Gradle cache
uses: gradle/gradle-build-action@3fbe033aaae657f011f88f29be9e65ed26bd29ef # v2.3.3
Expand Down
9 changes: 2 additions & 7 deletions .github/workflows/rnmobile-ios-runner.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,8 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm

- run: npm ci
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Prepare build cache key
run: find package-lock.json packages/react-native-editor/ios packages/react-native-aztec/ios packages/react-native-bridge/ios -type f -print0 | sort -z | xargs -0 shasum | tee ios-checksums.txt
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/static-checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
- name: Use desired version of Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
Expand Down
10 changes: 2 additions & 8 deletions .github/workflows/storybook-pages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,8 @@ jobs:
with:
ref: trunk

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm

- name: Install Dependencies
run: npm ci
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Build Storybook
run: npm run storybook:build
Expand Down
37 changes: 12 additions & 25 deletions .github/workflows/unit-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,16 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node
with:
node-version: ${{ matrix.node }}
cache: npm

- name: Npm install and build
- name: Npm build
# It's not necessary to run the full build, since Jest can interpret
# source files with `babel-jest`. Some packages have their own custom
# build tasks, however. These must be run.
run: |
npm ci
npx lerna run build
run: npx lerna run build

- name: Running the tests
run: npm run test:unit -- --ci --maxWorkers=2 --cacheDirectory="$HOME/.jest-cache"
Expand Down Expand Up @@ -79,11 +76,8 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Set up Node.js
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

##
# This allows Composer dependencies to be installed using a single step.
Expand Down Expand Up @@ -116,10 +110,8 @@ jobs:
with:
custom-cache-suffix: $(/bin/date -u --date='last Mon' "+%F")

- name: Install npm dependencies
run: |
npm ci
npm run build
- name: Npm build
desrosj marked this conversation as resolved.
Show resolved Hide resolved
run: npm run build

- name: Docker debug information
run: |
Expand Down Expand Up @@ -235,19 +227,14 @@ jobs:
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0

- name: Use desired version of NodeJS
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
with:
node-version-file: '.nvmrc'
cache: npm
- name: Setup Node.js and install dependencies
uses: ./.github/setup-node

- name: Npm install and build
- name: Npm build
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this seems to be running the Lerna build script.

Suggested change
- name: Npm build
- name: Build Lerna

Copy link
Member Author

@kevin940726 kevin940726 Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

"Build Lerna" isn't really descriptive either though. I think this should be "Build packages" and just run npm run build:packages. But it's already the way it is in trunk, so maybe we should change it in another PR?

# It's not necessary to run the full build, since Jest can interpret
# source files with `babel-jest`. Some packages have their own custom
# build tasks, however. These must be run.
run: |
npm ci
npx lerna run build
run: npx lerna run build

- name: Running the tests
run: npm run native test -- --ci --maxWorkers=2 --cacheDirectory="$HOME/.jest-cache"