AlexxNB / tinro

Highly declarative, tiny, dependency free router for Svelte's web applications.
MIT License
669 stars 30 forks source link

Fallback not triggered #84

Closed Prinzhorn closed 2 years ago

Prinzhorn commented 2 years ago

So far I'm loving this router, I'm a sucker for declarative code.

When a user is not authenticated, I'm redirecting to /login and pass the page they where on as query param target (should be safe from XSS and open redirect because of pushState).

When an authenticated user hits /login I redirect to query.target or the index page.

But something is broken https://svelte.dev/repl/155bd89d71684fa2a16e1e7fa5ef62d7?version=3.42.1

  1. Hit "Login"
  2. Hit "Logout"
  3. You are in a broken state. $router didn't change. The fallback redirect was not triggered.

I haven't looked at the source at all, I assume <Route> does not properly execute the logic in onMount? In my REPL as soon as you logout this should be triggered <Route fallback redirect="/login?target={encodeURIComponent(meta.url)}" />

AlexxNB commented 2 years ago

Hello! Because of some Svelte's constraints your root Route will check its childs only on first mount. When if triggers it doesn't know, that first Route with '/' was gone and doesn't show fallback. I'll try to add additional checks. But right now you have two ways to avoid this:

  1. Use key block to remount root Route on variable changes.

    {#key isLoggedIn}
    <Route let:meta>
    ...
    </Route>
    {/key}
  2. Or split root for each case of if statement.

    {#if isLoggedIn}
    <Route let:meta>
        ...
        <Route path="/login" redirect={decodeURIComponent(meta.query.target ?? '/')} />
        <Route fallback>404</Route>
    </Route>
    {:/else}
    <Route let:meta>
        <Route path="/login"><button type="button" on:click={() => isLoggedIn = true}>Login</button></Route>
        <Route fallback redirect="/login?target={encodeURIComponent(meta.url)}" />
    </Route>
    {/if}
Prinzhorn commented 2 years ago

Nice, thanks for the workarounds!

your root Route will check its childs only on first mount

Could you explain why this is needed? I've never implemented a router in Svelte. Can't you communicate back and forth using a store inside context? So that the parent doesn't need to imperatively check its children but the children will report their existence and params to the parent? One of the beauties of Svelte and declarative programming in general is that everything is always consistent. But you break this assumption by taking a snapshot of the children.

AlexxNB commented 2 years ago

Main problem it that there is no way to know does component have any childs right now or not. When root Route is mounted it wait when child Routes "say" to him, "Hello, I'm exist, please register me." And then on each URL changes every registered route should inform parent that it match current path or not. When no registered routers matches, it show fallback. In case of if statement from your example, Route with '/' path was gone, but URL didn't change, that is why parent keep thinking that it is still showing.

AlexxNB commented 2 years ago

I think I can fix it.

AlexxNB commented 2 years ago

closed in bc0c210c6a8d0570d0d5e1ad65feba655c4e5d1a

Prinzhorn commented 2 years ago

Thanks!