Fix Fetch dropping repeated form fields and leaking src into history - #643
Conversation
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.
Code ReviewRisk: Medium — issues that should be addressed before merge. Updates 1 issue found:
Review usage: 20,680 in (3,677 cached) / 705 out tokens — $0.0129 (openrouter/openai/gpt-5.6-luna, thinking: low) Reviewed by @weareikko/code-review v0.9.5 for commit 7edbbdd. |
Export sizeBundled per export with peer dependencies left external, dynamic imports excluded and the output minified; sizes are gzipped. @studiometa/ui
Unchanged (128)@studiometa/ui
@studiometa/ui-mapbox
@studiometa/ui-motion
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
============================================
+ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Two faults in
Fetch's URL resolution, both found while building a facet filter form. Targetingmain(v1) deliberately:Fetch.tsis byte-identical onmainand2.x, so this patch applies to both, and v1 is what consumers are on today.1. A repeated GET field only sent one value
get url()folded the form onto the base withurl.searchParams.set(key, value)per entry.setreplaces, so for a checkbox group — repeated names by design — only one value survived.Measured in a browser before the fix, on a form with two boxes ticked:
Ticking a second box changed nothing on screen, which is how it was noticed.
setwas there for a reason the docblock states — a fixed query insrcmust be overridable by a live field — soappendalone is not the fix: it would let the base's stale value through. The fold now deletes the base's values the first time it meets a name, then appends, which keeps the override and stops discarding the rest:?genre=stale§ion=keep, formgenre=rock,genre=jazzset(before)genre=jazz§ion=keepappendonlygenre=stale&genre=rock&genre=jazz§ion=keepgenre=rock&genre=jazz§ion=keepThis also covers
<select multiple>, which repeats a name without the[]suffix — so the fix cannot key on the name's shape.2. History pushed the fetched URL, not the navigation
update()pushed the URL that was requested. Withsrcset to a lighter endpoint, its parameters landed in the address bar and in every link a visitor copied.srcanswers what to request. The newhistoryUrlgetter answers what this navigation is: the element's own destination, folded with the same form data.Requests the
sections=listingURL, pushes/projects/page/2?orderby=title.A URL passed explicitly to
fetch(url)is still pushed as given — a caller that named a URL meant that URL — sofetch()now captures whether the URL came from the element or from the caller.Nothing changes for an element without
src, where the request URL and the destination are the same.update()'s signature is untouched, and a directupdate()call still pushes the URL it was handed.One consequence, documented rather than engineered around
A back or forward navigation re-fetches
window.location.href, which is now the pushed URL rather than thesrcone.selectortherefore has to match elements present in both responses. There is a tip about it in the docs.Tests
packages/tests/Fetch/Fetch.spec.tsgains eight cases: repeated fields with and without brackets, repeated fields overriding a conflictingsrcquery,historyUrlfor links and for forms,historyUrlfalling back tourl, history pushing the destination rather than thesrc, and history pushing an explicitly passed URL.<select multiple>is not among them, with a comment saying why: happy-dom puts only the first selected option intoFormData, so the case cannot be expressed in this environment. The bracket-free coverage uses two same-named inputs instead, which makes the same point.893 tests pass,
npm run lintclean.