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

[Feat] Investigate changing route component to a structural directive #37

Open
brandonroberts opened this issue Jul 23, 2020 · 4 comments

Comments

@brandonroberts
Copy link
Collaborator

Currently we use a route component to define a route.

<route path="/">
  <my-component *routeComponent></my-component>
</route>
  • This has an easy mental model for registering a route
  • Provides an easy way to pass route params as inputs.
  • Provides a placeholder for rendering lazy loaded components
<route path="/" #myComponenRoute>
  <my-component *routeComponent [params>="myComponentRoute.routeParams$ | async"></my-component>
</route>
  • Extra DOM elements that have to rendered
  • It wraps the actual component, which might make it more difficult to style
  • Can't use display: none on the route element

This effort would look at potential options to use a structural directive to register the route, and have the route component rendered in its place with no wrapper.

Thoughts

  <my-component *route path="/home"></my-component>
  <ng-template *route path="/lazy" [load]="components.lazy"></ng-template>

Reference: https://twitter.com/LayZeeDK/status/1285969664917614592?s=20

@LayZeeDK
Copy link

LayZeeDK commented Jul 23, 2020

Maybe it could even be a structural directive on the <router> element level, that is *router instead of <router>. This structural would be a responsible for managing the individual routed components.

<ng-container *router>
  <!-- For nested routes use exact: false -->
  <route path="/blog" [exact]="false">
    <app-blog></app-blog>
  </route>
  <route path="/posts/:postId">
    <app-post></app-post>
  </route>
  <route path="/about">
    <app-about></app-about>
  </route>
  <route path="/" redirectTo="/blog"> </route>
  <route path="/" [exact]="false">
    <app-page-not-found></app-page-not-found>
  </route>
</ng-container>

Not sure if this is possible. It would be a more complicated implementation, but it might be an easier developer experience to just apply one structural directive instead of one on each routed component.

@meeroslav
Copy link
Collaborator

From the ease of development and readability, I would vote for current functionality, over structural directives for router and route.

However, adding structural directives as an option would be great, unless we cripple the implementation by doing so.
I'll give it a go these days and see what comes out of it.

@LayZeeDK, @brandonroberts: what would be the use-case for using display: none on route element? It does not render anything meaningful apart from being a wrapper to inner child component.

@LayZeeDK
Copy link

LayZeeDK commented Aug 15, 2020

what would be the use-case for using display: none on route element? It does not render anything meaningful apart from being a wrapper to inner child component.

The default for custom elements is display: inline;, so there is a small cost to adding elements to the DOM, even empty elements. Angular components are hopelessly tied to the DOM. The DOM of a dynamic page gets slower the more visible elements are added because of memory consumption, render, layout, and paint.

@meeroslav
Copy link
Collaborator

what would be the use-case for using display: none on route element? It does not render anything meaningful apart from being a wrapper to inner child component.

The default for custom elements is display: inline;, so there is a small cost to adding elements to the DOM, even empty elements. Angular components are hopelessly tied to the DOM. The DOM of a dynamic page gets slower the more visible elements are added because of memory consumption, render, layout, and paint.

In short - DOM rendering performance. I thought there was some specific use case in mind. Thanks for the clarification.

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

No branches or pull requests

3 participants