feat: Install Node.js in version-specific directories with fallback support #22806
+904
−748
Conversation
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
Test Results1 298 files ± 0 1 298 suites ±0 1h 16m 15s ⏱️ +17s Results for commit 0f9c990. ± Comparison against base commit 2d180d7. This pull request removes 2 tests.♻️ This comment has been updated with latest results. |
Artur-
force-pushed
the
version-installed-node
branch
from
eac568b to
ee0f285
Compare
Artur-
changed the title
…upport - Install Node.js to version-specific directories (e.g., node-v24.10.0/) instead of a single node/ directory to support multiple versions - Add fallback logic to use existing compatible Node.js installation when the requested version is not available - Scan install directory for node-v* folders, filter by minimum supported version (24.0), and select the newest compatible version - Make FrontendTools.SUPPORTED_NODE_VERSION public for reuse - Add FileIOUtils.copyDirectory() and delete() methods
Artur-
force-pushed
the
version-installed-node
branch
from
ee0f285 to
c8c808b
Compare
Add ActiveNodeInstallation record to cache the node executable path and version. This ensures node lookup happens only once and the same location is used throughout the build process. Remove redundant validateNodeAndNpmVersion calls since NodeInstaller now ensures we have a compatible node version during resolution.
On Unix systems, npm is installed in lib/node_modules while on Windows it's in node_modules. The getNpmCliScriptPath method was using a hardcoded path that only worked on Windows.
The node.auto.update configuration option is no longer meaningful with version-specific installation folders. Previously, this setting controlled whether to update an existing Node installation in-place when a newer version was available. With versioned folders (e.g., ~/.vaadin/node-v24.10.0/), each version is installed in its own directory, so there is no "update" operation - the requested version is simply installed to its own folder. Remove the setting from: - FrontendTools, FrontendToolsSettings, Options - Constants and InitParameters - Maven plugin (FlowModeAbstractMojo, BuildDevBundleMojo, GenerateNpmBOMMojo) - Gradle plugin (VaadinFlowPluginExtension, GradlePluginAdapter, etc.) - Plugin base (BuildFrontendUtil, ConvertPolymerCommand, PluginAdapterBase) - Related tests and test configurations
When npm/npx couldn't be found in any location (project, global, or alternative dir), getNpmCliToolExecutable would return an empty list. Flags like --no-update-notifier were then added to this empty list, resulting in a malformed command that tried to execute the flag as the program name. Added a fallback that triggers node installation when npm/npx is not found and an alternativeDirGetter is available. After installation, uses the paths from activeNodeInstallation which correctly point to the version-specific node installation directory (e.g., node-v24.10.0) rather than the old non-versioned 'node' directory.
Artur-
force-pushed
the
version-installed-node
branch
from
35be2f6 to
f96f1ee
Compare
Node.js is now only looked up in:
- Global PATH (if version is supported)
- Alternative directory (~/.vaadin) with versioned paths
Removed:
- Project folder node lookup ({baseDir}/node/node)
- Project folder npm lookup
- Unused methods: getExecutable(), getNodeCommands(), getNpmScriptCommand()
- Unused constants: NPM_BIN_PATH, NPM_BIN_LINUX_LEGACY_PATH
Updated tests to work with versioned installation paths and marked
tests that relied on project folder behavior as @ignore.
Artur-
force-pushed
the
version-installed-node
branch
from
8076ac3 to
d9def5a
Compare
Simplified FrontendTools by separating node resolution logic: - Created NodeResolver class to handle one-time node resolution - Made forceAlternativeNode field final (no more mutation) - Added thread-safe resolution with double-checked locking - Simplified getNpmCliToolExecutable from ~110 to ~25 lines - Removed rejectUnsupportedNodeVersion and resolveOrInstallNode methods The node resolution now happens once in a dedicated class, and FrontendTools simply checks the cache and delegates to NodeResolver when empty. This makes the code much easier to understand and maintain.
Added verification that npx-cli.js exists before using it. Previously, the code assumed npx-cli.js would be in the same directory as npm-cli.js without checking, which could lead to failures if the file is missing. Now throws a clear IllegalStateException with the expected path if the file is not found.
Fixed version comparison to handle the 'v' prefix inconsistency. The `node --version` command returns versions without the 'v' prefix (e.g., "24.10.0"), while the nodeVersion field includes it (e.g., "v24.10.0"). Both versions are now normalized by stripping the 'v' prefix before comparison, ensuring exact version matches are detected correctly.
Move all Node.js resolution logic (checking for existing installations, finding fallback versions) from NodeInstaller to NodeResolver. This establishes clear separation of concerns: - NodeResolver: Decides which Node.js version to use - NodeInstaller: Installs the specified version Changes: - Move resolution methods from NodeInstaller to NodeResolver - Remove activeNodeVersion field from NodeInstaller - NodeInstaller.install() now unconditionally installs without checking - Create FrontendToolsTestHelper for test-only installation utilities - Remove obsolete tests that tested resolution behavior in NodeInstaller The NodeInstaller class is now a focused installer that performs installation when requested, while NodeResolver handles all decision making about which version to use.
The forceAlternativeNodeExecutable() method was functionally identical to getNodeExecutable() - both just called ensureNodeResolved().nodeExecutable(). The "force" behavior is already handled by the forceAlternativeNode field passed to NodeResolver, so this method was redundant and had a misleading name. Changes: - Remove forceAlternativeNodeExecutable() method from FrontendTools - Replace all usages with getNodeExecutable() - Simplify getNodeBinary() to always use getNodeExecutable() - Update TaskRunNpmInstall and TaskRunDevBundleBuild to use getNodeExecutable() This is an API break, but keeping a misleading API is worse than breaking it.
Import resetFrontendToolsNodeCache statically instead of using the fully qualified name com.vaadin.flow.testutil.FrontendStubs.
The 'active' local variable was unnecessary. Assign directly to activeNodeInstallation field and return it.
This commit addresses 6 @ignore annotations added in this branch: Deleted 3 obsolete tests: - nodeIsBeingLocated_unsupportedNodeInstalled_*: Project folder node detection was removed in refactoring, these tests are no longer valid - getNpmCacheDir_returnsCorrectPath: Tests baseDir npm which is no longer used (now uses global npm) Fixed and re-enabled 3 tests: - installNodeFromFileSystem_*: Updated prepareNodeDownloadableZipAt() to include npm-cli.js in mock archives for versioned paths - forceHomeNode_useHomeNpmFirst: Updated to use real node installation via FrontendToolsTestHelper instead of stubs with non-versioned paths All flow-server tests now pass (4658 tests, 0 failures).
Artur-
force-pushed
the
version-installed-node
branch
from
18e56c7 to
db9cf94
Compare
Member
Author
|
Maybe it makes sense now |
Critical Issue #1: Global Node.js is now actually used when found and suitable. Previously it was detected but then ignored. Now if a global Node.js installation meets version requirements and has npm, it will be used unless forceAlternativeNode is true. Major Concern #7: Made error messages consistent between npm and npx. When npm-cli.js is not found, the error message now includes the expected path, matching the npx error format. Changes: - Renamed tryFindSuitableGlobalNode() to tryUseGlobalNode() - tryUseGlobalNode() now returns ActiveNodeInstallation instead of File - Added getGlobalNpmCliScript() to locate npm in global installations - Updated createActiveInstallation() to show expected path in error
Added a main() method to FrontendToolsTest that demonstrates which Node.js installation will be used based on configuration. This is useful for manual testing and debugging the Node.js resolution logic. Usage examples: - Test with global node: mvn exec:java -Dexec.mainClass="com.vaadin.flow.server.frontend.FrontendToolsTest" -Dexec.classpathScope=test - Test forcing alternative: mvn exec:java ... -Dalternative=true - Test with custom version: mvn exec:java ... -DnodeVersion=v20.0.0 The method outputs: - Configuration settings (base dir, node version, force alternative) - Resolved node executable path, version, and npm version - Installation type (GLOBAL or ALTERNATIVE) - Verification that the executable works
Updated the FrontendToolsTest.main() method to clearly distinguish between: - Minimum supported version (v24.0.0) - Preferred version (to install if needed, e.g., v24.10.0) - Actual version used (what was resolved, e.g., v24.9.0 from global) This clarifies that the resolution logic will use ANY installed Node.js >= minimum supported version. The preferred version is only installed if no suitable version exists. Example output now shows: Minimum supported version: 24.0.0 Preferred version (to install if needed): v24.10.0 Actual version used: 24.9.0 This makes it clear when an existing installation is used vs when the preferred version would be installed.
Added MAX_SUPPORTED_NODE_MAJOR_VERSION constant (24) to prevent using untested Node.js versions with newer major versions that may not be compatible. Changes: - Added FrontendTools.MAX_SUPPORTED_NODE_MAJOR_VERSION constant - Updated NodeResolver.tryUseGlobalNode() to reject Node.js > v24.x - Updated NodeResolver.findCompatibleInstalledVersion() to skip v25+ - Updated manual test output to show "Supported version range: 24.0.0 - 24.x.x" Example scenarios now handled correctly: - Node v25.2.1 installed → Skipped, installs v24.10.0 instead - Node v24.5.0 installed → Used (within range) - Node v23.1.0 installed → Skipped (too old) This ensures only tested Node.js 24.x versions are used. The constant can be updated in future releases to support newer major versions.
Updated DEFAULT_PNPM_VERSION from 8.6.11 to 10.24.0 (latest stable). This ensures users get the latest pnpm features and improvements when Vaadin installs pnpm automatically.
Introduce MINIMUM_AUTO_INSTALLED_NODE constant to enforce higher version requirements for Node.js installations managed in ~/.vaadin, while keeping lenient requirements for global installations. - Global Node.js: Accepts >= 24.0.0 (SUPPORTED_NODE_VERSION) - ~/.vaadin Node.js: Accepts >= 24.10.0 (MINIMUM_AUTO_INSTALLED_NODE) - Maximum major version: 24 for both This ensures that while we accept any compatible global Node.js installation that users may have, we maintain higher quality standards for our own managed installations.
caalador
requested changes
Nov 28, 2025
flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendTools.java
Outdated
Show resolved
Hide resolved
This reverts commit 659f569.
…error messages With NodeResolver automatically installing a suitable Node.js version to ~/.vaadin when needed, the error messages suggesting manual node installation are no longer applicable. This commit: - Removes INSTALL_NODE_LOCALLY constant and references - Removes node version validation (NodeResolver guarantees suitable version) - Treats npm version mismatches as internal errors since bundled npm should always be compatible - Removes TOO_OLD message and validateToolVersion() method - Simplifies BAD_VERSION message (only used for specific known-bad npm versions like 9.2.0) - Removes tests for deleted validateToolVersion() method Fixes reviewer comment about outdated frontend-maven-plugin installation advice.
caalador
reviewed
Nov 28, 2025
flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendToolsTest.java
Show resolved
Hide resolved
|
caalador
approved these changes
Nov 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Uh oh!
There was an error while loading. Please reload this page.