Compatibility with react-use useSearchParam #164
Conversation
|
@molefrog I discarted the commit that changed the event names declarations and made it so that they are transformed to lowercase when registering listeners. Do you think this is ok? This way |
| @@ -6,7 +6,7 @@ import { useEffect, useRef, useState, useCallback } from "./react-deps.js"; | |||
| const eventPopstate = "popstate"; | |||
| const eventPushState = "pushState"; | |||
| const eventReplaceState = "replaceState"; | |||
| export const events = [eventPopstate, eventPushState, eventReplaceState]; | |||
There was a problem hiding this comment.
I also remove this export statement because it isn't being used by any other file
There was a problem hiding this comment.
Well, this is intended, although not documented. Some apps use the name of the events in order to subscribe to their own location changes. I'm thinking about removing it in the next major release though.
|
The |
|
Hey @joaopaulobdac, I don't think the names of these events really matter. The events that wouter emits are virtual and non-standard: there is no built-in way to subscribe to location changes, so wouter matches native methods and triggers custom events. If both wouter and react-use patch the History API, they should play along well. Take a look at #169, I have a feeling that the reason wouter didn't react to search changes was that it applied caching that ignore the search query. |
I think that one assignment overrides another. If I do |
Not really, the original method (prior to patching) is always called with the same arguments before an event is triggered. and wouter does it here: https://github.com/molefrog/wouter/blob/master/use-location.js#L64 In the end, it creates a chain of hooks, so both wouter and react-use events should be fired. The worst thing that could happen is that they could have identical names for the events, but that's fine: the app will be rendered twice which isn't critical in case of a React app. |
|
There are some weak points that patching has, well described in this issue: #167 However, these are mostly performance-related concerns. |
|
Great, I'll close this since it seems that with #169 we can react to changes to search params with |
ghost
closed this
|
@joaopaulobdac There is a new version that I've just published v2.7.2, hope it helps to resolve the problem. |
I commented about this on #58 , so I thought I should open a PR to fix the problem I ran into. Currently, wouter doesn't have support for search params so I tried to use the
useSearchParamhook from react-use, problem is that wouter and react-use added listeners for different events, making them incompatible sinceuseSearchParamdoesn't update when wouter does.Plus, this change also follows the naming convention of window events.