Skip to content

adopt(vendure-refresh): ready to send: dashboard double submit still reproduces on 3.8.1; patch rebased and re-tested #158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MeharPro wants to merge 1 commit into
base: main
Choose a base branch
from
Open

adopt(vendure-refresh): ready to send: dashboard double submit still reproduces on 3.8.1; patch rebased and re-tested #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/adoption/hunts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ the project's maintainers to take or leave. One report per target; nothing here
| target | report | patch and draft text | harness |
|---|---|---|---|
| Strapi admin panel (5.59.0) | [strapi.md](strapi.md) | [strapi.patch](strapi.patch), [strapi-issue.md](strapi-issue.md) | [strapi/](strapi/) |
| Vendure dashboard (3.8.1, refreshed 2026-11-03 at `f97b187`) | [vendure.md](vendure.md) | [vendure.patch](vendure.patch), [vendure-issue.md](vendure-issue.md) | [vendure-hunt.mjs](vendure-hunt.mjs), [vendure-refresh/](vendure-refresh/) |

Every report gives the versions and commits, the exact commands, every scenario with its verdict including the
negatives, the finding with its evidence in causal order, the root cause, the fix, the test result, the check on the
Expand Down
160 changes: 119 additions & 41 deletions docs/adoption/hunts/vendure-issue.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,61 @@
# Vendure: draft issue and draft pull request

Drafts for the owner to send. Nothing here has been posted to vendurehq/vendure. Evidence and method:
[`vendure.md`](./vendure.md). The patch: [`vendure.patch`](./vendure.patch) (against `master` at `aba3e9c`).

Before filing: check that nobody has filed the same dashboard bug since 2026-10-24, and decide whether to file a new
issue (below) or to comment on [vendurehq/vendure#2117](https://github.com/vendurehq/vendure/issues/2117). #2117 is the
legacy admin-ui report of the same bug; its last maintainer comment (2025-04-02) says the fix "will be included in a
future release of the new `@vendure/dashboard`". The issue below links it. If the new issue gets a number, the
`#2117` references in the patch's comments and test title can be changed to it.
[`vendure.md`](./vendure.md) (refreshed 2026-11-03, section 9). The patch: [`vendure.patch`](./vendure.patch) (against
`master` at `f97b187`, 2026-11-02; it also applies unchanged to the hunt's `aba3e9c`).

## New issue or a comment on #2117: file a new issue

Checked 2026-11-03. [vendurehq/vendure#2117](https://github.com/vendurehq/vendure/issues/2117) is still open, labelled
`@vendure/admin-ui`, `P4: low`, `type: bug`, `contributions welcome`, assigned to michaelbromley, with no linked branch
or pull request. Its last comment is still dlhck's of 2025-04-02: "Will be included in a future release of the new
`@vendure/dashboard`. Happy for community contributions to the legacy `@vendure/admin-ui`". The only later activity is
a label added and removed on 2026-07-17, with no comment.

File a **new issue** for the dashboard, and do not comment on #2117:

- Vendure's CONTRIBUTING asks for a bug fix to start with "an issue detailing the bug" in which you "indicate that you
intend to fix it", and its `AGENTS.md` asks for `Fixes #ISSUE_NUMBER` (or `Relates to #...`) in the commit body. The
patch fixes the React dashboard only. `Fixes #2117` would close the admin-ui report, which still reproduces (report
section 6). A dashboard issue gets the dashboard label and a number the PR can close.
- [vendurehq/vendure#4884](https://github.com/vendurehq/vendure/issues/4884) (closed 2026-09-30 by
[#5007](https://github.com/vendurehq/vendure/pull/5007)) says "the React dashboard does avoid the double-click". On
3.8.1 it does not. A new issue puts that on record with the steps. A comment on a two-year-old admin-ui issue would not.
- Mentioning #2117 and #4884 in the new issue adds the cross-reference to both threads, so no comment is needed.

## Before filing (what the 2026-11-03 refresh did, and what is left)

Done by the refresh session (report section 9):

- [x] Searched the tracker for anything filed on the same bug since 2026-10-24. Nothing on the dashboard's double submit
was found. The search covered issues and pull requests for "double click", "double-click", "double submit",
"twice dashboard create", "duplicate dashboard create", "isPending createMutation", "submit twice" and
"useDetailPage isPending". The related #4884 and #5007 are linked below.
- [x] Read the current CONTRIBUTING.md, the issue and PR templates, `.github/ISSUE_TEMPLATE/config.yml` (blank issues
off) and the repository's `AGENTS.md`. Neither CONTRIBUTING nor AGENTS.md has an AI policy. Many recent `master`
commits carry a `Co-authored-by: Claude` trailer (36 of the 270 commits since 2026-10-01). Contributions need the
CLA, which a bot asks you to sign when the PR opens. There is no DCO.
- [x] Rebased the patch on `master` `f97b187` (it applied unchanged). Following Vendure's `AGENTS.md`, the e2e test
moved from a new `e2e/tests/regression/` file into the existing `e2e/tests/catalog/collections.spec.ts`:
"`tests/regression/` — **only** for tests that genuinely don't fit any existing suite".
- [x] Ran the unit tests and the e2e tests three times each on the unfixed code and three times each on the fixed code,
and re-ran the reproduction without GenClass on both. Ran `check-types`, the dashboard unit suite, and eslint and
prettier on the changed files.
- [x] Rewrote the version, commit, environment and numbers in the drafts below. Line numbers are unchanged since
`aba3e9c`.

Left for the sender, by hand:

- [ ] Search the tracker once more on the day you file (the queries above), in case something was filed after
2026-11-03.
- [ ] Reproduce by hand: run the dev server, double-click **Create** on a new collection, and see two collections and
two `CreateCollection` requests (steps in the issue below).
- [ ] Apply the patch to a fresh fork at current `master`. Replace `#2117` with the new issue's number in the four
files (`sed -i 's/#2117/#NNNN/g'` on the paths in the patch). Run the tests yourself:
`npx vitest run src/lib/framework/form-engine/use-generated-form.spec.tsx`, then the e2e command in the PR
text.
- [ ] Read the whole diff (240 added and 9 removed lines in four files).
- [ ] File the issue, then open the PR against `master` from your fork. Sign the CLA when the bot asks.

---

Expand All @@ -19,7 +67,10 @@ future release of the new `@vendure/dashboard`". The issue below links it. If th

On every detail page built with `useDetailPage` (collections, products, facets, customers and the rest), a quick
double-click on the **Create** button sends the create mutation twice and creates two entities. A double-click on
**Update** sends the update mutation twice. This is the dashboard counterpart of #2117 (legacy admin-ui).
**Update** sends the update mutation twice. This is the dashboard counterpart of #2117 (legacy admin-ui). It is also
not covered by #5007: that fix stops concurrent updates from writing duplicate translation rows (#4884), but a double
Create still makes two separate entities. #4884 says "the React dashboard does avoid the double-click", and on 3.8.1
that is not the case.

Two things combine:

Expand All @@ -41,28 +92,34 @@ Two things combine:
4. Open the collection list: there are two collections with that name (two slugs, `name` and `name-2`), and the
network tab shows two `CreateCollection` requests.

It also reproduces with Playwright's plain `locator.dblclick()`: the regression test in the linked PR fails on
`master` in 5 of 5 runs for Create and 5 of 5 for Update (two mutations each time).
It also reproduces with Playwright's plain `locator.dblclick()`. The regression test in the linked PR fails on
`master` in 3 of 3 runs for Create and 3 of 3 for Update, with two mutations each time.

**Expected behavior**

One create (or update) mutation per double-click, and one entity.

**Actual behavior**

Two mutations; two entities on Create. Measured on `master` (`aba3e9c`, 3.8.1) with the SQLite dev server and the
production build of the dashboard, presses sent through CDP 0 to 150 ms apart: 77 of 160 Create trials sent `create*` twice (all 40 at a 0 ms gap, 33 of 40 at 30 ms, 4 of 40 at 80 ms)
and left 60 duplicate collections, products and facets in the database; 23 of 48 Update trials sent the update
twice. With 100 ms of added request latency (a remote server), 37 of 48 Create trials doubled, including every page at
an 80 ms gap. The duplicates get de-duplicated slugs or codes (`name`, `name-2`), so nothing fails visibly.
The mutation is sent twice, and Create makes two entities. Measured on `master` (`f97b187`, 3.8.1) with the SQLite
dev server and the production build of the dashboard. Each trial sent two presses through Chrome DevTools Protocol,
0, 30, 80 or 150 ms apart, 3 trials per page and gap:

- **Create** (Collection, Product, Facet, Customer): 21 of 48 trials sent `create*` twice. That was 12 of 12 at a 0 ms
gap, 9 of 12 at 30 ms, and none at 80 or 150 ms. The trials left 15 duplicate collections, products and facets in the
database.
- **Update**: 25 of 48 trials sent the update twice. That was 12 of 12 at 0 ms, 9 of 12 at 30 ms, and 4 of 12 at
80 ms.

On Customer the server rejects the second create (the email address is taken), so only the request is duplicated
there.
The duplicates get de-duplicated slugs or codes (`name`, `name-2`), so nothing fails visibly. On Customer the server
rejects the second create (the email address is taken), so only the request is duplicated there. The window is about
one request long, so it grows with latency. With 100 ms added to every request, a measurement on 2026-10-24 found
doubled creates on every page at an 80 ms gap.

**Environment**

- @vendure/core version: 3.8.1 (`master` at `aba3e9c`, 2026-10-23)
- Nodejs version: 22.22.0
- @vendure/core version: 3.8.1 (`master` at `f97b187`, 2026-11-02)
- Nodejs version: 22.23.4
- Database: SQLite (dev server, `DB=sqlite`)
- Operating System: Linux
- Browser: Chromium 153
Expand All @@ -74,52 +131,69 @@ Click once and wait for the toast.

**Additional context**

I intend to fix this and have a patch ready: `createMutation.isPending` added to `isPending`, and a guard in
`useGeneratedForm`'s submit handler that ignores a submit while the previous one is being validated or while the
promise returned by `onSubmit` is pending (`useDetailPage` returns its `mutateAsync` promise). It adds no dependency
and does not change the public API apart from `GeneratedFormOptions.onSubmit` also accepting a returned promise. Unit
tests in `use-generated-form.spec.tsx` and a Playwright regression test in `e2e/tests/regression/` are included. I
found it with GenClass, a browser library that reports duplicate submits; the steps above need nothing but the
dashboard.
I intend to fix this and have a patch ready. It adds `createMutation.isPending` to `isPending`. It also adds a guard in
`useGeneratedForm`'s submit handler that ignores a submit while the previous one is being validated, or while the
promise returned by `onSubmit` is pending (`useDetailPage` returns its `mutateAsync` promise). The patch adds no
dependency. The only API change is that `GeneratedFormOptions.onSubmit` may also return a promise. It includes unit
tests in `use-generated-form.spec.tsx` and a Playwright test in `e2e/tests/catalog/collections.spec.ts`. Related:
#2117 (the same bug in the legacy admin-ui, which this does not touch), #4884 and #5007.

**AI assistance:** I used Claude (Claude Code) to investigate this, write the fix and the tests, and draft this
description; I reviewed the change and ran the tests and the manual check myself. The race was found with GenClass, a
tool I work on, which replays the same steps with the server's responses delivered in different orders.

---

## Draft pull request (template: `.github/pull_request_template.md`)

**Base:** `master`. **Title (conventional, lower-case subject):** `fix(dashboard): submit a detail form only once on double-click`

**Commit message** (one commit, body per Vendure's `AGENTS.md`):

```
fix(dashboard): submit a detail form only once on double-click

Fixes #NNNN. Relates to #2117.
```

(Many Vendure commits also carry a `Co-authored-by:` trailer for Claude. Adding one is optional and fits the
disclosure below.)

# Description

Fixes #ISSUE (dashboard counterpart of #2117).
Fixes #NNNN. Relates to #2117 (the same bug in the legacy admin-ui, not touched here).

A double-click on Create on any `useDetailPage` page created the entity twice, and a double-click on Update sent the
A double-click on Create on any `useDetailPage` page created the entity twice. A double-click on Update sent the
update twice:

- `useDetailPage` left `createMutation.isPending` out of the `isPending` it returns, so the Create button stayed
enabled while the create mutation was in flight.
- Even with `isPending` wired in, the submit handler in `useGeneratedForm` awaits validation before it calls
`onSubmit`, and the button is only disabled on the next render, so a fast second click still submitted again. Adding
`createMutation.isPending` alone does not fix it: with only that change the new e2e test still fails 10 of 10 runs.
`createMutation.isPending` alone does not fix it. With only that change, the new e2e test still failed 10 of 10
runs in an earlier measurement (2026-10-24).

Changes:

- `useGeneratedForm`: the submit handler ignores a submit while the previous one is being validated, and while the
promise returned by `onSubmit` is pending. `GeneratedFormOptions.onSubmit` may now return a promise (returning
nothing keeps the old behavior apart from the validation-time guard).
- `useDetailPage`: `onSubmit` returns the create or update `mutateAsync` promise (errors still go to the mutation's
`onError`, the rejection is caught), and `isPending` includes `createMutation.isPending`, so the Create button is
disabled while the request is in flight, like Update.
promise returned by `onSubmit` is pending. `GeneratedFormOptions.onSubmit` may now return a promise. If it returns
nothing, the only change is the guard during validation.
- `useDetailPage`: `onSubmit` returns the create or update `mutateAsync` promise. Errors still go to the mutation's
`onError`, and the rejection is caught. `isPending` now includes `createMutation.isPending`, so the Create button is
disabled while the request is in flight, as the Update button is.

Tests:

- `use-generated-form.spec.tsx`: a second submit during validation is ignored; further submits are ignored until the
`onSubmit` promise settles; a submit that fails validation releases the guard. The first two fail on `master`.
- `e2e/tests/regression/double-submit-on-save.spec.ts`: `dblclick()` on Create creates one collection (checked on the
wire and through the Admin API); `dblclick()` on Update sends one update. On `master` both fail 5 of 5 runs; with
the fix both pass 5 of 5.
- `use-generated-form.spec.tsx` covers three cases: a second submit during validation is ignored; further submits
are ignored until the `onSubmit` promise settles; a submit that fails validation releases the guard. The first two
fail on `master`.
- `e2e/tests/catalog/collections.spec.ts`, `Issue #NNNN: Double-click on Create or Update submits once`, has two
tests. `dblclick()` on Create must create one collection, checked on the wire and through the Admin API.
`dblclick()` on Update must send one update. On `master` both failed 3 of 3 runs. With the fix both passed 3 of 3.

Checked locally: `npm run check-types`, `eslint` and `prettier` on the changed files, the dashboard unit suite (961
passed, 4 skipped), and the new e2e spec.
Checked locally on `master` `f97b187`: `npm run check-types`, eslint and prettier on the changed files, the dashboard
unit suite (972 passed, 4 skipped) and the new e2e tests:
`CI=true VITE_TEST_PORT=5176 npx playwright test --config e2e/playwright.config.ts e2e/tests/catalog/collections.spec.ts --reporter=list`.

# Breaking changes

Expand All @@ -139,3 +213,7 @@ Not applicable.
👍 Most of the time:
- [x] I have added or updated test cases
- [ ] I have updated the README if needed (not needed)

**AI assistance:** I used Claude (Claude Code) to investigate this, write the fix and the tests, and draft this
description; I reviewed the change and ran the tests and the manual check myself. The race was found with GenClass, a
tool I work on, which replays the same steps with the server's responses delivered in different orders.
44 changes: 44 additions & 0 deletions docs/adoption/hunts/vendure-refresh/summarize.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Summarises the raw run files of docs/adoption/hunts/vendure-hunt.mjs (its OUT=... JSON) into the small table the
// 2026-11-03 refresh of docs/adoption/hunts/vendure.md commits as vendure-refresh/summary.json. Not part of CI.
//
// node docs/adoption/hunts/vendure-refresh/summarize.mjs master-nogc.json patch-nogc.json master-gc.json > summary.json
//
// Per run: trials, failed trials, trials that sent the mutation twice, duplicate entities in the database (Create),
// error responses, and, when GenClass was injected, trials with a duplicate-request witness on the mutation; per
// action and gap the same counts, so the report's tables can be checked against the raw rows.

import { readFileSync } from "node:fs";
import { basename } from "node:path";

const out = {};
for (const file of process.argv.slice(2)) {
const run = JSON.parse(readFileSync(file, "utf8"));
const rows = run.rows.filter((r) => !r.failed);
const count = (rs, f) => rs.filter(f).length;
const cell = (rs) => ({
trials: rs.length,
twoMutations: count(rs, (r) => r.wire >= 2),
duplicateEntities: rs.some((r) => r.action === "create") ? rs.reduce((a, r) => a + Math.max(0, (r.created ?? 0) - 1), 0) : null,
trialsWithTwoEntities: rs.some((r) => r.action === "create") ? count(rs, (r) => r.created >= 2) : null,
genclassDuplicate: run.genclass ? count(rs, (r) => r.genclass > 0) : null,
genclassFalse: run.genclass ? count(rs, (r) => r.genclass > 0 && r.wire < 2) : null,
});
const byActionGap = {};
for (const r of rows) ((byActionGap[`${r.action} ${r.gap}`] ??= []).push(r));
const byPage = {};
for (const r of rows) ((byPage[`${r.action} ${r.entity}`] ??= []).push(r));
out[basename(file, ".json")] = {
label: run.label,
base: run.base,
latencyMs: run.latency,
genclass: run.genclass,
failed: run.rows.length - rows.length,
errors: [...new Set(rows.flatMap((r) => r.errors ?? []))],
total: Object.fromEntries(
[...new Set(rows.map((r) => r.action))].map((a) => [a, cell(rows.filter((r) => r.action === a))]),
),
byActionGap: Object.fromEntries(Object.entries(byActionGap).map(([k, rs]) => [k, cell(rs)])),
byPage: Object.fromEntries(Object.entries(byPage).map(([k, rs]) => [k, cell(rs)])),
};
}
console.log(JSON.stringify(out, null, 2));
Loading
Loading