Skip to content

Commit b9b5632

Browse files
vaadin-botZheSun88claude
authored
fix: install pnpm deps in hoisted mode + declare @babel/types (#24288) (CP: 25.0) (#24296)
Cherry-pick of #24288 (commit d9290f8) from main to 25.0. ## 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 25.0 This branch 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` already in `vite/package.json`) instead of main's `7.29.0`. 25.0 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 25.0, and the React function location plugin only imports `@babel/types`. Everything else is identical: - `flow-server/.../npmrc`: adds `node-linker=hoisted`. - `FrontendTools.java`: replaces `--shamefully-hoist=true` CLI flag with `--config.node-linker=hoisted`. - All three test files updated to match the new assertions. ## 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 2142a79 commit b9b5632

6 files changed

Lines changed: 19 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
@@ -362,7 +362,13 @@ public List<String> getPnpmExecutable() {
362362
List<String> pnpmCommand = getSuitablePnpm();
363363
assert !pnpmCommand.isEmpty();
364364
pnpmCommand = new ArrayList<>(pnpmCommand);
365-
pnpmCommand.add("--shamefully-hoist=true");
365+
// Force hoisted (flat npm-style) layout. CLI takes precedence over
366+
// .npmrc, so this is unambiguous even if the project lacks the
367+
// generated .npmrc. Replaces the previous --shamefully-hoist=true,
368+
// which only controls the partial-hoist heuristic on top of the
369+
// default isolated layout and did not consistently expose every
370+
// transitive at the project root.
371+
pnpmCommand.add("--config.node-linker=hoisted");
366372
return pnpmCommand;
367373
}
368374

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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,9 @@ public void knownFaultyNpmVersionThrowsException() {
363363
@Test
364364
public void getPnpmExecutable_executableIsAvailable() {
365365
List<String> executable = tools.getPnpmExecutable();
366-
// command line should contain --shamefully-hoist=true option
367-
Assert.assertTrue(executable.contains("--shamefully-hoist=true"));
366+
// command line should force hoisted node-linker so transitive
367+
// deps are always installed at the project root
368+
Assert.assertTrue(executable.contains("--config.node-linker=hoisted"));
368369
Assert.assertTrue(
369370
executable.stream().anyMatch(cmd -> cmd.contains("pnpm")));
370371
}

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
@@ -162,6 +162,7 @@ public void getDefaultDevDependencies_includesAllDependencies_whenUsingVite() {
162162
expectedDependencies.add("transform-ast");
163163
expectedDependencies.add("strip-css-comments");
164164
expectedDependencies.add("@babel/preset-react");
165+
expectedDependencies.add("@babel/types");
165166
expectedDependencies.add("@types/react");
166167
expectedDependencies.add("@types/react-dom");
167168
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
@@ -225,6 +225,7 @@ public void runPnpmInstall_npmRcFileNotFound_newNpmRcFileIsGenerated()
225225
String content = FileUtils.readFileToString(npmRcFile,
226226
StandardCharsets.UTF_8);
227227
Assert.assertTrue(content.contains("shamefully-hoist"));
228+
Assert.assertTrue(content.contains("node-linker=hoisted"));
228229
}
229230

230231
@Test

0 commit comments

Comments
 (0)