Skip to content

Commit 3eac663

Browse files
vaadin-botZheSun88claude
authored
fix: install pnpm deps in hoisted mode + declare @babel/types (#24288) (CP: 24.10) (#24297)
Cherry-pick of #24288 (commit d9290f8) from main to 24.10. ## 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`) are always reachable from project root, eliminating the `vite-basics` IT hang and similar symptoms. ## Adjustments for 24.10 24.10 is pre-`flow-build-tools` refactor (#23161), so all the Java files the original commit touches (`FrontendTools.java`, `FrontendToolsTest.java`, `NodeUpdaterTest.java`, `TaskRunPnpmInstallTest.java`) live under `flow-server/` here. The edits are otherwise identical. Two version adjustments to match this branch's dep graph: 1. **`@babel/types` pinned to `7.28.5`** (matching `@babel/preset-react`) instead of main's `7.29.0`. 24.10 does not declare `@babel/core`, so version coherence is anchored to `preset-react`. 2. **Skipped `@babel/core` and `@babel/plugin-transform-react-jsx-development`** declarations from the main commit — not declared as direct deps on 24.10, and the React function location plugin only imports `@babel/types`. ## Supersedes #24289 This PR replaces #24289 (the narrower @babel/types-only fix). Once this lands, #24289 should be closed. ## Test plan - [ ] CI on this PR passes (specifically `it-tests (1)` and `it-tests (2)`). - [ ] `mvn -pl flow-server test -Dtest=NodeUpdaterTest,TaskRunPnpmInstallTest,FrontendToolsTest` passes. - [ ] After install in any vite-using IT module, `node_modules/.pnpm/` does not exist (or is empty); `node_modules/@babel/types/` and `node_modules/@lit/reactive-element/` exist as real directories at the project root. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Zhe Sun <zhe@vaadin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 30cf92c commit 3eac663

6 files changed

Lines changed: 20 additions & 3 deletions

File tree

flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendTools.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,13 @@ public List<String> getPnpmExecutable() {
589589
List<String> pnpmCommand = getSuitablePnpm();
590590
assert !pnpmCommand.isEmpty();
591591
pnpmCommand = new ArrayList<>(pnpmCommand);
592-
pnpmCommand.add("--shamefully-hoist=true");
592+
// Force hoisted (flat npm-style) layout. CLI takes precedence over
593+
// .npmrc, so this is unambiguous even if the project lacks the
594+
// generated .npmrc. Replaces the previous --shamefully-hoist=true,
595+
// which only controls the partial-hoist heuristic on top of the
596+
// default isolated layout and did not consistently expose every
597+
// transitive at the project root.
598+
pnpmCommand.add("--config.node-linker=hoisted");
593599
return pnpmCommand;
594600
}
595601

flow-server/src/main/resources/com/vaadin/flow/server/frontend/dependencies/vite/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"@rollup/plugin-replace": "6.0.3",
1616
"@rollup/pluginutils": "5.3.0",
1717
"@babel/preset-react": "7.28.5",
18+
"@babel/types": "7.28.5",
1819
"rollup-plugin-visualizer": "6.0.5",
1920
"rollup-plugin-brotli": "3.1.0",
2021
"vite-plugin-checker": "0.12.0",

flow-server/src/main/resources/npmrc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,11 @@
33
#
44
# This file sets the default parameters for manual `pnpm install`.
55
#
6+
# node-linker=hoisted produces a flat node_modules layout (like npm),
7+
# so every transitive dependency lives at the project root. Flow's
8+
# frontend tooling and bundled Vite plugins (e.g. the React function
9+
# location plugin) import transitive deps that pnpm's symlink-based
10+
# layout did not consistently hoist, even with shamefully-hoist=true.
11+
node-linker=hoisted
612
shamefully-hoist=true
713
strict-peer-dependencies=false

flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendToolsTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,8 +449,10 @@ public void knownFaultyNpmVersionThrowsException() {
449449
@Test
450450
public void getPnpmExecutable_executableIsAvailable() {
451451
List<String> executable = tools.getPnpmExecutable();
452-
// command line should contain --shamefully-hoist=true option
453-
Assert.assertTrue(executable.contains("--shamefully-hoist=true"));
452+
// command line should force hoisted node-linker so transitive
453+
// deps are always installed at the project root
454+
Assert.assertTrue(
455+
executable.contains("--config.node-linker=hoisted"));
454456
Assert.assertTrue(
455457
executable.stream().anyMatch(cmd -> cmd.contains("pnpm")));
456458
}

flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdaterTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ public void getDefaultDevDependencies_includesAllDependencies_whenUsingVite() {
169169
expectedDependencies.add("transform-ast");
170170
expectedDependencies.add("strip-css-comments");
171171
expectedDependencies.add("@babel/preset-react");
172+
expectedDependencies.add("@babel/types");
172173
expectedDependencies.add("@types/react");
173174
expectedDependencies.add("@types/react-dom");
174175
expectedDependencies.add("@preact/signals-react-transform");

flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskRunPnpmInstallTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ public void runPnpmInstall_npmRcFileNotFound_newNpmRcFileIsGenerated()
245245
String content = FileUtils.readFileToString(npmRcFile,
246246
StandardCharsets.UTF_8);
247247
Assert.assertTrue(content.contains("shamefully-hoist"));
248+
Assert.assertTrue(content.contains("node-linker=hoisted"));
248249
}
249250

250251
@Test

0 commit comments

Comments
 (0)