AlexxNB / tinro

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

Rendered together routes with parameters and chars #7

Closed frederikhors closed 4 years ago

frederikhors commented 4 years ago

Using this:

<Route path="/player/new">
  <New />
</Route>
<Route path="/player/:playerID" let:params>
  <Show playerID={params.playerID} />
</Route>

or this:

<Route path="/player/*">
  <Route path="/new">
    <New />
  </Route>
  <Route path="/:playerID" let:params>
    <Show playerID={params.playerID} />
  </Route>
</Route>

it renders /player/new and /player/123 together.

Expected

It should render <New /> if I'm on /player/new and <Show /> if I'm on /player/* (everything is not /player/new).

Am I wrong?

AlexxNB commented 4 years ago

"/player/new" and "/player/:playerID" are both exact pathes and should be shown both. You should use other logic in your URL like "/player/new" and "/player/get/:playerID" or "/players/new" and "/player/:playerID". It is more clear to understand not only for router but and for humans who will be read your code after you.

frederikhors commented 4 years ago

I agree.

I'm trying with this code:

<!-- Teams.svelte -->

<Route path="todos/new">
  <New />
</Route>
<Route path="todo/:todoID" let:params>
  <Todo todoID={params.todoID} />
</Route>

but when I use these imports in App.svelte:

<script>
  const Home = import('./Home.svelte')
  const Teams = import('./Teams.svelte')
</script>

<Route>
  <Route path="/">
    <Lazy component={Home} />
  </Route>
  <Route path="teams/*">
    <Lazy component={Teams} />
  </Route>
  <Route fallback>
    <Lazy component={import('./404.svelte')} />
  </Route>
</Route>

it renders on /teams/todo/123 both the <Todo /> and fallback component. Why?

Note I also tried with the code below: the same:

<Route path="todos/*">
  <Route path="new">
    <New />
  </Route>
</Route>
<Route path="todo/*">
  <Route path=":todoID" let:params>
    <Todo todoID={params.todoID} />
  </Route>
</Route>

If I not use import()s it works good.

I tried to reproduce it on Codesandbox (here: https://codesandbox.io/s/tinro-nested-path-two-not-one-107kj) but CSB doesn't work with import()s.

AlexxNB commented 4 years ago

You can reproduce this in the REPL I think.

frederikhors commented 4 years ago

One moment.

frederikhors commented 4 years ago

Here we are: https://svelte.dev/repl/427e3be14358416488945dd29da5ee87?version=3.21.0 but strange things happening...

frederikhors commented 4 years ago

It is working although it opens new windows on each link. But locally it renders fallback and <Todo />.

AlexxNB commented 4 years ago

Yeah, you should use /#/page notation for Links in REPL. Because REPL has own handler for all links inside.

frederikhors commented 4 years ago

One moment.

frederikhors commented 4 years ago

Updated with # and it is working. I can assure you locally it doesn't.

frederikhors commented 4 years ago

One moment.

frederikhors commented 4 years ago

A gif(t) for you!

123456

frederikhors commented 4 years ago

And the same on /new too obviously.

AlexxNB commented 4 years ago

Downloaded and run your example on localhost. The problem is when nested routes are in <Lazy> router can't figure out that the path is exists and show fallback, but when component loaded it shows it too. So no routes should be in dynamic imports(or fallback should be in same import too, and no root fallback), sorry.

frederikhors commented 4 years ago

You can't give up so easily with your genius!

There must be a way to solve it! 😢

AlexxNB commented 4 years ago

Solving is adding feature of async routes in the tinro itself with all handlings. But as I wrote before it is not prioritize. You should use something like yrv if lazyloaded routes are must have thing in your project.

frederikhors commented 4 years ago

I am very sad now. 😢

frederikhors commented 4 years ago

@AlexxNB, I thought that maybe there is a solution.

If tinro is a Svelte Store and the problem is:

when nested routes are in router can't figure out that the path is exists and show fallback

maybe in <Lazy /> component we can use some "flag" to say router that the path exists!

Example

<script>
  import { router } from 'tinro'
  export let component

  router.routeFound = true // <-- THIS ONE
</script>

{#await component}
  Loading...
{:then cmp}
  <svelte:component this={cmp.default} />
{/await}

In this case we can avoid the fallback rendering.

What do you think?

frederikhors commented 4 years ago

Is it an idea you can consider?

AlexxNB commented 4 years ago

Nope, router store is not why tinro works. It is just URL changing handler. You have only this choices at the moment:

  1. Do not use <Route> inside lazyloaded component. Consider to change your architecture:

Before:

<!-- Mypage.svelte -->
I'm the page
<Route path="/page1"><Lazy cmp={import('./Page1.svelte)}/></Route>
<Route path="/page2"><Lazy cmp={import('./Page2.svelte)}/></Route>

<!-- App.svelte -->
<Route>
    <Lazy cmp={import('./Mypage.svelte)}/>
</Route> 

After:

<!-- Pageroot.svelte -->
I'm the page

<!-- Mypage.svelte -->
<Lazy cmp={import('./Pageroot.svelte)} />
<Route path="/page1"><Lazy cmp={import('./Page1.svelte)} /></Route>
<Route path="/page2"><Lazy cmp={import('./Page2.svelte)} /></Route>

<!-- App.svelte -->
<Route>
    <Mypage />
</Route> 
  1. Choose different router from the list.