Conversation
Even with shamefully-hoist=true, pnpm's symlink-based layout does not consistently hoist every transitive dependency to the project root. Flow's frontend tooling and bundled Vite plugins (e.g. the React function location plugin) import transitive deps directly — when one of those is left only inside .pnpm/..., Node ESM cannot resolve it, Vite fails or returns broken modules, and Selenium tests hang waiting for pages that never render. The previous commit pinned @babel/types explicitly to fix that one case; missing-dep failures kept appearing on different transitives (cookie, set-cookie-parser, @lit/reactive- element, @preact/signals-react) as the dep graph shifts. Switch the bundled .npmrc template to node-linker=hoisted, which makes pnpm install in flat npm-style mode: every package lives at the project root, no .pnpm/ virtual store. All transitive resolutions become deterministic. Slightly larger node_modules and slower install (seconds, not minutes), but eliminates this whole class of flake. shamefully-hoist=true is left in the template since it's now redundant in hoisted mode but harmless, and removing it would change behavior for users with custom .npmrc files that lack node-linker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ZheSun88
changed the title
Setting node-linker=hoisted in .npmrc alone did not flip pnpm's actual install layout; node_modules/.pnpm/ kept appearing in CI runs and transitive deps stayed unreachable from the project root. The --shamefully-hoist=true CLI flag added by getPnpmExecutable() likely took precedence and locked pnpm to the isolated layout (where shamefully-hoist makes sense as a partial-hoist heuristic). Replace --shamefully-hoist=true with --config.node-linker=hoisted on the CLI. CLI > .npmrc > defaults in pnpm's config precedence, so this forces flat npm-style install regardless of project-level config. The .npmrc still keeps shamefully-hoist=true and node-linker=hoisted for manual pnpm invocations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Artur-
reviewed
May 8, 2026
Member
Artur-
left a comment
Artur-
left a comment
There was a problem hiding this comment.
Looks like a good workaround but why do we need to add the same parameter to both npmrc and the command?
Artur-
approved these changes
May 8, 2026
Member
|
Let's simplify in another PR - this unblocks everything else |
Artur-
pushed a commit
that referenced
this pull request
May 8, 2026
## Summary
The Vite dev server intermittently fails (or hangs) on `vite-basics` and
other Vite-using IT modules because transitive npm dependencies are not
consistently reachable from the project's root `node_modules`. Selenium
tests then wait for pages that never render, and the matrix shard times
out at the 30-minute job cap.
Two distinct symptoms observed:
1. **`Cannot find package '@babel/types'`** — the bundled Vite plugin
`react-function-location-plugin.js` does
`import * as t from '@babel/types';`, but `@babel/types` was only
present transitively via `@babel/core` and pnpm did not always hoist
it to the root.
2. **`Failed to resolve import "@lit/reactive-element/..."`,
`"cookie"`, `"set-cookie-parser"`, `"@preact/signals-react/runtime"`** —
all transitives of declared deps (`lit`, `react-router`,
`@preact/signals-react-transform`) — also not consistently hoisted.
Both are the same underlying problem: pnpm with `shamefully-hoist=true`
does not deterministically expose every transitive at the project root.
## Fix
Three commits, layered:
### 1. Declare `@babel/types` directly
Adds `"@babel/types": "7.29.0"` (matching `@babel/core`) to the bundled
`devDependencies` in
`flow-server/.../dependencies/vite/package.json`. Correct on its own
merits — the plugin imports the package, so the package should be a
real dependency. Updates `NodeUpdaterTest`'s expected-set accordingly.
### 2. Switch the bundled `.npmrc` template to `node-linker=hoisted`
Adds `node-linker=hoisted` to `flow-server/src/main/resources/npmrc`,
the template `TaskRunNpmInstall` writes to the project. This tells
pnpm to install in **flat npm-style mode** — every package (declared
or transitive) at the project root, no `.pnpm/` virtual store.
Resolutions become deterministic.
`shamefully-hoist=true` is left in the template alongside, so manual
`pnpm install` invocations from a developer shell still pick up at
least the legacy partial-hoist behavior if some user-level config
overrides our `node-linker`.
### 3. Pass `--config.node-linker=hoisted` on the pnpm CLI
The `.npmrc` change alone did not actually flip pnpm's install layout
in CI: `node_modules/.pnpm/...` paths kept appearing in error
messages, indicating pnpm was still using its default `isolated`
mode. The pre-existing `--shamefully-hoist=true` CLI flag added by
`FrontendTools.getPnpmExecutable()` likely took precedence and locked
pnpm to the isolated layout (where `shamefully-hoist` makes sense as
a partial-hoist heuristic).
Replace `--shamefully-hoist=true` with `--config.node-linker=hoisted`
on the CLI. CLI > `.npmrc` > defaults in pnpm's config precedence, so
the install is now unambiguously hoisted regardless of any project-
or user-level `.npmrc` content. `FrontendToolsTest` updated to assert
the new flag.
## Trade-offs
- **Disk/install time**: hoisted mode uses slightly more disk and is
a few seconds slower than the symlink store for fresh installs.
Acceptable for the determinism gain.
- **Loose isolation**: hoisted mode allows packages to import any
other package, declared or not. Flow's frontend code already relies
on this (e.g. plugins importing transitives), so this is the
current de-facto behavior — just made deterministic.
- **Behavioral change for downstream users**: anyone with a custom
`.npmrc` that lacks the auto-gen marker keeps their config (per
existing `TaskRunNpmInstall.createNpmRcFile()` logic), but the new
CLI flag still applies — they'll silently get hoisted mode for
Flow-driven installs. Worth flagging in release notes.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 8, 2026
ZheSun88
added a commit
that referenced
this pull request
May 8, 2026
… (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>
ZheSun88
added a commit
that referenced
this pull request
May 8, 2026
… (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>
ZheSun88
added a commit
that referenced
this pull request
May 8, 2026
… (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>
ZheSun88
added a commit
that referenced
this pull request
May 8, 2026
… (CP: 24.9) (#24298) Cherry-pick of #24288 (commit d9290f8) from main to 24.9. ## 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.9 24.9 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.27.1`** (matching `@babel/preset-react`) instead of main's `7.29.0`. 24.9 does not declare `@babel/core`, so version coherence is anchored to `preset-react` — which on 24.9 is on the older `7.27.x` line. 2. **Skipped `@babel/core` and `@babel/plugin-transform-react-jsx-development`** declarations from the main commit — not declared as direct deps on 24.9, and the React function location plugin only imports `@babel/types`. ## 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>
Contributor
Author
|
related conversation in slack https://vaadin.slack.com/archives/C6RAXJATF/p1778162634388459 |
ZheSun88
added a commit
that referenced
this pull request
May 12, 2026
… (CP: 23.7)
## Summary
The Vite dev server intermittently fails (or hangs) on `vite-basics` and
other Vite-using IT modules because transitive npm dependencies are not
consistently reachable from the project's root `node_modules`. Selenium
tests then wait for pages that never render, and the matrix shard times
out at the 30-minute job cap.
Two distinct symptoms observed:
1. **`Cannot find package '@babel/types'`** — the bundled Vite plugin
`react-function-location-plugin.js` does
`import * as t from '@babel/types';`, but `@babel/types` was only
present transitively via `@babel/core` and pnpm did not always hoist
it to the root.
2. **`Failed to resolve import "@lit/reactive-element/..."`,
`"cookie"`, `"set-cookie-parser"`, `"@preact/signals-react/runtime"`** —
all transitives of declared deps (`lit`, `react-router`,
`@preact/signals-react-transform`) — also not consistently hoisted.
Both are the same underlying problem: pnpm with `shamefully-hoist=true`
does not deterministically expose every transitive at the project root.
On 23.7 the same root cause surfaces as `Cannot find module 'rollup'`
when Vite 3.2.11 tries to load `vite.generated.js`.
## Fix
### 1. Switch the bundled `.npmrc` template to `node-linker=hoisted`
Adds `node-linker=hoisted` to `flow-server/src/main/resources/npmrc`,
the template `TaskRunNpmInstall` writes to the project. This tells
pnpm to install in **flat npm-style mode** — every package (declared
or transitive) at the project root, no `.pnpm/` virtual store.
Resolutions become deterministic.
`shamefully-hoist=true` is left in the template alongside, so manual
`pnpm install` invocations from a developer shell still pick up at
least the legacy partial-hoist behavior if some user-level config
overrides our `node-linker`.
### 2. Pass `--config.node-linker=hoisted` on the pnpm CLI
The `.npmrc` change alone did not actually flip pnpm's install layout
in CI: `node_modules/.pnpm/...` paths kept appearing in error
messages, indicating pnpm was still using its default `isolated`
mode. The pre-existing `--shamefully-hoist=true` CLI flag added by
`FrontendTools.getPnpmExecutable()` likely took precedence and locked
pnpm to the isolated layout (where `shamefully-hoist` makes sense as
a partial-hoist heuristic).
Replace `--shamefully-hoist=true` with `--config.node-linker=hoisted`
on the CLI. CLI > `.npmrc` > defaults in pnpm's config precedence, so
the install is now unambiguously hoisted regardless of any project-
or user-level `.npmrc` content. `FrontendToolsTest` updated to assert
the new flag.
### 3. Update `PnpmUsedIT` to check `pnpm-lock.yaml` instead of the
legacy symlink layout
`flow-tests/test-frontend/test-pnpm/.../PnpmUsedIT.pnpmIsUsed` used
to assert that `node_modules/lit` was a symlink — the pnpm
isolated-mode signature. With hoisted mode that symlink no longer
exists, so the assertion fails (this was caught by the first PR run).
Replace it with a `pnpm-lock.yaml` existence check, which is the
remaining reliable signal that pnpm (not npm) ran the install. This
matches the equivalent change already on main (commit 1dd7ab3).
## Backport notes (23.7)
* Source paths remapped from `flow-build-tools/` (post-refactor on
newer branches) back to `flow-server/` where these classes still
live on 23.7.
* The `@babel/types` dependency addition and the matching
`NodeUpdaterTest` change from the original commit were **omitted**:
23.7 uses Vite 3.2.11 and does not ship the React function-location
plugin that consumes `@babel/types`, so declaring it would be dead
weight.
* `PnpmUsedIT` is also updated here (inlined `"pnpm-lock.yaml"` rather
than `Constants.PACKAGE_LOCK_YAML`, which does not exist on 23.7).
## Trade-offs
- **Disk/install time**: hoisted mode uses slightly more disk and is
a few seconds slower than the symlink store for fresh installs.
Acceptable for the determinism gain.
- **Loose isolation**: hoisted mode allows packages to import any
other package, declared or not. Flow's frontend code already relies
on this (e.g. plugins importing transitives), so this is the
current de-facto behavior — just made deterministic.
- **Behavioral change for downstream users**: anyone with a custom
`.npmrc` that lacks the auto-gen marker keeps their config (per
existing `TaskRunNpmInstall.createNpmRcFile()` logic), but the new
CLI flag still applies — they'll silently get hoisted mode for
Flow-driven installs. Worth flagging in release notes.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit d9290f8)
ZheSun88
added a commit
that referenced
this pull request
May 12, 2026
… (CP:23.7) (#24322) ## Summary The Vite dev server intermittently fails (or hangs) on `vite-basics` and other Vite-using IT modules because transitive npm dependencies are not consistently reachable from the project's root `node_modules`. Selenium tests then wait for pages that never render, and the matrix shard times out at the 30-minute job cap. Two distinct symptoms observed: 1. **`Cannot find package '@babel/types'`** — the bundled Vite plugin `react-function-location-plugin.js` does `import * as t from '@babel/types';`, but `@babel/types` was only present transitively via `@babel/core` and pnpm did not always hoist it to the root. 2. **`Failed to resolve import "@lit/reactive-element/..."`, `"cookie"`, `"set-cookie-parser"`, `"@preact/signals-react/runtime"`** — all transitives of declared deps (`lit`, `react-router`, `@preact/signals-react-transform`) — also not consistently hoisted. Both are the same underlying problem: pnpm with `shamefully-hoist=true` does not deterministically expose every transitive at the project root. On 23.7 the same root cause surfaces as `Cannot find module 'rollup'` when Vite 3.2.11 tries to load `vite.generated.js`. ## Fix ### 1. Switch the bundled `.npmrc` template to `node-linker=hoisted` Adds `node-linker=hoisted` to `flow-server/src/main/resources/npmrc`, the template `TaskRunNpmInstall` writes to the project. This tells pnpm to install in **flat npm-style mode** — every package (declared or transitive) at the project root, no `.pnpm/` virtual store. Resolutions become deterministic. `shamefully-hoist=true` is left in the template alongside, so manual `pnpm install` invocations from a developer shell still pick up at least the legacy partial-hoist behavior if some user-level config overrides our `node-linker`. ### 2. Pass `--config.node-linker=hoisted` on the pnpm CLI The `.npmrc` change alone did not actually flip pnpm's install layout in CI: `node_modules/.pnpm/...` paths kept appearing in error messages, indicating pnpm was still using its default `isolated` mode. The pre-existing `--shamefully-hoist=true` CLI flag added by `FrontendTools.getPnpmExecutable()` likely took precedence and locked pnpm to the isolated layout (where `shamefully-hoist` makes sense as a partial-hoist heuristic). Replace `--shamefully-hoist=true` with `--config.node-linker=hoisted` on the CLI. CLI > `.npmrc` > defaults in pnpm's config precedence, so the install is now unambiguously hoisted regardless of any project- or user-level `.npmrc` content. `FrontendToolsTest` updated to assert the new flag. ## Backport notes (23.7) * Source paths remapped from `flow-build-tools/` (post-refactor on newer branches) back to `flow-server/` where these classes still live on 23.7. * The `@babel/types` dependency addition and the matching `NodeUpdaterTest` change from the original commit were **omitted**: 23.7 uses Vite 3.2.11 and does not ship the React function-location plugin that consumes `@babel/types`, so declaring it would be dead weight. The load-bearing fixes (pnpm CLI flag + npmrc) are sufficient to resolve the `Cannot find module 'rollup'` failure observed in the 23.7 nightly build. ## Trade-offs - **Disk/install time**: hoisted mode uses slightly more disk and is a few seconds slower than the symlink store for fresh installs. Acceptable for the determinism gain. - **Loose isolation**: hoisted mode allows packages to import any other package, declared or not. Flow's frontend code already relies on this (e.g. plugins importing transitives), so this is the current de-facto behavior — just made deterministic. - **Behavioral change for downstream users**: anyone with a custom `.npmrc` that lacks the auto-gen marker keeps their config (per existing `TaskRunNpmInstall.createNpmRcFile()` logic), but the new CLI flag still applies — they'll silently get hoisted mode for Flow-driven installs. Worth flagging in release notes. (cherry picked from commit d9290f8) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vaadin-bot
added a commit
that referenced
this pull request
May 12, 2026
… (CP:23.7) (#24322) (CP: 23.6) (#24323) This PR cherry-picks changes from the original PR #24322 to branch 23.6. --- #### Original PR description > …) (CP: 23.7) > > ## Summary > > The Vite dev server intermittently fails (or hangs) on `vite-basics` and other Vite-using IT modules because transitive npm dependencies are not consistently reachable from the project's root `node_modules`. Selenium tests then wait for pages that never render, and the matrix shard times out at the 30-minute job cap. > > Two distinct symptoms observed: > > 1. **`Cannot find package '@babel/types'`** — the bundled Vite plugin `react-function-location-plugin.js` does `import * as t from '@babel/types';`, but `@babel/types` was only present transitively via `@babel/core` and pnpm did not always hoist it to the root. > > 2. **`Failed to resolve import "@lit/reactive-element/..."`, `"cookie"`, `"set-cookie-parser"`, `"@preact/signals-react/runtime"`** — all transitives of declared deps (`lit`, `react-router`, `@preact/signals-react-transform`) — also not consistently hoisted. > > Both are the same underlying problem: pnpm with `shamefully-hoist=true` does not deterministically expose every transitive at the project root. On 23.7 the same root cause surfaces as `Cannot find module 'rollup'` when Vite 3.2.11 tries to load `vite.generated.js`. > > ## Fix > > ### 1. Switch the bundled `.npmrc` template to `node-linker=hoisted` > > Adds `node-linker=hoisted` to `flow-server/src/main/resources/npmrc`, the template `TaskRunNpmInstall` writes to the project. This tells pnpm to install in **flat npm-style mode** — every package (declared or transitive) at the project root, no `.pnpm/` virtual store. Resolutions become deterministic. > > `shamefully-hoist=true` is left in the template alongside, so manual `pnpm install` invocations from a developer shell still pick up at least the legacy partial-hoist behavior if some user-level config overrides our `node-linker`. > > ### 2. Pass `--config.node-linker=hoisted` on the pnpm CLI > > The `.npmrc` change alone did not actually flip pnpm's install layout in CI: `node_modules/.pnpm/...` paths kept appearing in error messages, indicating pnpm was still using its default `isolated` mode. The pre-existing `--shamefully-hoist=true` CLI flag added by `FrontendTools.getPnpmExecutable()` likely took precedence and locked pnpm to the isolated layout (where `shamefully-hoist` makes sense as a partial-hoist heuristic). > > Replace `--shamefully-hoist=true` with `--config.node-linker=hoisted` on the CLI. CLI > `.npmrc` > defaults in pnpm's config precedence, so the install is now unambiguously hoisted regardless of any project- or user-level `.npmrc` content. `FrontendToolsTest` updated to assert the new flag. > > ## Backport notes (23.7) > > * Source paths remapped from `flow-build-tools/` (post-refactor on newer branches) back to `flow-server/` where these classes still live on 23.7. > * The `@babel/types` dependency addition and the matching `NodeUpdaterTest` change from the original commit were **omitted**: 23.7 uses Vite 3.2.11 and does not ship the React function-location plugin that consumes `@babel/types`, so declaring it would be dead weight. The load-bearing fixes (pnpm CLI flag + npmrc) are sufficient to resolve the `Cannot find module 'rollup'` failure observed in the 23.7 nightly build. > > ## Trade-offs > > - **Disk/install time**: hoisted mode uses slightly more disk and is a few seconds slower than the symlink store for fresh installs. Acceptable for the determinism gain. > - **Loose isolation**: hoisted mode allows packages to import any other package, declared or not. Flow's frontend code already relies on this (e.g. plugins importing transitives), so this is the current de-facto behavior — just made deterministic. > - **Behavioral change for downstream users**: anyone with a custom `.npmrc` that lacks the auto-gen marker keeps their config (per existing `TaskRunNpmInstall.createNpmRcFile()` logic), but the new CLI flag still applies — they'll silently get hoisted mode for Flow-driven installs. Worth flagging in release notes. > > > (cherry picked from commit d9290f8) > Co-authored-by: Zhe Sun <31067185+ZheSun88@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
This ticket/PR has been released with Vaadin 25.2.0-alpha7. |
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.
Summary
The Vite dev server intermittently fails (or hangs) on
vite-basicsandother Vite-using IT modules because transitive npm dependencies are not
consistently reachable from the project's root
node_modules. Seleniumtests then wait for pages that never render, and the matrix shard times
out at the 30-minute job cap.
Two distinct symptoms observed:
Cannot find package '@babel/types'— the bundled Vite pluginreact-function-location-plugin.jsdoesimport * as t from '@babel/types';, but@babel/typeswas onlypresent transitively via
@babel/coreand pnpm did not always hoistit to the root.
Failed to resolve import "@lit/reactive-element/...","cookie","set-cookie-parser","@preact/signals-react/runtime"—all transitives of declared deps (
lit,react-router,@preact/signals-react-transform) — also not consistently hoisted.Both are the same underlying problem: pnpm with
shamefully-hoist=truedoes not deterministically expose every transitive at the project root.
Fix
Three commits, layered:
1. Declare
@babel/typesdirectlyAdds
"@babel/types": "7.29.0"(matching@babel/core) to the bundleddevDependenciesinflow-server/.../dependencies/vite/package.json. Correct on its ownmerits — the plugin imports the package, so the package should be a
real dependency. Updates
NodeUpdaterTest's expected-set accordingly.2. Switch the bundled
.npmrctemplate tonode-linker=hoistedAdds
node-linker=hoistedtoflow-server/src/main/resources/npmrc,the template
TaskRunNpmInstallwrites to the project. This tellspnpm to install in flat npm-style mode — every package (declared
or transitive) at the project root, no
.pnpm/virtual store.Resolutions become deterministic.
shamefully-hoist=trueis left in the template alongside, so manualpnpm installinvocations from a developer shell still pick up atleast the legacy partial-hoist behavior if some user-level config
overrides our
node-linker.3. Pass
--config.node-linker=hoistedon the pnpm CLIThe
.npmrcchange alone did not actually flip pnpm's install layoutin CI:
node_modules/.pnpm/...paths kept appearing in errormessages, indicating pnpm was still using its default
isolatedmode. The pre-existing
--shamefully-hoist=trueCLI flag added byFrontendTools.getPnpmExecutable()likely took precedence and lockedpnpm to the isolated layout (where
shamefully-hoistmakes sense asa partial-hoist heuristic).
Replace
--shamefully-hoist=truewith--config.node-linker=hoistedon the CLI. CLI >
.npmrc> defaults in pnpm's config precedence, sothe install is now unambiguously hoisted regardless of any project-
or user-level
.npmrccontent.FrontendToolsTestupdated to assertthe new flag.
Trade-offs
a few seconds slower than the symlink store for fresh installs.
Acceptable for the determinism gain.
other package, declared or not. Flow's frontend code already relies
on this (e.g. plugins importing transitives), so this is the
current de-facto behavior — just made deterministic.
.npmrcthat lacks the auto-gen marker keeps their config (perexisting
TaskRunNpmInstall.createNpmRcFile()logic), but the newCLI flag still applies — they'll silently get hoisted mode for
Flow-driven installs. Worth flagging in release notes.