Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested routes info and problem reproduction #13

Closed
frederikhors opened this issue Jun 3, 2019 · 16 comments · Fixed by #21
Closed

Nested routes info and problem reproduction #13

frederikhors opened this issue Jun 3, 2019 · 16 comments · Fixed by #21

Comments

@frederikhors
Copy link

I apologize in advance if I'm wrong to open the issue and I shouldn't have done it here.
But I think it really concerns the basics of Navaid.

I'm also ready to open and update a wiki page for all newcomers like me.

THE PROBLEM:

I started from svelte-demo (https://github.com/lukeed/svelte-demo) and I'm trying to use nested routes for a list/detail view.

REPRODUCTION: https://codesandbox.io/s/quizzical-dirac-2d0qy

To come to the problem you should:

  • open the console from the tab on the bottom
  • click on "Go to Teams Page"
  • click on "Go to Team 1 Page" (or others)
  • click on "Go to Team 1 Page" (or others) from the list
  • as you can see in console until now just four messages from "App draw()" and one "PageTeamRoute draw()"
  • now you can clear the console
  • click on "Go to Player 1 Page" (or others) from the list
  • you can see now the problem: 4 messages in console:

image

Why?

Am I correctly using Navaid with nested routes?

Is this correct in App.svelte?

.on("team/*", obj => draw(PageTeam, obj))
@lukeed
Copy link
Owner

lukeed commented Jun 3, 2019

I'm pretty sure this is solved by #12 Navaid wasn't initially meant to run listen() multiple times within a page, but there's probably good reason too now.

You can follow the discussion there about current concerns/roadblocks.

Thanks for a very good repro!

@lukeed
Copy link
Owner

lukeed commented Jun 3, 2019

Well, you also have two handlers defined for every team/* route.

Once in the App and once in the PageTeam

@frederikhors
Copy link
Author

Well, you also have two handlers defined for every team/* route.

Once in the App and once in the PageTeam

And how do you think I could do?

How would you do it?

This is one of my doubts.

@lukeed
Copy link
Owner

lukeed commented Jun 3, 2019

Personally I would move everything up to the App router. Since you already have & known all the components ahead of time, to me, it doesn't really make sense to sanction things off into units.

As of now, they're all one App.

For me, the separate (sub)router approach make sense for when you are dynamically loading some variable set of widgets into your application, each of which will have their own routing needs. Aka, unknown, unrelated, and/or physically separate things. For example, in my mind, I'm thinking of a WordPress editor that loads in different content blocks from different authors, each of which has their own series of config/states tied to URL segments.

Since you're developing all of this together, I would define the router together. Personally.

Now, after #12, you can keep this approach but remove team/* from App. That is the redundant one but still fires on every URL change within that namespace

@frederikhors
Copy link
Author

Now, after #12, you can keep this approach but remove team/* from App. That is the redundant one but still fires on every URL change within that namespace

Mmm, ok. So until then I can't remove it from App. Ok I'm not crazy! :)

I will try to all in App now but what I don't understand is how to have the PageTeam which renders internally the PagePlayersList and PagePlayerDetail pages alternatively.

This is the problem. Am I wrong?

@lukeed
Copy link
Owner

lukeed commented Jun 3, 2019

I forked your app and have it so that all routing is under App here: https://codesandbox.io/s/flamboyant-worker-g1zlk

@frederikhors
Copy link
Author

Oh dear. You removed PageTeam.svelte which I need!

In that page I insert other things, from GUI elements to other components.

I need a mother page (PageTeam.svelte) with sons (PagePlayersList and PagePlayerDetail).

This is the problem.

How to do this with all in the App.svelte router? Impossible I think.

@lukeed
Copy link
Owner

lukeed commented Jun 3, 2019

Oh, I was just blasting through it, sorry.

I updated the link: https://codesandbox.io/s/flamboyant-worker-g1zlk

Navaid is completely agnostic to what's happening & up until today/yesterday, there had been no concept of nesting or slots. That's up to the view layer, since it's there that view composition actually happens.

In Svelte's world, this is what <slot/> is for.

@frederikhors
Copy link
Author

frederikhors commented Jun 4, 2019

Ok. Great idea but now <Team> component gets mounted and unmounted every time I switch from List to Detail.

I need it to stay mounted during navigation.

Or am I wrong?

You can try by adding this in Team.svelte:

{Date.now()}

@lukeed
Copy link
Owner

lukeed commented Jun 4, 2019

Right, to me I didn't think it mattered, but I guess it was just because of the simple repro.

This should do what you want then: https://codesandbox.io/s/reverent-tesla-c0kq0

Takes you closer to what you originally had, but just calls router.run on any history events. When #12 lands, you can remove this stuff and just have .listen on the Team's router instance. That effectively does the same thing.


Edit: In the console, you'll see repeat ~> App is drawing :: Team logs, but because the Team component is already mounted, it's not redrawn. I incorporated your Date.now() visual so that you can see it remain unchanged.

@frederikhors
Copy link
Author

Ok, thanks.

I think this is a common need for web apps.

I hope with #12 we can fix this once and for all by using the listen() on the router instance in Team.svelte.

Do you see any performance problem using this approach?

Are we missing some best practice?

Can we avoid the repetition of "~> App is drawing :: Team" and so can we avoid to call that function again every time?

@paulocoghi
Copy link
Contributor

paulocoghi commented Jun 4, 2019

This is the reason why I created a new routing library on top of Svelte. It's not so easy for most of developers to implement routing on it from scratch.

I already have made it using Svelte v2, but I ask you to wait just a few days, since I'll release a new version on top of v3 and make a complete documentation.

The v2 branch was using page.js, but in v3 we will use navaid, that's much much better.

@frederikhors
Copy link
Author

I agree Navaid is much better!

@frederikhors
Copy link
Author

I'm sorry @lukeed.

There are people asking me about this:

Edit: In the console, you'll see repeat ~> App is drawing :: Team logs, but because the Team component is already mounted, it's not redrawn. I incorporated your Date.now() visual so that you can see it remain unchanged.

Can you explain it to me deeply please?

I know with Date.now() it seems ok but now I need to understand it better because with:

  import { afterUpdate } from 'svelte'

  afterUpdate(() => {
    console.log('ComponentPage just updated')
  })

I can see everytime the message in console.

@lukeed
Copy link
Owner

lukeed commented Jul 13, 2019

Where did you add the afterUpdate hook

@lukeed
Copy link
Owner

lukeed commented Jul 13, 2019

You probably added it to App or to Team components, both of which are (in the examples above) are routing components. And because the routing component (RC) needs to update its child whenever something else needs to be displayed, the RC updated.

This is what we had:

  let Route, params;

  function draw(m, obj) {
    console.log("~> App is drawing :: ", m.name);
    params = obj || {};
    Route = m;
  }

New routes assign new Route and params values which are "updates" – these are how the new components (eg, PagePlayersList or PageTeamList) can be displayed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants