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

Support eagerly configuring child routers #90

Open
bryanrsmith opened this issue May 8, 2015 · 46 comments
Open

Support eagerly configuring child routers #90

bryanrsmith opened this issue May 8, 2015 · 46 comments
Assignees
Milestone

Comments

@bryanrsmith
Copy link
Contributor

Lazily configuring child routers has a couple downsides:

  1. We can't support hierarchical nav menus because the application's route structure isn't known until all child routes are activated.
  2. More importantly, we can't support generating URLs for routes in "sibling" routers for the same reason.

If we let child routers be eagerly configured we could implement features that require knowledge of the whole apps routing structure. Ideas:

  1. We could let route configs specify child router configs inline, so an App module could configure a larger hierarchy of routes.
  2. We could let route config objects indicate that their modules will configure a child router that should be eager-loaded.
@sorenhoyer
Copy link

+1 this would indeed be a nice enhancement. :)

@aozisik
Copy link

aozisik commented Feb 17, 2016

I would love this, as well. Until then, is there a workaround one could use?

@npelletm
Copy link

+1 for this feature, at least a workaround to avoid first route routing issue.

@larsfjerm
Copy link

+1 - fingers crossed for this!

@Cedware
Copy link

Cedware commented Feb 24, 2016

+1 configuring routes in the ViewModel isn't really pretty

@danlourenco
Copy link

+1

@npelletm
Copy link

For the next RC could we expected this feature ?

@claylaut
Copy link

+1

@bryanrsmith
Copy link
Contributor Author

Sorry, but I don't know of any plans to work on this before 1.0. I really want it too, but I haven't had much time to spare lately. If anyone wants to work on a PR I'd be more than happy to discuss the necessary changes and answer questions.

@ngelx
Copy link

ngelx commented Apr 3, 2016

+1

@bhatnagar-ankur
Copy link

+1

@ogliznutsa
Copy link

+1, any plans on this feature?

@EisenbergEffect
Copy link
Contributor

We are working towards v1 release in the next few weeks. After that we'll look at adding some new features to the router. This is high on the list.

(As this is open source, all community members are welcome to implement this and send a PR. That helps to move certain requests ahead faster.)

@heruan
Copy link

heruan commented Aug 4, 2016

I think the solution for this and #89 should address a more general scenario. Aurelia is so modular it comes natural to have an app built upon different external modules which may contribute to the route hierarchy. For example:

/       (main-app)
/admin  (admin-app)
/blog   (blog-app)
/wiki   (wiki-app)

Then blog-app and wiki-app may depend both on admin-app, and their view-models might need to generate a route to a view-model in admin-app (for example a user's profile). Problem is, the code of both apps do not know how main-app would configure the base path for admin-app (could be nested too)! The only assumption an app can make about a route of another app it depends on is that it should be mapped somewhere in the hierarchy.

The solution I propose in #89 (comment) could work for this if it's supposed to start from the AppRouter (which AFAIK is the root router of the app) and its generate function which would take a path of route names to let the router navigate through the path building and configuring the needed children and expanding the route templates it encounters with provided parameters. For example, a view-model in blog-app may call router.generate('admin-app/users/profile', { id: user.id }).

I'm willing to work on this, but since it's a bit of a work and I need to study the router internals, I'd like to know if it could be an accepted solution or not 😉

@EisenbergEffect
Copy link
Contributor

@heruan We'd love to have you work on it. It's a very tricky problem, especially since the child apps may not even be loaded and you've got to account for parameters and wild cards at various levels.

@heruan
Copy link

heruan commented Aug 4, 2016

I'm just studying the RouteLoader logic and since loading is involved it's clear that generate will never be able to return synchronously. I'm evaluating a more straightforward approach, of a RouteMapper which would be built with all the involved routers configurations and thus have a static hierarchy of url templates. I'll publish ASAP a repo with examples.

@thomas-darling
Copy link
Sponsor

thomas-darling commented Aug 4, 2016

Yeah, whatever is done here, it has to be considered how it might affect bundling.
Lets say we have the root router, and a child router for each area of the site.
The app would also be bundled this way; one core bundle, and one bundle for each area.

To make this work, I think we would need to move the configureRouter function to a separate file, such that this file can be included in the core bundle, despite being physically located in the directory of an area, thus keeping the solution nice and organized.

Having the configureRouter function in the view model of a router view is problematic, as the view model may, for whatever other reasons, need to reference other things within the area-specific bundle, thus preventing us from including it in the core app bundle - because if we did, it would effectively end up pulling the entire area bundle into the core bundle.

Maybe this could be as simple as moving the configureRouter function to an index.js file in the area folder (i.e. next to the view model it would currently be defined in), and simply have the router eagerly look for that file by convention? Then we could include those index.ts files in the core bundle.

Of course, this "eagerly look for index.ts" behavior should be explicitly enabled with a per-route option - this way it won't break existing code, and it allows some routers to be eagerly configured and others lazy.

Maybe this could somehow tie in nicely with aurelia/framework#427?

Does that make sense?

@EisenbergEffect
Copy link
Contributor

I think this could be a good approach. One idea I had, just off the top of my head, was very similar. Basically, enable a separate mapper or some additional config at the app level that would provide info for generation. Child routers could maybe reference this information so they didn't need to repeat themselves. It's a difficult problem. We are open to ideas.

@heruan
Copy link

heruan commented Aug 4, 2016

I came up with this: https://github.com/heruan/aurelia-route-mapper
and this example app: https://github.com/heruan/aurelia-route-mapper-example

The concept is to use a RouteMapper which is an extended RouteRecognizer and has a map(routes: RouteConfig[]) function which adds to its recognized routes the ones passed as the argument, but also looks for a settings.childRoutes and recursively adds them prefixing names and routes accordingly.

Any external application or submodule could just export its routes and the main application will configure the RouteMapper.

@Kukks
Copy link

Kukks commented Oct 24, 2016

@heruan Your solution has worked really well for me! Are you planning any more work on it?

@heruan
Copy link

heruan commented Oct 27, 2016

@Kukks Yes, I'm currently working on a JavaScript object-mapper but I'm open to suggestions on how to improve the route-mapper!

@deviprsd
Copy link

deviprsd commented Nov 10, 2016

@thomas-darling I also had the similar idea, how about just making only the routes a separate file like symfony and not the whole function which could be basically models/data holders. So this way maybe it will be cheap on resources eagerly loaded.

@smithaitufe
Copy link

@heruan Thanks for the good job. I tried the demo you provided and it worked nicely. @EisenbergEffect Can we go ahead to use this? Any idea if this will be added to one of the releases soon? Thanks

@EisenbergEffect
Copy link
Contributor

@bryanrsmith I'd love to see what you think of the solution that @heruan has come up with. It seems like we could build this into Aurelia without much work if we think it will solve the problem. Let me know what you think.

@activebiz
Copy link

I found this to be good solution and simple and its working ...

http://www.jeremyg.net/entry/create-a-menu-with-child-routes-using-aurelia

https://github.com/jagonalez/aurelia-menu

@Kukks
Copy link

Kukks commented Feb 23, 2017

@heruan I've ended up using your route-mapper and its concepts and made a package for my use case where I can generate child router applications registered without loading them.
https://github.com/Kukks/aurelia-modules-example
The examples themselves may be a little out of date as I'm working on the src in another private project as part of the solution.

@tdamir
Copy link

tdamir commented Feb 23, 2017

I have a modified version of solution suggested by @activebiz. First we have to gather all the routes from all modules and after that we can use the service to generate the parent/child route combination like this:
routeGeneratorService.generate([{ route: 'products', params: { id: 1 } }, { route: 'top', params: { top: 100 } } ])

The code: https://gist.github.com/tdamir/34861b86c015e3fb5c0e25d477b11bbb

@jagonalez
Copy link
Contributor

jagonalez commented Feb 23, 2017

Thanks for the mention @activebiz

Just my two cents but I believe the solution proposed by @heruan is more elegant.

As I mentioned in my blog post using the composition engine loads each module in the route config which could cause performance issues.

I don't believe it'll retrieve any routes added using the router instead of the RouterConfiguration.

@tdamir
Copy link

tdamir commented Feb 24, 2017

I agree that loading modules in advance could cause performance issues and is a very hacky. I also agree that the solution @heruan proposed is more elegant and not that hacky. I might be missing something but I think that the route-mapper cannot handle parent+child navigation properly if the name of route parameters is the same (eg. parentRoute/{id}, childRoute/{id})?

@tdamir
Copy link

tdamir commented Feb 25, 2017

I've forked a route-mapper and handled the problem I mentioned before. If someone needs it: https://github.com/tdamir/aurelia-route-mapper

@dasch88
Copy link

dasch88 commented May 6, 2017

Any update on when/if we can include any of these solutions into Aurelia itself so others can use it more easily? This seems like a huge win to just let it sit here in limbo. Any thoughts @heruan, @bryanrsmith, @EisenbergEffect?

@jeffgrann
Copy link

I agree with @dasch88. @heruan's solution should be included in Aurelia ASAP unless there is something better that is being considered or worked on.

@tbnovaes
Copy link

+1 I just struggle with that few days ago. @bryanrsmith @EisenbergEffect any plans of releasing this soon?

@ordinaryorange
Copy link

@heruan are you doing anything on this still ? I've got a working solution with inspiration from your alpha, and from the nested navigation @jagonalez made. The library is a plugin that I think meets everyone's needs here. I hope to have the git repo up soon once I've finalised some aspects. Will post back here when done, am keen to get reviews.

@fkleuver
Copy link
Member

I recently created a little plugin (npm i aurelia-router-metadata) to provide router configuration through decorators, basically to replace most configureRouter code with convention-based configuration.

Eager loading of childRoutes wasn't my initial plan with it, but it turned out to be really simple to add in.

Essentially there's two decorators (they can both be applied to the same class):

  • @routable({..}) configures a ViewModel to expose a RouteConfig
  • @mapRoutables([PLATFORM.moduleName(".."), PLATFORM.moduleName(".." ]) configures a ViewModel to grab the RouteConfigs from specified modules and map those to its own router.

A quick run-through of how this works:

  • The @routable() decorator generates a RouteConfig based on information found on the class and passed into it. It uses aurelia-pal to find the moduleId of the class it's applied to, then uses that as the key to store the RouteConfig in a metadata resource using aurelia-metadata

  • The @mapRoutables() decorator needs these moduleNames passed into it. When configureRouter is called it tells aurelia-loader to load them, which invokes their decorators (recursively, if eager loading is enabled) and so the whole routing tree is stored in metadata after the root's configureRouter is done.

You can access the child routes through NavModel.config.settings.childRoutes and combine the route property with the parent nav's href into full URL's, and let the router take care of the rest.

While the modules are loaded upfront in order to execute all the decorators, I don't believe this will impact initial load time too much. Apart from loading the modules, nothing really happens (no component lifecycles are invoked, no child routers are configured, etc)

I'm not really sure where to take it from here, so I'm very much open to suggestions and feedback.

@EisenbergEffect @bryanrsmith My idea with this is to have a sort of extra "metadata layer" on top of the router to enhance its configuration and exposure capabilities, rather than actually adding functionality to the router itself. In that light, does it make sense to keep this in a separate package (I could rename it to something else if you don't like aurelia-router-metadata) or do you see this as potentially part of aurelia-router? Another consideration for me is that I'm simply more comfortable with TypeScript..

@EisenbergEffect
Copy link
Contributor

Hey @davismj Can you take a look at this? Seems interesting.

@davismj
Copy link
Member

davismj commented Feb 16, 2018

@fkleuver This is exactly what I had in mind from the start. It's going to be very at home with the .NET crowd.

@davismj davismj self-assigned this Feb 16, 2018
@fkleuver
Copy link
Member

I've uploaded a sample project to showcase how to utilize this for building a navigation menu. I'm using a value converter to traverse the routeconfigs and build the url which is still a bit hacky imo. Should I add in some route generator functionality to make the href part a bit cleaner? Opinions?

@3cp
Copy link
Member

3cp commented Feb 18, 2018

I come to the party quite late. I have implemented hierarchical nav menus since 2016 when I picked up Aurelia, and I had totally different view on this problem.

To me, having delayed router config is NOT a bad thing. Dynamic behavior is good, is flexible, please don't "FIX" it.

In my view, the problem here is not to show hierarchical menus of hierarchical router config. "Hierarchical menus" is pure view logic, is only presentation, you don't need to have a nested matching router model.

Here is how my top level router config would look like.

{nav: true, route: '', name: 'home', ...},
{nav: true, route: 'products', name: 'products', ...},
{nav: true, route: 'manage/products', name: 'manageProducts', settings: { group: 'Manage'} ...},
{nav: true, route: 'manage/users', name: 'manageUsers', settings: { group: 'Manage'} ...},
{nav: true, route: 'settings/plugins', name: 'pluginSettings', settings: { group: 'Settings'} ...},
{nav: true, route: 'settings/webhooks', name: 'webhooksSettings', settings: { group: 'Settings'} ...},

Instead of repeat.for="row of router.navigation", I do repeat.for="rowOrGroup of groupedNavigation", you can figure out the rest of the story. You can imagine there are not many code needed.

@fkleuver
Copy link
Member

fkleuver commented Feb 19, 2018

@huochunpeng

To me, having delayed router config is NOT a bad thing. Dynamic behavior is good, is flexible, please don't "FIX" it.

I completely agree with you there. And the example you gave does work fine in most smaller projects.

"Hierarchical menus" is pure view logic, is only presentation, you don't need to have a nested matching router model.

Maybe not for a shallow admin panel or line-of-business application where you know the structure upfront.
Consider e-commerce, CRM, ERP and other "category/data-heavy" types of web applications. I've worked on several large projects of this sorts with Aurelia, and I can guarantee your approach isn't always feasible.

This is a key point for me:

It's about automation and composition

To create a large-scale dynamic data-driven website completely in Aurelia with limited time and resources, manually keeping multiple layers in sync (server-and-client, view-and-viewmodel, navigation-and-routerconfig, etc) is the biggest time sink by a mile. How well this is managed is literally what makes or breaks a project.

I use code generation for things like services, models and view-models based on C# (long live Roslyn), and have lots of small composable template parts to let Aurelia stitch it all together based on the data at run-time. Aurelia's strength, for me, lies in its amazing ability to compose. It's like having a DSL for the UI.

Except when it comes to navigation?

It seems like it always needs to be done from scratch (more or less) so this is where I currently spend 80% or so of my time.
For data-driven back-end applications I ended up making width-oriented recursive navigation (think portal.azure.com) where each view is a narrow list of properties or navigable items. It's perfect for large and complex back-end applications.

You can't do this to visitors of a webshop, you want to give those extensive exploration options. I for one hate it when I need to click through multiple times just to see the next level of categories. And you can't do it to yourself to put all of that in the top-level route config settings when you've got a million products with arbitrarily deep category structures. You can't really generate that either (generation works better with "one per file" principle). And having a separate model for the menu means you don't have things like .isActive and properly generated routes.

Conclusion:

  1. It can make perfect architectural sense to let Aurelia recursively generate a big menu upfront
  2. You need some form of (selective) eager loading for that, and
  3. Currently it doesn't have this capacity
  4. There are plenty of lucrative business cases (amazon has a brilliantly working navigation but they also have the resources to it with whatever's available)

So I'm committed to it for one.
I've tried a dozen different approaches and haven't quite found the sweet spot yet - nor have others by the looks of it. So now I try to generalize bits and pieces of my own solutions into a plugin, which I'll combine with solutions others have proposed, and mess around with it until it works in a way that makes sense and fills the "gap"

@davismj maybe this is really a templating-router concern?

@3cp
Copy link
Member

3cp commented Feb 19, 2018

@fkleuver I understand your argument, you want the framework to help to organize the code.

My argument is, I might not need much from the framework in order to meet this. Since your complex structured routes are static anyway, why not push them up to top level routes definition. You can have some code to automate the routes creation without fighting the framework.

This is just my view, I understand everyone have different opinion and approach. And aurelia is permissive enough to tolerant me :-) I was very hard to breath in react.

BTW, we have something in common. I hate AZURE portal ui too with a passion (if that's what you mean).

FYI, my front-end apps are not small projects. The following stats is for (biggest) one of my apps. I have separate front-end domain logic lib and shared front-end ui lib (they have about 8k lines in addition). Similar to what you described, my apps are heavily driven by meta-data to compose the view. Just put here for reference.

~/b/bcx-broker-au-frontend on master ◦ cloc src/
     679 text files.
     676 unique files.                                          
       3 files ignored.

github.com/AlDanial/cloc v 1.74  T=3.26 s (207.1 files/s, 16343.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JavaScript                     343           4951            353          27948
HTML                           312           1288             24          14444
Sass                            21            816             24           3501
-------------------------------------------------------------------------------
SUM:                           676           7055            401          45893
-------------------------------------------------------------------------------

@davismj
Copy link
Member

davismj commented Feb 19, 2018

@fkleuver I think this is a 2.x level change. Appreciate the feedback, extremely useful.

@Kukks
Copy link

Kukks commented Feb 19, 2018

@fkleuver I also had a much more rough and primitive automatic router configurator but I ran out of time to work on it and the tracing breaking change(PLATFORM.moduleName) wreaked havoc on it and made me scrap it. You can find an example repo of its usage here

@fkleuver
Copy link
Member

@huochunpeng

you want the framework to help to organize the code.

Correct, and I acknowledge that in the end this boils down to coder preference and experience.

I put a lot of work into my codegen templates to make the output as robust and feature-rich as possible. Popular tools such as the Visual Studio TypeWriter extension and T4 templates generate one output file per input file. I've had to resort to wonky build scripts to concatenate some of these outputs and you get all these "don't touch it once it works" artifacts that (for me) make a project more volatile over time.

However even without code generation, I quite fancy the idea of creating the pages that I want, configuring everything related to those pages on those pages, then in my app just reference the moduleIds and everything just wires up like magic. Navigation included.

Aurelia already works like that for the most part (it does SO. MUCH. STUFF. under the covers) so why not have that magic for routing as well, is my thought here.

@Kukks That's a pretty neat concept, thanks for the link!
PLATFORM.moduleName (with webpack) was the direct trigger for me to create my plugin. I was dynamically creating creating routes in code and webpack suddenly starts tree-shaking off all my pages unless they've got those long unparameterized PLATFORM.moduleName calls. So if they need to be there, then they'll be the ONLY thing I'll have there and then it's acceptable I suppose.

@fkleuver
Copy link
Member

@davismj makes sense. Has TypeScript been considered for a 2.0 release as well? I converted it as a learning exercise and feels promising to me. The type checking exposes everything ruthlessly :)

@fkleuver
Copy link
Member

I've made several updates to the plugin and in it's current form is actually serving me pretty well in a moderately sized app. Using it for the decorator convention-based configuration though, so I haven't really tested the navigation-templating side of things that much.

I'm guessing next steps are to make the regular configureRouter() completely replaceable with the decorators so it's just a matter of exposing more api and configuration. Core functionality seems to be good, but of course I would love to hear other opinions :)

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

No branches or pull requests