solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.13k stars 143 forks source link

setSearchParams is reactive #270

Closed danieltroger closed 1 year ago

danieltroger commented 1 year ago

Sorry for going against all codes of conducts and the absolutely terrible bug report. I'm already overdue on a solid-start project and therefore don't have time to report all the bugs properly but I think I have an easy fix for this one and it's quite an obvious mistake, hear me out.

On the following line, reading location.pathname and location.hash is reactive, since location is a store. Therefore when using setSearchParams inside of an effect it will re-execute when the location changes, leading to unexpected behavior. I think that expression (and all accesses to properties of location) in that function should be wrapped in untrack to prevent this. https://github.com/solidjs/solid-router/blob/d0695a715912db2480e082d03e5f0533a8b87e2d/src/routing.ts#LL108C36-L108C36

Or is it intended behavior that effects that call setSearchParams re-execute when the location changes?

ryansolid commented 1 year ago

Yeah this seems wrong. Thanks for reporting. Sorry for not being hasty on this I was traveling for a couple of months.