Wrapping app-router in another element for reuse

[bruce]

First of all, thank you for this excellent component.

I have a number of apps that would require some common routes, so I thought I'd be able to accomplish this with a new element, eg, our-routes that used app-router:

<polymer-element name="our-routes" noscript>
  <template>
    <app-router mode="hash">
      <app-route path="/login" element="our-login"></app-route>
      <content></content>
      <app-route path="*" element="our-not-found"></app-route>
    </app-router>
  </template>
</polymer-element>

An example from an app:

<our-routes>
  <app-route path="/" element="page-home"></app-route>
  <app-route path="/locations" element="page-locations"></app-route>
  <app-route path="/alarms" element="page-alarms"></app-route>
</our-routes>

The problem here is that, while routing to our-login and our-not-found works flawlessly, any app-route inserted via content is ignored. (Not available early enough for app-router to see it? I'll admit to plenty of ignorance when it comes to the intricacies of polymer load order and how that might interact with vanilla custom elements).

Am I approaching this problem incorrectly? Would there be a better way to package up "standard" routes, essentially extending app-router (notably, I couldn't get this working with a polymer extends of app-router, either).

[erikringsmuth]

The concept seems right. That's the correct way to use <content> to insert DOM.

This is the code that loops over the <app-route>s https://github.com/erikringsmuth/app-router/blob/master/src/app-router.js#L188. You could try debugging here and see if it's missing the routes in <content>. Beyond that everything looks right.

[bruce]

My guess is the issue is with route = route.nextSibling -- maybe that needs to take into account how elements are inserted via <content></content>. I'll investigate; thanks for the pointer.

[bruce]

Verified that's the problem.

In short, the structure ends up looking like:

our-routes
  #shadow-root
    app-router
      app-route (our-login)
      content
      app-route (our-not-found)
  app-route (page-not-found)
  app-route (page-locations)
  app-route (page-alarms)

So, the app-routes provided as content end up outside the shadow dom (and aren't actually nested under app-route to be found by nextSibling).

[erikringsmuth]

Hm, the reason I went with router.firstElementChild and route.nextSibling is that router.querySelectorAll('app-route') will select all children <app-route>s. The problem is that a page like page-alarms may also contain a router inside of it and I don't want to select those routes, just the direct decedents of the current router. I'm not sure what to do to select routes projected using the <content> tag.

[bruce]

Yeah, I can definitely understand that.

What about something like: the app-route children of the app-router plus (if I'm inside a shadow dom), the app-route children of the shadow dom's parent?

[bruce]

Actually, since order matters, running into a content should cause the check to occur. I'll do a PR after I get it to work for your review.

[erikringsmuth]

Seeing the code changed my mind about the <content> tag https://github.com/erikringsmuth/app-router/pull/97/files#diff-8f495b0b37d1611ef1e4046e099aa7e1R181. The <content> isn't adding benefit here since it only "projects" the elements at render time and doesn't actually move anything in the DOM tree.

It also doesn't take into account the select attribute like this.

<content select=".routes"></content>

That's not the real issue though. Since it doesn't modify the DOM tree and projection doesn't cleanly help us determine which routes are in the light DOM, the <content> tag is out as a solution. I also found out router.querySelectorAll('app-route') won't select projected content.

I'm not sure what to look at next, but there may still be a way to do something like this.

[bruce]

I'm not sure I see the importance of the distinction between render-time projected elements and elements in the DOM, given that app-route elements can be reliably collected. No, it's not a single call (ie, querySelectorAll -- and while I've seen people use /deep/ and ::content, I'm not sure that would help here unless we limit depth) -- but valid app-route elements would be children of the shadow root .host.

Since select is only used to check the direct descendants (children) anyway, it seems like would be fairly trivial to check candidate elements with, eg, matches() in the while.

[erikringsmuth]

if (element.toString() === "[object ShadowRoot]") { ... and if (route.tagName === 'CONTENT') { ... may work, but I really don't like how hacky it is. If a selector like router.querySelectorAll(':host /local/ > app-route') were defined in the future where /local/ queried the children including projected children, that'd be ideal, but this feels like a big hack at this point.

[bruce]

I agree it's "hacky" (although, the tagName checking is exactly what you do for APP-ROUTE above, and I mentioned alternatives for the toString in the PR if iframes aren't a concern) -- but necessary (as far as I can see) precisely because the facility you want doesn't exist yet.

I'll give a shot later in the week and tracking down something more direct. In the meantime I can use app-router for our apps off my fork, treating this as I would a polyfill.

Thanks for looking at it.