fix: update current router URL on external location changes

[andreasgerstmayr]

When extension modules bring their own router (e.g. react-router or @tanstack/router), Fava's router is not aware of URL changes, and the router.current property will get stale.

This is a problem when the global filters are changed (i.e. time, account or filter), because then Fava will redirect to that stale URL.

This PR subscribes to all navigation changes, and updates the router.current param of the router.

[yagebu]

I don't like this (overriding prototypes of builtins). What's the use case that you need a router for the extension? We could consider exposing enough of the router to the extension to then enable this.

[andreasgerstmayr]

I don't like this (overriding prototypes of builtins).

We could also use the new navigate event, but browser support is not widely available yet. Therefore other routers (examples: svelte kit, @tanstack/router) monkey-patch the history object.

What's the use case that you need a router for the extension? We could consider exposing enough of the router to the extension to then enable this.

I want to store state (current extension page, chart type, sorting params) in the URL. With a custom router it won't trigger a full page refresh. Adding (sub-)pages in the extension is also possible with a small adjustment of the router.

Alternative solutions

• as you mentioned, Fava could expose (part of) the router to the extension
• Fava could listen to a custom event (fava:navigate for example)
• utilize the (not widely available) navigate event
• change new URL(router.current) to new URL(window.location.href) here:

  fava/frontend/src/sidebar/FilterForm.svelte

  Line 62 in e2c60be

  const url = new URL(router.current);

  - this would fix the issue of redirecting to a stale URL

Which one do you think is best?

[yagebu]

TIL the navigation API exists :) I think it's fine for Fava to only target recent enough browsers (although this only seems to have been in Firefox, my main browser, since the latest version). I guess listening for the navigate event would already fix the problem you are seeing (as you hopefully have a recent browser) and "upgrade browser" is just as good a bugfix or better than "upgrade fava".

However, I'd rather want to improve the APIs for extensions here so that an extension does not have to bring their own router... Will have to think on what that might have to entail, suggestions welcome as well.

PS: I have the feeling that the navigation API might make #2075 actually feasible?

[andreasgerstmayr]

TIL the navigation API exists :) I think it's fine for Fava to only target recent enough browsers (although this only seems to have been in Firefox, my main browser, since the latest version). I guess listening for the navigate event would already fix the problem you are seeing (as you hopefully have a recent browser) and "upgrade browser" is just as good a bugfix or better than "upgrade fava".

Yep, I think it's fine if it only works in the latest browser. The bug is merely an inconvenience, but it doesn't prevent users from using the extension.

However, I'd rather want to improve the APIs for extensions here so that an extension does not have to bring their own router... Will have to think on what that might have to entail, suggestions welcome as well.

I think it's great to have flexibility in the choice of framework (React, Svelte, Vue, etc.), and router. Some libraries (e.g. nuqs) are also convenient to use with a router. Which advantages do you see for users or developers if the extension utilizes Fava's router?

PS: I have the feeling that the navigation API might make #2075 actually feasible?

I think for #2075 we either need a way to skip storing the URL for the modal in the history, or don't change the URL at all when opening a modal.

[yagebu]

I think it's great to have flexibility in the choice of framework (React, Svelte, Vue, etc.), and router. Some libraries (e.g. nuqs) are also convenient to use with a router. Which advantages do you see for users or developers if the extension utilizes Fava's router?

AFAICT, your extension has a couple of "sub-pages" and wants to keep a handful of state in query params. It just feels like having to bring a router for that is excessive. (in particular since these are intrusive libraries built around crap browser APIs and has evidenced here might interfere with Fava's router).

[yagebu]

I had to revert this as it broke back navigation. Seems like merged PRs cannot be re-opened, but we'll have to have another attempt at this...

[andreasgerstmayr]

I had to revert this as it broke back navigation. Seems like merged PRs cannot be re-opened, but we'll have to have another attempt at this...

Ah sorry, I didn't notice that this breaks the back button. I just debugged it, the reason was that the navigate handler set the current URL, and then the popstate handler, which usually handles the back/forward button, did not detect any change in the URL, and therefore did not load the previous page.

I updated the PR in #2209 to handle only push and replace state changes. I tested the back button and it works now.