fix(node): enforce Frappe-compatible Node version before asset build#98
Open
umairsy wants to merge 1 commit into
Open
fix(node): enforce Frappe-compatible Node version before asset build#98umairsy wants to merge 1 commit into
umairsy wants to merge 1 commit into
Conversation
Getting/building an app on a bench whose Frappe needs Node >= 24 (v16)
failed with a cryptic, deep-stack yarn error ("engine 'node' is
incompatible ... Expected >=24. Got 14.8.0") whenever a stale Node was
already on PATH.
Root cause: install_node() only installed Node `if not which("node")`, so
an existing (old) Node was never upgraded; and the get_app build path never
ensured Node at all.
Fix:
- install_node() now (re)installs when Node is missing OR older than
REQUIRED_NODE_MAJOR (24), and raises a clear, actionable error if a
supported Node still isn't on PATH afterwards.
- macOS install follows `brew install` with `brew upgrade` to move past an
older linked keg.
- build_assets() and build_assets_for_app() (the get_app path) now ensure a
supported Node before building from source; the prebuilt-asset path is
untouched.
Adds unit tests covering skip/upgrade/raise paths and a guard keeping the
nodesource channel in sync with REQUIRED_NODE_MAJOR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
@umairsy thanks for the PR, Can you somehow enforce this during init and only version checks during build (for backward compatibility) since ensure that node is installed during init. bench-cli/bench_cli/commands/init.py Line 94 in 27cee5a Also can you ask Claude to make the code less verbose 😄 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Building an app on a v16 bench fails with an opaque yarn error (
engine "node" is incompatible ... Expected >=24. Got 14.8.0) when a stale Node is already on PATH.Cause
install_node()only installed Nodeif not which("node"), so an existing old Node was never upgraded — and theget_appbuild path never ensured Node at all.Fix
install_node()reinstalls when Node is missing or older thanREQUIRED_NODE_MAJOR(24), and raises a clear, actionable error if it's still too old.brew installwithbrew upgrade nodeto pass an old keg.build_assets()/build_assets_for_app()ensure Node before building from source (prebuilt path untouched).