Discussion: leaky implementation for pushState/replaceState events

[jvdsande]

Hi!

Looking at the code for useLocation, I wonder if we can do something about the way pushState and replaceState events are currently handled.

Basic issue

Basically what I find problematic, is the fact that the monkey-patching is done eagerly. While I understand that this is needed for Wouter to correctly react to history changes triggered from outside Wouter, I wonder if we could somehow make this behavior opt-in (without having to rely on a custom useLocation hook)

Real-world problematic use-case

The main use-case that bothers me is when using Wouter inside a Micro-frontend architecture such as a Unified Single Page Application.

If multiple microfrontends all bundle their own version of Wouter, the replaceState and pushState event will get monkey-patched several times, resulting in multiple events being fired at each call.

Possible solutions

Flagging

We could maybe simply add a flag to replaceState/pushState (or even directly on window) to mark that they have been patched by Wouter. This way, subsequent Wouters won't patch them again.

The downside is that does not allow users to opt-out of the patching altogether, it really only focuses on the microfrontends use-case.

Exposing a pure endpoint

This bit of code in useLocation is actually the only part of Wouter that makes it un-pure. Extracting it out of useLocation's base code would allow us to provide two entry points. Either users import "wouter" or "wouter/useLocation" to get the default behavior, or they import "wouter/pure" or "wouter/pure/useLocation" to get the side-effect-free implementation.

The upside is that it allows true opt-out of the side-effect, so it covers all use-cases when such an opt-out is needed.

The downside is that it makes the export API a little more complex because of those new entry-points. It might also make the code more complex, as right now Wouter's internal routing relies on those events too. With this solution we might want to add a call to checkForUpdates in the default navigate function.

Making it pure by default

Finally, this last option is a bit more drastic. We could export a setupEventListeners function with this side-effect code, and have wouter/useLocation be pure by default. Users could then opt-in for event listeners by calling this somewhere in their app.

The upside is that it allows true opt-out of the side-effect, so it covers all use-cases when such an opt-out is needed, while maintaining simple entry-points.

The downside is that most users might want to opt-in, so this would be cumbersome for most. I personally don't think this is the best idea, I just put it here for the sake of completeness.

---

Happy to hear your thoughts on this @molefrog :)

[molefrog]

Hi @jvdsande ! I anticipated this 😄 I currently lean towards flagging, as it seems like the most user-friendly method, although not the purest one (modifying the environment is always a bad idea, although we already do that with patching)...

[molefrog]

Perhaps, we could use something like:

Object.defineProperty(history, Symbol.for('wouter'), { value: 1 })

(might need to test that under different conditions, not sure that history is mutable everywhere)

[jvdsande]

I think we can safely use this snippet. If an environment does not support writing properties to window.history, the patching wouldn't work in the first place anyway :)

[HansBrende]

@jvdsande @molefrog fyi... it is possible to check this already if you are in need of a quick fix.

const isThereMonkeyBusiness = window?.history?.pushState.name === ''

[jvdsande]

Hi @HansBrende :)

In the end I never followed up on this after opening the initial issue. It has not proven a big problem in my production environment yet. Definitely something that I'll follow more closely if we start seeing strange effects or bottlenecks.

The issue I see with your code is that is looks for any monkey patching of the window.history object, not specifically matching done by Wouter. If the history object has been touched by another third party, we still want Wouter to apply its logic on top.

[HansBrende]

@jvdsande yup! Agreed: it's suitable for a "quick fixes" only. A robust implementation will involve symbol for sure.

[jvdsande]

Thinking about @molefrog's comment on window.history mutability, I guess the simplest workaround would be to store the "symboled" entry... On window itself. At least we're sure it's mutable everywhere

[molefrog]

Thinking about @molefrog's comment on window.history mutability, I guess the simplest workaround would be to store the "symboled" entry... On window itself. At least we're sure it's mutable everywhere

This actually sounds like a good idea. I would also add a major + minor version number to the symbol name in case if there are two instances of the library.

@jvdsande @molefrog fyi... it is possible to check this already if you are in need of a quick fix.

const isThereMonkeyBusiness = window?.history?.pushState.name === ''

Nice one! I agree with @jvdsande on that it wouldn't cover cases when history is patched by another library.

[jvdsande]

For the major + minor version... Is it really wise?

In the main use case I see (Independent microfrontends), minor version can easily drift, and in some cases, major version too.

But the logic for patching the history object will most likely stay the same between versions, it's not the code most likely to change. So by suffixing the symbol with the version we will just make it again apply the same patching logic, everytime a different version is loaded.

Maybe instead we should have an independent versioning scheme specifically for this symbol? This way it would stay consistent across versions, and we can bump it whenever the actual patching implementation changes. What do you think?

[molefrog]

This has been fixed in #350 and is now published as v3.0.0-rc.1