fix: stop pnpm from reinstalling before the postinstall script - #25779
Merged
Merged
Conversation
Before pnpm runs a script it checks that node_modules is up to date, and that check starts an install of its own. It repeats the install Flow has just run, but without the arguments that exempt the packages Vaadin publishes from the minimum release age and without --ignore-scripts. When it fails, the postinstall script is never run, and nothing reported it because the exit code was ignored. Fixes #21662, Fixes #24333
The command is now logged next to the install command at debug level, so a test can assert that the argument reaches it. Without the log, the only coverage was of the method that builds the argument, and dropping the call that adds it to the command would have left the suite green.
|
Contributor
Artur-
approved these changes
Sep 17, 2026
Collaborator
|
Hi @totally-not-ai[bot] and @Artur-, when i performed cherry-pick to this commit to 25.2, i have encountered the following issue. Can you take a look and pick it manually? |
vaadin-bot
added a commit
to vaadin/docs
that referenced
this pull request
Sep 17, 2026
Source PR: fix: stop pnpm from reinstalling before the postinstall script Source commit: ea9d15109b1275dffce1d57e66af9933825b0eaa Categories: BEHAVIOR_CHANGE
Contributor
|
Pull request created: #6067
|
Contributor
|
Documentation Bot: Draft documentation pull request for this change: vaadin/docs#6067 Files updated:
It was written from the state of this pull request as you see it now. Please review it and mark it ready for review.
|
vaadin-bot
added a commit
that referenced
this pull request
Sep 17, 2026
… (CP: 25.3) (#25780) This PR cherry-picks changes from the original PR #25779 to branch 25.3. --- #### Original PR description > ## Summary > > When pnpm is used, the `postinstall` scripts Flow runs after the frontend install could fail because pnpm started a second install of its own first. Flow now tells pnpm to skip that check, and it also fails the build when a postinstall script fails instead of silently ignoring it. > > ## What changed > > **Behavior change:** a `postinstall` script that exits with a non-zero status now fails the build with an `ExecutionFailedException` that names the package and includes the script output. Before, the exit code was ignored and the build continued as if nothing happened. This affects all package managers (npm, pnpm, bun). Builds that were already broken this way now report the real error instead of failing later in a confusing place. > > - `TaskRunNpmInstall` passes `--config.verify-deps-before-run=false` to the postinstall command when pnpm is enabled. Before running a script, pnpm checks that `node_modules` is up to date, and that check starts its own install. That install repeats the one Flow just ran, but without the arguments that exempt Vaadin's own packages from the minimum release age and without `--ignore-scripts`. When it failed, the postinstall script never ran. > - npm and bun get no extra argument: they run scripts without checking anything, and bun rejects an argument it does not know. > - The postinstall command is now also logged at debug level, next to the install command. > > No public or protected API was added, removed or changed. > > Fixes #21662, Fixes #24333 > > ## Test summary > > | # | Status | What the test verifies | Why it matters | > |---|--------|------------------------|----------------| > | 1 | ✅ | With pnpm enabled, the postinstall arguments are exactly `--config.verify-deps-before-run=false` | This flag is the whole fix; without it pnpm starts a second install and the script never runs | > | 2 | ✅ | A real pnpm build logs that flag as part of the postinstall command | Pins that the argument is actually added to the command, not just built by a helper | > | 3 | ✅ | npm and bun get no extra argument | bun fails on an argument it does not expect, so passing one would break every bun build | > | 4 | ✅ | A postinstall script exiting with status 3 fails the build, and the message names the package and contains the script output | A silently ignored failure is what hid these bugs; the output is what makes the error debuggable | > > - `TaskRunNpmInstallTest.postinstallArguments_pnpm_doesNotVerifyTheDependencies` → 1 > - `TaskRunPnpmInstallTest.runPnpmInstall_postinstallDoesNotVerifyTheDependencies` → 2 > - `TaskRunNpmInstallTest.postinstallArguments_npmAndBun_needNone` → 3 > - `TaskRunNpmInstallTest.runNpmInstall_postInstallFails_buildFailsWithTheScriptOutput` → 4 (new test resource `fake-package-with-failing-postinstall.json` provides the failing script) > > Row 4 runs for every package manager, because `TaskRunPnpmInstallTest` and the bun test class extend `TaskRunNpmInstallTest`. Left untested on purpose: that pnpm itself honours the flag and really skips its install, since that depends on the installed pnpm version and cannot be asserted in a unit test; and the exact debug log wording. A successful postinstall run is still covered by the existing tests in `TaskRunNpmInstallTest`. Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com>
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
2 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
When pnpm is used, the
postinstallscripts Flow runs after the frontend install could fail because pnpm started a second install of its own first. Flow now tells pnpm to skip that check, and it also fails the build when a postinstall script fails instead of silently ignoring it.What changed
Behavior change: a
postinstallscript that exits with a non-zero status now fails the build with anExecutionFailedExceptionthat names the package and includes the script output. Before, the exit code was ignored and the build continued as if nothing happened. This affects all package managers (npm, pnpm, bun). Builds that were already broken this way now report the real error instead of failing later in a confusing place.TaskRunNpmInstallpasses--config.verify-deps-before-run=falseto the postinstall command when pnpm is enabled. Before running a script, pnpm checks thatnode_modulesis up to date, and that check starts its own install. That install repeats the one Flow just ran, but without the arguments that exempt Vaadin's own packages from the minimum release age and without--ignore-scripts. When it failed, the postinstall script never ran.No public or protected API was added, removed or changed.
Fixes #21662, Fixes #24333
Test summary
--config.verify-deps-before-run=falseTaskRunNpmInstallTest.postinstallArguments_pnpm_doesNotVerifyTheDependencies→ 1TaskRunPnpmInstallTest.runPnpmInstall_postinstallDoesNotVerifyTheDependencies→ 2TaskRunNpmInstallTest.postinstallArguments_npmAndBun_needNone→ 3TaskRunNpmInstallTest.runNpmInstall_postInstallFails_buildFailsWithTheScriptOutput→ 4 (new test resourcefake-package-with-failing-postinstall.jsonprovides the failing script)Row 4 runs for every package manager, because
TaskRunPnpmInstallTestand the bun test class extendTaskRunNpmInstallTest. Left untested on purpose: that pnpm itself honours the flag and really skips its install, since that depends on the installed pnpm version and cannot be asserted in a unit test; and the exact debug log wording. A successful postinstall run is still covered by the existing tests inTaskRunNpmInstallTest.