Skip to content

Fix Fetch dropping repeated form fields and leaking src into history (2.x) - #644

Open
titouanmathis wants to merge 1 commit into
2.xfrom
fix/fetch-2x
Open

Fix Fetch dropping repeated form fields and leaking src into history (2.x)#644
titouanmathis wants to merge 1 commit into
2.xfrom
fix/fetch-2x

Conversation

@titouanmathis

Copy link
Copy Markdown
Contributor

Cherry-pick of 7edbbdd from #643, which landed on main. Fetch.ts was byte-identical on the two branches, so it applied without conflict.

See #643 for the reasoning. In short:

  • a repeated GET form field sent one value, because the fold used set per entry — a checkbox group could never send more than one term;
  • history pushed the URL that was fetched, so a src pointing at a lighter endpoint leaked its parameters into the address bar. The new historyUrl getter follows the element's own destination instead.

Nothing changes for an element without src.

Verified on this branch: 100 Fetch tests, full suite green.

Two faults in the same resolver, both reported from a facet filter form.

Folding a GET form onto the base URL used `set` per field, which keeps one value
per name. A checkbox group is repeated names by design, so ticking a second box
changed nothing: FormData held both values, `new URLSearchParams(FormData)`
serialised both, and the request carried one. The fold now deletes the base's
values the first time it meets a name and appends after that, which keeps the
override `set` was there for and stops discarding the rest.

History pushed the URL that was fetched, so a `src` pointing at a lighter
endpoint put its own parameters in the address bar and in anything a visitor
copied out of it. History now follows the element's own destination — a link's
href, a form's action folded with its data — through a new `historyUrl` getter. A
URL passed explicitly to `fetch()` is still pushed as given.

Nothing changes for an element without `src`, where the two URLs are the same.
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Code Review

Risk: Medium — issues that should be addressed before merge.

This change preserves repeated GET form values and separates the requested URL from the URL written to browser history. It adds URL-resolution coverage and documentation for both behaviors, but the new history state persists across updates and can override URLs supplied to later direct update() calls.

1 issue found:

  • issuepackages/ui/src/Fetch/Fetch.ts:501 — Clear stale history URL before direct updates

Review usage: 19,961 in (3,677 cached) / 696 out tokens — $0.0124 (openrouter/openai/gpt-5.6-luna, thinking: low)

Reviewed by @weareikko/code-review v0.9.5 for commit a8e95f7.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.85%. Comparing base (c939977) to head (a8e95f7).

Additional details and impacted files
@@             Coverage Diff              @@
##                2.x     #644      +/-   ##
============================================
+ Coverage     87.80%   87.85%   +0.05%     
  Complexity      145      145              
============================================
  Files           153      153              
  Lines          5443     5460      +17     
  Branches       1037     1040       +3     
============================================
+ Hits           4779     4797      +18     
+ Misses          580      579       -1     
  Partials         84       84              
Flag Coverage Δ
unittests 87.85% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/ui/src/Fetch/Fetch.ts 100.00% <100.00%> (+0.70%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant