router icon indicating copy to clipboard operation
router copied to clipboard
verified publisher icon TanStack

Invalid search param causes loaders to run twice

Open go3323 opened this issue 1 year ago • 2 comments

Describe the bug

When navigating to a route containing search parameters and passing a search value which cannot be deserialized, the beforeLoad and loader methods of each match in the route tree run an additional time.

Your Example Website or App

https://stackblitz.com/edit/tanstack-router-rxn37z?file=src%2Froutes%2Fhas-search.tsx

Steps to Reproduce the Bug or Issue

Bug

  1. Open the StackBlitz example app
  2. Open the browser console F12
  3. Append /has-search?list=aaa to the StackBlitz URL (manually copy and paste into URL bar)
  4. Within the browser console, observe two loader messages and two beforeLoad messages

Correct behavior as reference

  1. Navigate to /has-search?list=%5B%5D (the list param is a correctly serialized [])
  2. Within the browser console, loader and beforeLoad are shown to only be called once each

Expected behavior

I expected all loader and beforeLoad methods to always be called only once regardless of whether the search params of a specific route match could be deserialized correctly.

Screenshots or Videos

No response

Platform

  • OS: Windows
  • Browser: Chrome
  • Version: 1.12.7

Additional context

This behavior only occurs when Strict Mode is enabled. However, I believe this behavior still needs to be adjusted since it is inconsistent - within Strict Mode, valid deserializable params cause loaders to run only once, while invalid params cause loaders to run twice.

go3323 avatar Jan 26 '24 05:01 go3323

Please try with the latest version. I believe we may have fixed this from a different issue.

tannerlinsley avatar Jan 26 '24 07:01 tannerlinsley

The issue still appears to be present in the latest router 1.12.12.

go3323 avatar Jan 26 '24 07:01 go3323

A few things after studying this out:

  • The router detects an "invalid" URL that is inconsistent with a schema, so it needs to do a replaceState and fix it
  • beforeLoad calls should be prepared to be called more than once if necessary
  • We can't remove this "fixing" stage without severely impacting other features.

I think the better issue I would like to see is "What are you attempting to do in beforeLoad that you don't want to run twice?"

tannerlinsley avatar Apr 10 '24 20:04 tannerlinsley