fix(lib): stop the url sync from restoring a removed filter tag - #862
Merged
Conversation
Removing a hashtag filter had no effect: the chip reappeared immediately and the tag stayed in the url. `removeFilterTag` drops the tag from the filter state and rewrites the address bar in one go, but the router location still carries the old `tags` value for one render. The sync effect ran on that stale location, saw a filter state that no longer matched the url, and faithfully restored the tag it had just removed — including writing it back into the url. The sync now follows the url alone: it remembers which `tags` value it last applied and does nothing while that value is unchanged, so a state update can no longer trigger a reconcile against a stale location. It also waits for the available tags before recording, which keeps deep links working. The effect moved out of UtopiaMapInner into `useSyncFilterTagsWithUrl` so it can be covered by tests. The suite reproduces the regression with a real BrowserRouter — MemoryRouter cannot, because `useFilter` reads `window.location`, which MemoryRouter never touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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
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.
Symptom
Filtering the map by hashtags worked, but the filters could not be removed again: clicking ✕ on a tag chip left the chip in place and the tag in the url.
Cause
removeFilterTagdrops the tag from the filter state and rewrites the address bar in one go. The router location, however, still carries the oldtagsvalue for one render. A render log of the click makes it visible:The sync effect ran on that stale location, saw a filter state that no longer matched the url, and restored the tag it had just removed — writing it back into the url as well. Because the effect listed
filterTagsamong its dependencies, every state change triggered exactly this reconcile.Fix
The sync now follows the url alone. It remembers the
tagsvalue it last applied and does nothing while that value is unchanged, so a state update can no longer trigger a reconcile against a stale location. It also waits for the available tags before recording anything, which keeps deep links such as/?tags=gartenworking when the tag list arrives late.The effect moved out of
UtopiaMapInnerintouseSyncFilterTagsWithUrlso it can be covered by tests.Tests
Four cases in
useSyncFilterTagsWithUrl.spec.tsx: reading one tag from the url, reading several, removing a tag, and removing one of two. The two removal tests fail without the fix.The suite needs a real
BrowserRouter.MemoryRoutercannot reproduce the regression becauseuseFilterreadswindow.location, whichMemoryRouternever touches — worth knowing before anyone rewrites these tests.vitest run(52 passed),eslint --max-warnings 0andtsc --noEmitall pass inlib.Note
That
useFilterreads the globalwindow.locationwhile the sync uses the router location is the underlying weakness that made this possible. This PR does not change it, to keep the fix small — it looks like a worthwhile follow-up.🤖 Generated with Claude Code