fix: install pnpm deps in hoisted mode + declare @babel/types (#24288) (CP: 25.1)#24295
Merged
fix: install pnpm deps in hoisted mode + declare @babel/types (#24288) (CP: 25.1)#24295
Conversation
… (CP: 25.1) Cherry-pick of d9290f8 from main. Adjusted for 25.1's dep-graph: @babel/types is pinned to 7.28.5 to match @babel/preset-react's version on this branch (main pins to 7.29.0 to match @babel/core which is not declared on 25.1). @babel/core and @babel/plugin-transform-react-jsx-development from the main commit are not added — they are not declared as direct deps on 25.1 and the React function location plugin only imports @babel/types. The .npmrc and FrontendTools changes that switch pnpm to hoisted mode apply unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vaadin-review-bot
previously approved these changes
May 8, 2026
Collaborator
Author
|
This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR [Message is sent from bot] |
## Summary Validation runs occasionally lose ~30 minutes on `it-tests (11, ...)` because the step runs to the job timeout instead of finishing — costing a full re-run attempt to produce green. Example: https://github.com/vaadin/flow/actions/runs/25435931074/job/74614298073 (attempt 1 cancelled at 30:00, attempt 2 succeeded.) ## Current behavior (root cause) The `flow-test-redeployment` module starts its app via `spring-boot-maven-plugin:start` (pre-IT) and stops it via `:stop` (post-IT). Spring DevTools is on the classpath and the test churns class files (touching `Application.class` to trigger reloads), which in this run produced **8 DevTools-driven restarts in 20 seconds**, the last firing 442 ms before `spring-boot:stop` ran: 12:47:56.241 [File Watcher] Restarting due to 1 class path change ... 12:47:56.683 [INFO] --- spring-boot-maven-plugin:4.1.0-RC1:stop ... [ERROR] Spring application lifecycle JMX bean not found. Could not stop application gracefully: org.springframework.boot:type=Admin,name=SpringApplication `stop` queries the admin MBean exactly once and there is no retry; during a restart the bean is briefly unregistered, so the call fails. The forked Spring Boot JVM (PID 3969 in this run) is therefore **never killed**. Maven moves on under `-fae`, finishes 16 more modules, and exits with BUILD FAILURE — but the orphan JVM still holds the write end of the pipe inherited from `mvn ... | tee -a mvn-it-tests-11.out` (`.github/workflows/validation.yml`). `tee` cannot reach EOF, the bash pipeline cannot complete, and the job hangs until GitHub force-cancels at the 30-min job timeout. The runner cleanup confirms the orphan: `Terminate orphan process: pid (3969) (java)`. ## Fix Two small, independent changes: 1. **`flow-tests/test-redeployment/pom.xml`** — insert a maven-antrun-plugin execution bound to post-integration-test (declared before spring-boot-maven-plugin so it runs first in that phase) that sleeps 5 seconds before spring-boot:stop runs. This module's tests deliberately trigger DevTools restarts; the original failure was a 442 ms race between the last restart and stop-server querying the admin JMX bean. The 5-second quiet period lets DevTools finish re-registering the bean so stop-server can shut the app down cleanly instead of leaving an orphan JVM behind. 2. **`.github/workflows/validation.yml`** — add `timeout-minutes: 22` on the `Run ITs` step (the job-level 30-min cap stays as the outer fence). If a future plugin produces a similar shape of hang, the step now fails after 22 min instead of 30, leaving room within the original job budget for report packaging, artifact upload, and the existing `run_attempt > 1` retry path. 3. **`.github/workflows/validation.yml`** — replace mvn ... | tee ...out with file redirection (>"$LOG" 2>&1 + tail -f --pid) so an orphan child JVM holding an inherited stdout fd can no longer pin the step open. Add a follow-up Kill leftover Java processes step (if: always()) that SIGTERM/SIGKILLs any remaining java processes before report packaging — defensive cleanup against any future plugin that leaks a JVM.
vaadin-review-bot
approved these changes
May 8, 2026
Collaborator
Author
|
This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR[Message is sent from bot] |
|
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
3 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.
Cherry-pick of #24288 (commit d9290f8) from main to 25.1.
Summary
Same fix as #24288: switches pnpm install to hoisted (flat npm-style)
layout so transitive npm deps (
@babel/types,@lit/reactive-element,cookie,set-cookie-parser,@preact/signals-react/runtime) arealways reachable from project root, eliminating the
vite-basicsIThang and similar symptoms.
Adjustments for 25.1
The cherry-pick required two small adjustments to match this branch's
dep graph:
@babel/typespinned to7.28.5(matching@babel/preset-reactalready in
vite/package.json) instead of main's7.29.0. 25.1does not declare
@babel/core, so version coherence is anchored topreset-react.Skipped
@babel/coreand@babel/plugin-transform-react-jsx-developmentdeclarations from the main commit. These are not declared as direct
deps on 25.1, and the React function location plugin
(
flow-build-tools/.../react-function-location-plugin.js) onlyimports
@babel/types.Everything else is identical:
flow-server/.../npmrc: addsnode-linker=hoisted.flow-build-tools/.../FrontendTools.java: replaces--shamefully-hoist=trueCLI flag with--config.node-linker=hoisted.NodeUpdaterTest,TaskRunPnpmInstallTest,FrontendToolsTestupdated to match.
Test plan
it-tests (1)andit-tests (2)).mvn -pl flow-build-tools test -Dtest=NodeUpdaterTest,TaskRunPnpmInstallTest,FrontendToolsTestpasses.node_modules/.pnpm/does not exist (or is empty);node_modules/@babel/types/andnode_modules/@lit/reactive-element/exist as real directories at the project root.🤖 Generated with Claude Code