feat: upgrade to Node 14 and nw-builder 3.8.3 for GitHub Actions #82
Conversation
- Upgrade Node.js from 12 to 14.21.3 (LTS) - Upgrade nw-builder from 3.5.7 to 3.8.3 (fixes NW.js download) - Add .nvmrc for nvm users - Update yarn.lock with Node 14 compatible dependencies Based on PR emuflight#519 which resolves NW.js download failures
- Add postinstall script to patch nw-builder/lib/downloader.cjs - Set rq.proxy = false to prevent ECONNREFUSED on 127.0.0.1:80 - Fixes 'Unable to download NWjs' error during build
- Apply Codacy linting suggestions - Use double quotes consistently - Remove emoji from console output for better compatibility
- Complete Codacy linting fixes for consistent quote usage
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.github/workflows/build.yml(1 hunks).nvmrc(1 hunks)package.json(2 hunks)scripts/patch-nw-builder.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-2022)
🔇 Additional comments (4)
package.json (2)
12-12: Verify cross-platform compatibility of the postinstall script.The script uses Unix shell syntax (
test -f) which may not work in all Windows environments. While npm typically uses bash-compatible shells even on Windows, this should be tested on Windows to ensure it doesn't fail.Consider a more portable alternative using Node.js directly:
- "postinstall": "test -f scripts/patch-nw-builder.js && node scripts/patch-nw-builder.js || true", + "postinstall": "node -e \"try{require('fs').accessSync('scripts/patch-nw-builder.js');require('child_process').execSync('node scripts/patch-nw-builder.js',{stdio:'inherit'})}catch(e){}\"",Or keep the current approach but test on Windows runners in CI to confirm it works.
86-86: Good practice: exact version pin for patched dependency.Pinning nw-builder to an exact version (removing the
^) is the right approach since the postinstall script patches specific code in this version. This ensures the patch remains compatible and prevents unexpected breakage from minor version updates..nvmrc (1)
1-1: Node version is correct but consider future upgrade path.Node 14.21.3 is the last LTS release of Node 14, which correctly aligns with the CI workflow change. However, as noted in the workflow review, Node 14 reached EOL in April 2023.
The same verification from
.github/workflows/build.ymlapplies here regarding nw-builder compatibility with newer Node.js versions.scripts/patch-nw-builder.js (1)
9-14: Verify the path by checking your installed nw-builder package.The verification could not be completed definitively because the sandbox environment does not have dependencies installed and web searches did not return specific file listings for nw-builder 3.8.3. The script assumes downloader.cjs is at
node_modules/nw-builder/lib/downloader.cjs, but this should be verified locally:ls -la node_modules/nw-builder/lib/ | grep downloaderIf the file exists at that path, the patch script will work correctly. If not, the script will silently exit (as intended), but the patch won't apply—verify this is the intended behavior.
.github/workflows/build.yml
Outdated
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '12' | ||
| node-version: '14' |
There was a problem hiding this comment.
🧩 Analysis chain
Consider Node.js 16 or 18 instead of EOL Node 14.
While upgrading from Node 12 to 14 is necessary, Node.js 14 reached End-of-Life in April 2023 and no longer receives security updates. Unless there are specific compatibility constraints with nw-builder or other dependencies, consider upgrading to Node.js 16 (LTS until September 2023, also EOL) or Node.js 18 (LTS until April 2025).
Search for the latest nw-builder compatibility and Node.js version requirements:
Also note: The workflow uses '14' while .nvmrc specifies 14.21.3. GitHub Actions will install the latest 14.x version available, which should be compatible, but consider aligning them for consistency.
🌐 Web query:
nw-builder 3.8.3 Node.js compatibility requirements
💡 Result:
nw-builder v3 (including 3.8.3) is tested to run on Node.js 14, 16 and 18. [1][2]
Sources:
- npm package (nw-builder v3 stable / readme). [1]
- Project changelog / versions listing showing v3.8.3. [2]
🌐 Web query:
Node.js LTS versions 2025 current
💡 Result:
As of Nov 10, 2025:
- Active LTS: Node.js v24 (entered Active LTS Oct 28, 2025). [1][2]
- Maintenance LTS: Node.js v22 and v20 (both in Maintenance as of late 2025; v22 moved to Maintenance Oct 21, 2025). [2][3]
Sources: Node.js release pages and the Node.js Release WG schedule. [1][2][3]
References
[1] nodejs.org — Releases (about/previous-releases).
[2] nodejs.org — Releases (en/about/previous-releases).
[3] github.com/nodejs/Release — Release schedule.
Upgrade to Node.js 24 LTS; Node.js 14 and recommended fallbacks are EOL.
Node.js 14 reached End-of-Life in April 2023 and no longer receives security updates. nw-builder 3.8.3 supports Node.js 14, 16, and 18, but as of November 2025, Node.js 16 (EOL September 2023) and 18 (EOL April 2025) are also no longer supported. Upgrade to Node.js 24, which entered Active LTS on October 28, 2025, or Node.js 20/22 if additional compatibility constraints exist.
Also: The workflow uses '14' while .nvmrc specifies 14.21.3. Align them for consistency when updating versions.
🤖 Prompt for AI Agents
In .github/workflows/build.yml around line 54, the workflow pins node-version:
'14' which is EOL; update node-version to '24' (or to '22'/'20' if you have
compatibility constraints) so the CI uses a supported LTS, and ensure you update
and align the project's .nvmrc to the exact same Node version string (e.g., "24"
or "24.x"/"24.0.0" as you prefer); also search the workflow for any other Node
version matrices or tools that assume Node 14 and update them accordingly, then
run CI locally/once to validate no compatibility breakages.
scripts/patch-nw-builder.js
Outdated
| let content = fs.readFileSync(downloaderPath, 'utf8'); | ||
| const originalContent = content; | ||
|
|
||
| // Disable proxy auto-detection | ||
| content = content.replace(/rq\.proxy = true;/, 'rq.proxy = false;'); |
There was a problem hiding this comment.
Add error handling and make the regex pattern more robust.
The current implementation has two concerns:
- Missing error handling:
fs.readFileSynccan throw errors (e.g., permission issues, file locked), which would crash the postinstall script. - Fragile regex pattern: The pattern
/rq\.proxy = true;/requires exact spacing and won't match formatting variations likerq.proxy=true;orrq.proxy = true ;.
Apply this diff to improve robustness:
-let content = fs.readFileSync(downloaderPath, 'utf8');
-const originalContent = content;
-
-// Disable proxy auto-detection
-content = content.replace(/rq\.proxy = true;/, 'rq.proxy = false;');
+let content;
+try {
+ content = fs.readFileSync(downloaderPath, 'utf8');
+} catch (error) {
+ console.error('[patch-nw-builder] Failed to read downloader.cjs:', error.message);
+ process.exit(1);
+}
+
+const originalContent = content;
+
+// Disable proxy auto-detection (flexible spacing)
+content = content.replace(/rq\.proxy\s*=\s*true\s*;/, 'rq.proxy = false;');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let content = fs.readFileSync(downloaderPath, 'utf8'); | |
| const originalContent = content; | |
| // Disable proxy auto-detection | |
| content = content.replace(/rq\.proxy = true;/, 'rq.proxy = false;'); | |
| let content; | |
| try { | |
| content = fs.readFileSync(downloaderPath, 'utf8'); | |
| } catch (error) { | |
| console.error('[patch-nw-builder] Failed to read downloader.cjs:', error.message); | |
| process.exit(1); | |
| } | |
| const originalContent = content; | |
| // Disable proxy auto-detection (flexible spacing) | |
| content = content.replace(/rq\.proxy\s*=\s*true\s*;/, 'rq.proxy = false;'); |
🤖 Prompt for AI Agents
In scripts/patch-nw-builder.js around lines 16 to 20, wrap the fs.readFileSync
call in a try/catch so file read errors are handled (log the error and exit with
a non-zero code or rethrow as appropriate), and use a more flexible regex to
match variations of the assignment (for example /\brq\.proxy\s*=\s*true\s*;?/i)
so replacements handle different spacing/casing; after performing the replace,
ensure you still proceed to write the modified content back and handle write
errors similarly.
| if (content !== originalContent) { | ||
| fs.writeFileSync(downloaderPath, content, 'utf8'); | ||
| console.log('[patch-nw-builder] ✓ Disabled proxy auto-detection'); | ||
| } |
There was a problem hiding this comment.
Add error handling for the write operation.
While checking before writing is good practice, fs.writeFileSync can also fail (e.g., permission issues) and should be wrapped in error handling.
Apply this diff:
if (content !== originalContent) {
- fs.writeFileSync(downloaderPath, content, 'utf8');
- console.log('[patch-nw-builder] ✓ Disabled proxy auto-detection');
+ try {
+ fs.writeFileSync(downloaderPath, content, 'utf8');
+ console.log('[patch-nw-builder] ✓ Disabled proxy auto-detection');
+ } catch (error) {
+ console.error('[patch-nw-builder] Failed to write downloader.cjs:', error.message);
+ process.exit(1);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (content !== originalContent) { | |
| fs.writeFileSync(downloaderPath, content, 'utf8'); | |
| console.log('[patch-nw-builder] ✓ Disabled proxy auto-detection'); | |
| } | |
| if (content !== originalContent) { | |
| try { | |
| fs.writeFileSync(downloaderPath, content, 'utf8'); | |
| console.log('[patch-nw-builder] ✓ Disabled proxy auto-detection'); | |
| } catch (error) { | |
| console.error('[patch-nw-builder] Failed to write downloader.cjs:', error.message); | |
| process.exit(1); | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/patch-nw-builder.js around lines 22 to 25, the synchronous file write
fs.writeFileSync(...) is unprotected and can throw (permissions, disk full,
etc.); wrap the write in a try/catch, on error log a descriptive message
including the caught error (console.error) and exit with a non-zero status
(process.exit(1)) or rethrow so failures are visible, while keeping the existing
success console.log when the write completes.
- Replace hardcoded node-version with node-version-file: '.nvmrc' - Ensures CI uses the same Node version as local development - Follows GitHub Actions best practices for version management
…lity - Add try/catch blocks for file read/write operations - Use more flexible regex pattern /\brq\.proxy\s*=\s*true\s*;?/i - Handle variations in spacing, casing, and optional semicolon - Exit with non-zero code on errors for proper CI failure detection
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tements - Suppress 'no-console' rule for build/CI tooling - Console statements are intentional for build transparency - Post-install scripts should report status and errors
- Update matrix.os to use macos-14 for future compatibility - Follows GitHub Actions runner deprecation schedule - No other changes needed for macOS support
Based on PR emuflight#519 which resolves NW.js download failures
Summary by CodeRabbit
Chores