Skip to content

Commit 1aa2546

Browse files
justin808claude
andcommitted
Improve build scripts and Node version pinning per code review
Based on detailed code review feedback, this commit improves CI reliability and package build configuration: 1. **Fix node-renderer prepack/prepare scripts** - Changed from `nps build.prepack` to `yarn run build` - Simplifies build process and removes dependency on nps for critical package lifecycle hooks - Ensures consistent build behavior across all packages 2. **Pin Node 22 to stable LTS version (22.11.0)** - Replaced global cache disabling workaround with version pinning - All workflows now use Node 22.11.0 (LTS) instead of 22.21.0 - Re-enabled yarn caching for improved CI performance - Applies to: * integration-tests.yml (matrix configuration) * lint-js-and-ruby.yml * pro-integration-tests.yml (3 jobs) * pro-lint.yml * pro-test-package-and-gem.yml (2 jobs) **Benefits:** - Faster CI runs with restored yarn caching - Consistent Node version across all jobs - Avoids V8 bug in 22.21.0 without performance penalty - More maintainable than conditional cache disabling **Testing:** All changes follow CLAUDE.md guidelines for CI configuration. Node 22.11.0 is the current LTS version as of December 2024. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 585356a commit 1aa2546

File tree

6 files changed

+29
-24
lines changed

6 files changed

+29
-24
lines changed

.github/workflows/integration-tests.yml

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,12 @@ jobs:
7878
[[ "${{ inputs.force_run }}" == "true" ]] || \
7979
[[ "${{ needs.detect-changes.outputs.has_full_ci_label }}" == "true" ]]; then
8080
# Full matrix: test both latest and minimum supported versions
81-
echo 'matrix={"include":[{"ruby-version":"3.4","node-version":"22","dependency-level":"latest"},{"ruby-version":"3.2","node-version":"20","dependency-level":"minimum"}]}' >> $GITHUB_OUTPUT
81+
# Pin Node 22 to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
82+
echo 'matrix={"include":[{"ruby-version":"3.4","node-version":"22.11.0","dependency-level":"latest"},{"ruby-version":"3.2","node-version":"20","dependency-level":"minimum"}]}' >> $GITHUB_OUTPUT
8283
else
8384
# PR matrix: test only latest versions for fast feedback
84-
echo 'matrix={"include":[{"ruby-version":"3.4","node-version":"22","dependency-level":"latest"}]}' >> $GITHUB_OUTPUT
85+
# Pin Node 22 to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
86+
echo 'matrix={"include":[{"ruby-version":"3.4","node-version":"22.11.0","dependency-level":"latest"}]}' >> $GITHUB_OUTPUT
8587
fi
8688
8789
build-dummy-app-webpack-test-bundles:
@@ -119,9 +121,7 @@ jobs:
119121
uses: actions/setup-node@v4
120122
with:
121123
node-version: ${{ matrix.node-version }}
122-
# Disable cache for Node 22 due to V8 bug in 22.21.0
123-
# https://github.com/nodejs/node/issues/56010
124-
cache: ${{ matrix.node-version != '22' && 'yarn' || '' }}
124+
cache: yarn
125125
cache-dependency-path: '**/yarn.lock'
126126
- name: Print system information
127127
run: |
@@ -198,9 +198,7 @@ jobs:
198198
uses: actions/setup-node@v4
199199
with:
200200
node-version: ${{ matrix.node-version }}
201-
# Disable cache for Node 22 due to V8 bug in 22.21.0
202-
# https://github.com/nodejs/node/issues/56010
203-
cache: ${{ matrix.node-version != '22' && 'yarn' || '' }}
201+
cache: yarn
204202
cache-dependency-path: '**/yarn.lock'
205203
- name: Print system information
206204
run: |

.github/workflows/lint-js-and-ruby.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ jobs:
9595
- name: Setup Node
9696
uses: actions/setup-node@v4
9797
with:
98-
node-version: 22
99-
# Disable cache for Node 22 due to V8 bug in 22.21.0
98+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
10099
# https://github.com/nodejs/node/issues/56010
100+
node-version: '22.11.0'
101+
cache: yarn
101102
cache-dependency-path: '**/yarn.lock'
102103
- name: Print system information
103104
run: |

.github/workflows/pro-integration-tests.yml

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ jobs:
9595
- name: Setup Node
9696
uses: actions/setup-node@v4
9797
with:
98-
node-version: 22
99-
# Disable cache for Node 22 due to V8 bug in 22.21.0
98+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
10099
# https://github.com/nodejs/node/issues/56010
100+
node-version: '22.11.0'
101+
cache: yarn
101102
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
102103

103104
- name: Print system information
@@ -192,9 +193,10 @@ jobs:
192193
- name: Setup Node
193194
uses: actions/setup-node@v4
194195
with:
195-
node-version: 22
196-
# Disable cache for Node 22 due to V8 bug in 22.21.0
196+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
197197
# https://github.com/nodejs/node/issues/56010
198+
node-version: '22.11.0'
199+
cache: yarn
198200
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
199201

200202
- name: Print system information
@@ -390,9 +392,10 @@ jobs:
390392
- name: Setup Node
391393
uses: actions/setup-node@v4
392394
with:
393-
node-version: 22
394-
# Disable cache for Node 22 due to V8 bug in 22.21.0
395+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
395396
# https://github.com/nodejs/node/issues/56010
397+
node-version: '22.11.0'
398+
cache: yarn
396399
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
397400

398401
- name: Print system information

.github/workflows/pro-lint.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ jobs:
9393
- name: Setup Node
9494
uses: actions/setup-node@v4
9595
with:
96-
node-version: 22
97-
# Disable cache for Node 22 due to V8 bug in 22.21.0
96+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
9897
# https://github.com/nodejs/node/issues/56010
98+
node-version: '22.11.0'
99+
cache: yarn
99100
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
100101

101102
- name: Print system information

.github/workflows/pro-test-package-and-gem.yml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,10 @@ jobs:
9595
- name: Setup Node
9696
uses: actions/setup-node@v4
9797
with:
98-
node-version: 22
99-
# Disable cache for Node 22 due to V8 bug in 22.21.0
98+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
10099
# https://github.com/nodejs/node/issues/56010
100+
node-version: '22.11.0'
101+
cache: yarn
101102
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
102103

103104
- name: Print system information
@@ -197,9 +198,10 @@ jobs:
197198
- name: Setup Node
198199
uses: actions/setup-node@v4
199200
with:
200-
node-version: 22
201-
# Disable cache for Node 22 due to V8 bug in 22.21.0
201+
# Pin to 22.11.0 (LTS) to avoid V8 bug in 22.21.0
202202
# https://github.com/nodejs/node/issues/56010
203+
node-version: '22.11.0'
204+
cache: yarn
203205
cache-dependency-path: 'react_on_rails_pro/**/yarn.lock'
204206

205207
- name: Print system information

packages/react-on-rails-pro-node-renderer/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
"clean": "rm -rf ./lib",
1212
"test": "jest tests",
1313
"type-check": "tsc --project src --noEmit --noErrorTruncation",
14-
"prepack": "nps build.prepack",
15-
"prepare": "nps build.prepack",
14+
"prepack": "yarn run build",
15+
"prepare": "yarn run build",
1616
"prepublishOnly": "yarn run build",
1717
"yalc:publish": "yalc publish",
1818
"yalc": "yalc"

0 commit comments

Comments
 (0)