feat(mantine): export useUrlSyncedState and support hash router [ADUI-10457] #3992
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.
Related ticket(s)
ADUI-10457
Proposed Changes
The short summary:
useUrlSyncedState
, so it can be used from Admin-UI too.(It appears to be the best way forward for Data Platform Foundation to fix another state + search parameters related issue).
#
) section of the URL.Officially the 'search' part comes before the 'hash', but for page hashes there should be a "fake" search inside the hash path too. This makes sense when you realize that the whole reason for using the HashRouter is making sure you don't send the actual visited pages to the server (including the client-side search params).
My team (Data Platform Foundation) has a defect which boils down to unfortunate (or just bad) state management related to search params. One of the issues is that we're tracking states in 3 different places, and we made the search parameters leading. Another issue is that we're not replacing the history like this hook is, so going back is like undo-ing your last filter/date range change. While this seemed like a nice feature when developing it, it is actually a dark pattern (back/forward buttons should be navigation, not undo/redo).
Long story short, I started prototyping replacing the state management we had with using the
useUrlSyncedState
hook. To do so I copied the hook to Admin-UI, and started using it. I ran into the same issue with the incorrect URL update, but fixed it differently as what was proposed in #3991. I basically kept usingwindow.location.href
, but just made the code sensitive to detect the hash route pattern (to my knowledge, it's always a#
hash followed by a/
forward slash). This keeps the benefits of the code always operating on the current URL, even though it doesn't have access to theuseSearchParameters
hook.Arguably it is a bit cleaner to mutate through
useSearchParameters
, but also more cumbersome, so I will leave it to reviewers to judge if they're as confident in this approach as I am.To make the hook more general purpose and hopefully also well behaved and performant, I changed some details as opposed to the previous version:
sync
parameter default totrue
(instead offalse
).To me this is the only logical choice; if you use
useUrlSyncedState
, the default behavior should be that it synchronizes. Otherwise you could just usesetState
.Of course the
useTable
hook is a bit of an exception, as it wants provide this feature as an option, and default to not syncing. So theuseTable
code had to be updated to always resolves thesync
parameter to a boolean value, defaulting tofalse
forundefined
.initialState
once (provided it doesn't change), usinguseMemo
.This is a minor detail, but the previous version of the code would apply the state to the location per field that changed, instead of per state update.
setState
.Potential Breaking Changes
These changes fix the linked defect (usage with HashRouter), and adds the export of this hook (so it is an additive change). While there are some behavioral changes to the
useUrlSyncedState
hook, it was only internal before this PR, so there is no behavior breaking change.Acceptance Criteria
README.md is adjusted to reflect the proposed changes (if relevant)☝️ I didn't really see a README that should be updated, and did my best to add clear TSDoc. Still, please point out if there is a readme that should be updated.