fix(core): restore the collection search term from the URL - #951
Open
marianmoldovan wants to merge 1 commit into
Open
fix(core): restore the collection search term from the URL#951marianmoldovan wants to merge 1 commit into
marianmoldovan wants to merge 1 commit into
Conversation
The `search` query param was write-only. `useUpdateUrl` serialised `&search=<term>` into the URL, but `parseFilterAndSort` only ever read `__sort`, `__sort_order` and the `*_op`/`*_value` filter pairs, so nothing read `search` back. On top of that, `searchString` was initialised to `undefined`, so the very first `useUpdateUrl` effect ran a `history.replaceState` that did not include the param: opening `/c/books?search=inimitable` visibly stripped the term from the URL and applied no filtering, while `__sort`/`__sort_order` survived. Fix: - Move the pure `encodeFilterAndSort`/`parseFilterAndSort` helpers into `table_url_params.ts`. They are not re-exported by any barrel file, so the public API is unchanged, but they can now be unit tested. `search` is handled by both, keeping them exact inverses. It is encoded once and read back with `URLSearchParams`, which decodes once, so terms containing a literal `%` no longer risk a URIError from a double decode. As a side effect the URL no longer gets a stray `?&search=` when there is no filter or sort; the old form still parses. - Seed the `searchString` state from the parsed param. - Plumb the restored term to the search input through a new optional `initialValue` prop on `SearchBar` and `initialSearchString` on `CollectionTableToolbar`, so the box shows the search that is actually being applied instead of looking empty. - Auto-initialise text search in `useTableSearchHelper` when a term was restored, since the search bar stays read only (not even clearable) until `textSearchInitialised` flips, and that normally only happens on a click. - Guard the data source call in the fetch effect with try/catch. A restored term can reach a data source whose text search was never initialised, and the Firestore delegate throws synchronously in that case; it is now surfaced as a data loading error instead of escaping the effect. Avoiding a regression of #702: commit a627f1d fixed a reference selection dialog inheriting the parent collection's sort from the URL by gating the seeding on the `updateUrl` prop, which is false for controllers rendered in a dialog. `searchString` is seeded with exactly the same gate as `filterValues` and `sortBy` — `updateUrl ? initialSearchUrl : undefined` — so a dialog controller still starts with no search term and neither reads from nor writes to the URL. `ReferenceSelectionTable` also does not pass `initialSearchString` to `useTableSearchHelper`, so it never auto-initialises text search either. Fixes #724 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
marianmoldovan
requested review from
Copilot and
fgatti675
and removed request for
fgatti675
July 29, 2026 17:07
There was a problem hiding this comment.
Pull request overview
This PR fixes a URL round-trip bug in the core collection table controller so ?search=<term> is restored on reload and preserved across the initial replaceState, aligning search behavior with existing sort/filter URL persistence.
Changes:
- Extracts URL encode/parse logic into a dedicated
table_url_params.tsmodule and adds Jest tests to ensure encode/parse remain exact inverses (including legacy?&search=parsing). - Restores
searchfrom the URL intouseDataSourceTableControllerstate (gated byupdateUrlto avoid regressing reference dialogs) and writes it back symmetrically. - Threads the initial search term through table toolbar →
SearchBar, and auto-initialises text search when a term is restored externally.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/components/SearchBar.tsx | Adds initialValue to seed the search input from restored state. |
| packages/firecms_core/test/table_url_params.test.ts | Adds unit tests covering search param parsing/encoding and round-trips. |
| packages/firecms_core/src/components/EntityCollectionView/EntityCollectionView.tsx | Passes restored search term into search helper and toolbar. |
| packages/firecms_core/src/components/EntityCollectionTable/internal/CollectionTableToolbar.tsx | Wires initialSearchString into SearchBar via initialValue. |
| packages/firecms_core/src/components/EntityCollectionTable/EntityCollectionTable.tsx | Propagates controller search string into the toolbar. |
| packages/firecms_core/src/components/common/useTableSearchHelper.ts | Adds auto-init behavior when an external/restored search term is present. |
| packages/firecms_core/src/components/common/useDataSourceTableController.tsx | Restores search from URL (when updateUrl), persists it back to URL, and adds sync-throw protection for datasource fetch/listen. |
| packages/firecms_core/src/components/common/table_url_params.ts | New module implementing symmetric encode/parse of filter/sort/search. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+94
to
+99
| const shouldAutoInitialise = Boolean(initialSearchString) && textSearchEnabled && !textSearchInitialised && Boolean(onTextSearchClick); | ||
| useEffect(() => { | ||
| if (autoInitialisedRef.current || !shouldAutoInitialise) return; | ||
| autoInitialisedRef.current = true; | ||
| onTextSearchClick?.(); | ||
| }, [shouldAutoInitialise]); |
Comment on lines
+91
to
+96
| if (key === "__sort") { | ||
| sortBy = [decodeURIComponent(value), entries.get("__sort_order") as "asc" | "desc"]; | ||
| } else if (key.endsWith("_op")) { | ||
| const field = key.replace("_op", ""); | ||
| const filterOp = decodeURIComponent(value) as WhereFilterOp; | ||
| const filterValStr = entries.get(`${field}_value`); |
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.
Fixes #724
The bug
Two halves, both in
useDataSourceTableController:useUpdateUrlserialised&search=<term>into the URL, butparseFilterAndSortonly matched__sort,__sort_orderand*_op/*_value. Nothing ever readsearchback.searchStringstartedundefined, so the firsthistory.replaceStatewrote a query string without it — which is why the param visibly disappeared rather than merely being ignored.Reproduced side by side:
The fix
encodeFilterAndSort/parseFilterAndSortare extracted into a newtable_url_params.ts(a module no barrel re-exports, so it is testable without widening the public API),searchnow round-trips, andinitialSearchStringis threaded throughuseTableSearchHelper→EntityCollectionView→CollectionTableToolbar→SearchBar.#702 is not regressed
a627f1d1efixed reference dialogs inheriting the parent collection's sort/filter from the URL, via theupdateUrlgate. The identical gate is applied here:A dialog controller (
updateUrl: false) therefore still neither reads nor writes the URL.initialSearchStringis also deliberately not passed inReferenceSelectionTable, so the auto-init cannot fire there.Two extras found while making encode/parse symmetric
__sortpath double-decodes (URLSearchParamsanddecodeURIComponent), which would throwURIErroron a term like100%— sosearchis read back without the second decode.?&search=xwhen there was no filter/sort. Fixed, with the legacy form still parsing (tested).Tests
9 round-trip tests, including escaping, combined sort+filter+search, the legacy stray-ampersand form, and not confusing a property literally named
search.@firecms/core: 246/249 passing (was 237/240) — 3 pre-existing failures unchanged.If a datasource's
search()requires a priorinit()— the local Fuse controller throwsIndex not found for path X— the first fetch races the auto-init, and the fetch effect's deps don't include text-search readiness, so it won't refetch when init resolves. A deep link can therefore land on an empty table until the user edits the term. The Typesense/FireCMS controller self-initialises insidesearch()and is unaffected. A try/catch guarantees no crash either way.Fixing this properly means moving ownership of
textSearchInitialisedbetweenuseDataSourceTableControlleranduseTableSearchHelper— a restructure judged out of scope for this fix.Not verified
Whether the search box visually renders the restored term needs a browser; the mechanism is a one-line state seed but is unexercised at runtime. Real Firestore/Typesense deep-link behaviour also untested.
🤖 Generated with Claude Code