Routing satisfied in two different sub-pages

[mercmobily]

I have this in my-app.html:

  <app-location route="{{route}}"></app-location>
  <app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subRoute}}"></app-route>

It's a fairly standard routing.

One of the sub-pages my-gigs.html, which is loaded dynamically (ala PSK) and contains:

    <iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404">
      ...
      <my-gig
        name="gig"
        route="{{subRoute}}"
      </my-gig>

      <my-city
        name="city"
        route="{{subRoute}}"
      </my-city>

In my-gig.html, I want to be able to view a specific gig depending on the URL. So, I have:

<app-route route="{{route}}" pattern="/:gigId" data="{{routeDataGig}}" active="{{activeGig}}"></app-route>

The trouble is that when I go to a different page in the application (say /city/perth-au, being displayed from my-city.html), the route above is active (!). It will have, as gigId, perth-au (which is totally wrong, since it's obviously NOT a valid ID).

I am lost. Whenever I had problems before with routing, I would use the active flag to check if the route was actually active. In this problem, I have a problem because /city/perth-au and /gig/445454545 will BOTH satisfy the routing in my-gig.html.

What I am doing wrong...?

[mercmobily]

Ad the moment I am "solving" this by having a check in my-gig.html:

         <iron-ajax
            id="ajax"
            auto
            url="{{_makeUrl(routeDataGig.gigId)}}"
            handle-as="json"
            debounce-duration="300"
            last-response="{{response}}"
          ></iron-ajax>

And then:

  _makeUrlIfNotInfo: function(gigId) {
    if( this.route.prefix != '/gig') return;
    return '/stores/gigs/' + gigId;
  }

But seriously...

[TimvdLippe]

I have encountered this issue as well and maybe we can implement the solution in iron-pages: What if we add a new property attribute-selected-value that is the value to be set for the attribute-selected on the selected items. Therefore you can do

<iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404"
  selected-attribute="route" selected-attribute-value="{{subRoute}}">
      <my-gig
        name="gig"
      </my-gig>
      <my-city
        name="city"
      </my-city>
</iron-pages>

This is less code to write and fixes your issue.

@bicknellr would you accept a pullrequest that implements the behavior described above?

[akc42]

After discovering several issues with app-route, this being one of them I ended up implementing an alternative. I am not particularly promoting this here, but you can find it on my github account if interested (akc-route). What is worth mentioning is that I implemented an additional parameter ifMatched which takes two strings colon separated. The left string is the parameter name and the right hand is the value.
This only works because I define a route as also containing the parameters from the previous route in the chain. BUT , since with app route it has the route-prefix, and it would be possible to have a parameter which required the last segment of the prefix to match a particular value.

Back when I was still using <app-route> I found that with iron-pages and the selected-item attribute, you could pass that along with the tail from app-route into the contained pages, and then check for selected=this as well as a matching route inside.

[mercmobily]

@TimvdLippe Are you proposing the behaviour purely so that you can save the one line:

route="{{subRoute}}"

...? How would it fix my issue in terms of stopping my-gig from thinking that the route was matched...?
Maybe I am missing something...

@akc42 I really do see the point of your module. However, I looked at Polymer 2, and I saw that they finally implemented atomic changes to multiple properties, making things easier. Since we will be transitioning Gigsnet to Polymer 2 sooner rather than later...

I must admit I found app-route beautiful in theory, but painful in practice -- especially when writing a complex, large one-page application.

[TimvdLippe]

@mercmobily Since selected-attribute is only set on the currently selected item (in your case a page), this means that when a page is not active, it's selected-attribute will be {}. When a page is active, it will be subRoute

[mercmobily]

Ah, I had totally missed it, good point!

…

On 5 December 2016 at 19:51, Tim van der Lippe ***@***.***> wrote: @mercmobily <https://github.com/mercmobily> Since selected-attribute is only set on the currently selected item (in your case a page), this means that when a page is not active, it's selected-attribute will be {}. When a page is active, it will be subRoute — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#164 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACB7XtWYmwXdJG-sULhzwQow6qNH8meIks5rE_qkgaJpZM4LDk8Z> .

[JosefJezek]

Use Conditional templates instead of iron-pages

https://www.polymer-project.org/1.0/docs/devguide/templates#dom-if

<template id="faq" is="dom-if" restamp>
  <page-faq class="page" app="{{app}}"></page-faq>
</template>

<template id="feed" is="dom-if">
  <page-feed class="page" app="{{app}}" route="{{route}}"></page-feed>
</template>

_pageChanged: function(newPage, oldPage) {
    if (!this.$[newPage]) {
      newPage = '404';
    }

    if (oldPage) {
      if (!this.$[oldPage]) {
        oldPage = '404';
      }

      this.$$('page-' + oldPage).active = false;
      this.$[oldPage].if = false;
    }

    this.$[newPage].if = true;

    // Wait 1 tick for template to stamp.
    this.async(function() {
      this.$$('page-' + newPage).active = true;

      // Load page import on demand. Show 404 page if fails. Disable async for after lazy loaded elements.
      var resolvedPageUrl = this.resolveUrl('page-' + newPage + '.html');
      this.importHref(resolvedPageUrl, null, this._showPage404, false);
    });
  },

  _showPage404: function() {
    this.page = '404';
  }

[mercmobily]

@JosefJezek OK OK it works but still...

[mercmobily]

Actually, this was pretty much all user talk... what do Polymer developers think about this one? Will this be addressed in P2.0?

[mercmobily]

This problem was driving absolutely bonkers. Every page had to constantly check if the first URL parameter was there because another page was simply being displayed. JosefJezek only really worked if I restamped everything all the time -- which also wasn't acceptable.

The real problem is that passing subRoute itself is a mistake. subRoute is treated as a legit route by the "child" page, but the point is that it shouldn't be. Each page should have its own subRoute, which is changed when subRoute for that specific page changes.

So... I think I actually fixed. Elegantly.

So in my-app.html I still have:

  <app-location route="{{route}}"></app-location>
  <app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subRoute}}"></app-route>

One of the sub-pages my-gigs.html, which is loaded dynamically (ala PSK), is a little different:

    <iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404">
      ...
      <my-gig
        name="gig"
        route="{{subRoutePage.gig}}"
      </my-gig>

      <my-city
        name="city"
        route="{{subRoutePage.city}}"
      </my-city>

In my-gig.html, I still want to be able to view a specific gig depending on the URL. So, I still have:

<app-route route="{{route}}" pattern="/:gigId" data="{{routeDataGig}}" active="{{activeGig}}"></app-route>

Basically, each page has its own subRoutePage object. This is how it should be, and it makes 100% perfect sense to be this way, since changing URL shouldn't imply a change in routing information for all the pages in the app.

Managing this is ridiculously simple. It's a property:

    subRoutePage: {
      type: Object,
      value: function(){ return {} },
    }

And then when subRoute changes, the "right" object within subRoutePage is changed:

  _subRouteChanged: function(subRoute) {
    this.set( 'subRoutePage.' + this.page, this.subRoute );
  },

Note that only the page's own subRoutePage object is modified.
Basically, it's:

1. One extra property
2. Three lines of code
3. A very slight modification in the (route="{{subRoutePage.city}}" instead if `route="{{subRoute}}".

I honestly think this is the neatest possible solution, and (unless the Polymer 2 routing solves this problem elegantly in the first place in some other way) it should become an established pattern, explained in the basic documentation.

I hope this helps!

Merc.

[mercmobily]

@arthurevans Please check with the guys that what I wrote makes sense, and if it's worthwhile documenting this. I think it's a biggie, as I had endless problems with routing before this solution, and I think it's a very very common use case.

[mercmobily]

Cancel that.
I made a fundamental mistake:

this.set( 'subRoutePage.' + this.page, this.subRoute );

This is bad news, because it will mean that the sub-page's own route object will be the same (same reference) as the app's subRoute, which will lead (and led...) to many hours wasting debugging things. So much for "elegant".

What I ended up doing is a hack that cannot possibly be considered a pattern. This is the summary. The first half is identical to my previous comment, while the second half isn't.

---

So in my-app.html I still have:

  <app-location route="{{route}}"></app-location>
  <app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subRoute}}"></app-route>

One of the sub-pages my-gigs.html, which is loaded dynamically (ala PSK), is a little different:

    <iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404">
      ...
      <my-gig
        name="gig"
        route="{{subRoutePage.gig}}"
      </my-gig>

      <my-city
        name="city"
        route="{{subRoutePage.city}}"
      </my-city>

In my-gig.html, I still want to be able to view a specific gig depending on the URL. So, I still have:

<app-route route="{{route}}" pattern="/:gigId" data="{{routeDataGig}}" active="{{activeGig}}"></app-route>

Basically, each page has its own subRoutePage object. This is how it should be, and it makes 100% perfect sense to be this way, since changing URL shouldn't imply a change in routing information for all the pages in the app.

Managing this is ridiculously simple. It's a property:

    subRoutePage: {
      type: Object,
      value: function(){ return {} },
    }

And then when subRoute changes, the "right" object within subRoutePage is changed:

  _p: function( s ){
    return s.split('/')[1]
  },

  _subRouteChanged: function(subRoute) {
    if( this._protectSubRoutePage ) return;

    var page = this._p( subRoute.prefix );
    if( page ){
      this.set( 'subRoutePage.' + page, JSON.parse(JSON.stringify( this.subRoute ) ) );
    }
  },

NOTE that I have a sentinel variable, as well as a lame deep-copy of the array that uses JSON's parse/stringify.

There is also a need to reflect the changes back to the app's subRoute, but _only if it came from the "current" page where subRoute's path matches with the subpage's.

This code won't win any awards:

  // This is triggered when a sub-page changes its route. Then the question is:
  // do we propagate it up to the app's `subRoute` property, which is the tail?
  // The answer is MAYBE: that is, ONLY if `subRoute` is currently set to
  // a path that matches the changed subRoutePage.
  _subRoutePageChanged: function(subRoutePage) {

    if( ! this.subRoute ) return;
    var page = this._p( this.subRoute.prefix );
    if( subRoutePage.path.split('.')[1] == page ){
      this._protectSubRoutePage = true;
      this.set( 'route', JSON.parse(JSON.stringify( subRoutePage.base[ page ] ) ) );
      this._protectSubRoutePage = false;
    }
  },

(The path function is called _p because I spent 5 minutes thinking of a good name and eventually gave up)

I hope this example will show the length I went through to get this to work.
I still think that passing the same subRoute to each subpage is conceptually speaking a mistake: one page's subRoute should be one page's subRoute, whereas the current pattern seems to imply that they are all shared (with the horrible problems described in my original post). But at this point I find myself voting for @TimvdLippe 's solution (he always seems to be one step ahead...) with one reservation:

selected-attribute="route" selected-attribute-value="{{subRoute}}

This is limited to one parameter... and it really feels tailored for app-route. However, there is no easy way to make it "generic" and allow it to accept multiple parameters.

[TimvdLippe]

@mercmobily multiple parameters would just pass an object with a field subRoute containing the value of subRoute. E.g. something like selected-attribute-value="[[_createObject(subRoute, other)]]" and

_createObject: function(subRoute, other) {
  return {
    subRoute: subRoute,
    other: other
  };
}

[mercmobily]

Ah true. Always one step ahead @TimvdLippe :D I see your point. It's just one of those problems where you keep thinking "there's gonna HAVE TO be a better, easier solution"...
But I am starting to think that there isn't. The main problem is that subRoute (or tail) shouldn't be shared amongst page, and your solution is probably the neatest one available (and frankly it isn't that neat, but it's definitely better than anything I've come up with and anything I've seen)

[JosefJezek]

@mercmobily I am using Conditional templates for security reason and low memory using.

[jsilvermist]

@TimvdLippe Is your fix using selected-attribute-value on the roadmap for this issue, or do we need to find another solution?

[TimvdLippe]

@jsilvermist Good question. I have not been able to work on it (it is a very easy change though), primarily because not a lot of PRs on PolymerElements are merged right now. Probably because they are working hybrid mode instead. I added it to my TODO-list so hopefully I can work on it soonTM.

[TimvdLippe]

@jsilvermist @mercmobily Please see PolymerElements/iron-selector#148

[jsilvermist]

I'm confused, why did you implement this on iron-selector instead of iron-pages?

[mercmobily]

@TimvdLippe I would love to have a simpler solution. The way I am dealing wit hit is working well, but it's a ridiculous solution.
Can I suggest you update the pull request explaining how to use your feature? The use is buried somewhere here as a comment, but I made a lot of noise with my ugly solution and...

[TimvdLippe]

@jsilvermist Because iron-pages uses IronSelectableBehavior which is what the pull request updates.

@mercmobily This solution is I think the simplest you can come up with. It is DRY.

[jsilvermist]

@TimvdLippe Ah didn't realize the context, makes perfect sense now! Nice!

[mercmobily]

The more I look at it, the more I like it.
There is only one issue I have. I wrote a rather large application, with two URLs:

• /gigs/:gigId (e.g. `/gigs/1234)
• /users/:userId (e.g. `/users/5678)

They both work the same way: they have an observer on gigId or userId, and if they change they make an AJAX call to load the correct info.
With this PR applied, if I navigate from /gigs/1234 to /users/5678, will subRoute be reset to {}? You wrote that it would, but looking at your diff, I don't see why it would...

Other than that, PLEASE YES PLEASE apply this PR, it solves a huge problem with app-route right now...

[mercmobily]

@TimvdLippe I just spent 2 hours fixing a problem with my "solution". Please tell me that there is some good news (see: your solution was committed) for this terrible problem that will affect any decently-sized app...!

[TimvdLippe]

@mercmobily I suspect all PRs will be reconsidered once Polymer 2.0 has officially been released. So a little more patience right now I think?

[mercmobily]

Ahhhhh that's what I feared. Oh well, my "solution" works well enough. It's just that I am about to release to production, and... let's be honest, it's not really a "solution"... whereas yours is.

[shahraship]

I tried applying the pull request that @TimvdLippe had, but it didn't work as expected. Is it certain that the solution works for the given original reported issue?

[mercmobily]

@TimvdLippe Thank you so much for your work on this. I am in the process of migrating to P2, and refactoring. How far away do you think this PR is? PolymerElements/iron-selector#148 Please let us know!

[TimvdLippe]

Still waiting for review, so I am not sure. You can rely on my fork with the branch though, if you want to use it already. Op do 14 dec. 2017 om 09:08 schreef Tony Mobily <[email protected]>:

…

@TimvdLippe <https://github.com/timvdlippe> Thank you so much for your work on this. I am in the process of migrating to P2, and refactoring. How far away do you think this PR is? PolymerElements/iron-selector#148 <PolymerElements/iron-selector#148> Please let us know! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#164 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFrDb-eZoKiQRC01cNrGeashd9TDqM-Yks5tANd0gaJpZM4LDk8Z> .

[mercmobily]

A couple of questions:

• With your PR, if a sub-page changes subRoute, will the change propagate up?

Your example reads:

<iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404" selected-attribute="route" selected-attribute-value="{{subRoute}}">
  <my-gig name="gig"></my-gig>
  <my-city name="city"></my-city>
</iron-pages>

So... if my-gig changes its own route attribute, the change won't travel up, right?

• With your PR, a page will "lose" the route value when it's not set. Right? So, if my-gig loads a gig's data and displays it, the second you get out of my-gig the route variable is zapped, and -- presumably -- the gig's info is actually lost, right? (This means that returning to that page will need to trigger a data reload)

Am I missing something?

The more I look at it, the more I think that there is a bit of an architectural issue,

[iSuslov]

Ok, enough of this ridiculously long discussion. First of all @TimvdLippe solution won't work. You guys trying to patch component which has nothing to do with this issue. iron-selector works exactly like it suppose to work. End of story number one.

Story number two. Issue #148. This is where we (core devs actually, but ohh well my generosity..) completely forgot how and why app-route worked before. Look at the old version code

app-route/app-route.html

Line 395 in 6b28803

//notify in a specific order

Yes, updates should be ordered. Not just active and data which is solved (actually not) here #218, but also data and tail. In this pull request @cdata says 'LGTM', but it is not good at all. Core implementation of setProperties relies on "for .. in" which does not guarantee order of enumeration for object properties.

So, why @TimvdLippe solution won't work? Because what happens now is tail property updates before data property, so first you gonna get id change and then page. If update would be ordered like this:
0) active-false if data changed 1) data 2) tail 3) active-initialValue
then we will get page update first, while updating tail we will have inactive app-route and it will be enough to use with combination of iron-pages selected-attribute="visible" to get real state of a page.

So what would be the fastest solution for now?

PR? Not so fast usually. If you need it work right now @mercmobily, go create a patch component:

<dom-module id="app-route-patch">
  <template>
    <slot id="slot"></slot>
  </template>
  
  <script>
    class AppRoutePatch extends Polymer.Element {
      
      static get is() {
        return 'app-route-patch'
      }
      
      connectedCallback() {
        super.connectedCallback();
        
        //filter all app-routes, should be one really
        const appRoutes = this.$.slot.assignedNodes().filter(node => {
          return node.nodeType == Node.ELEMENT_NODE && node.nodeName.toUpperCase() == "APP-ROUTE";
        });
        
        //patch according to this https://github.com/PolymerElements/app-route/issues/164
        appRoutes.forEach(appRoute => {
          const superSetProperties = appRoute.setProperties;
          appRoute.setProperties = function (propertyUpdates, isReadOnly) {
            if (propertyUpdates.hasOwnProperty("data")) {
              appRoute._setActive(false);
              appRoute.set("data", propertyUpdates.data);
            }
            propertyUpdates.hasOwnProperty("tail") && appRoute.set("tail", propertyUpdates.tail);
            appRoute._setActive(true);
          }
        })
      }
    }
    window.customElements.define(AppRoutePatch.is, AppRoutePatch);
  </script>
</dom-module>

And use it like this:

<app-route-patch>
      <app-route
        route="{{route}}"
        pattern="[[rootPath]]:page"
        data="{{routeData}}"
        active="{{routeActive}}"
        tail="{{routeTail}}"></app-route>
    </app-route-patch>

Full example:

<app-route-patch>
      <app-route
        id="route"
        route="{{route}}"
        pattern="[[rootPath]]:page"
        data="{{routeData}}"
        active="{{routeActive}}"
        tail="{{routeTail}}"></app-route>
    </app-route-patch>

<iron-pages id="pageSelector" role="main" selected="[[page]]" attr-for-selected="name" fallback-selection="view404" selected-attribute="visible" >
  <my-gig name="gig" parent-route-active="{{routeActive}}"></my-gig>
  <my-city name="city" parent-route-active="{{routeActive}}"></my-city>
</iron-pages>

Every page inherits from BasePage wich looks like this

<dom-module id="base-page">
  <script>
    class BasePage extends Polymer.Element {
      
      static get is() {
        return 'base-page'
      }
      
      static get observers() {
        return [
          '_computeActive(parentRouteActive, visible)'
        ]
      }
      
      static get properties() {
        return {
          routeTail: {
            type: Object,
            notify: true
          },
          parentRouteActive: {
            type: Boolean
          },
          visible: {
            type: Boolean,
            observer: "onVisibleChange"
          },
          active: {
            type: Boolean
          }
        }
      }
      
      _computeActive(routeActive, visible) {
        this.active = routeActive && visible
      }

    }
    window.customElements.define(BasePage.is, BasePage);
  </script>
</dom-module>

[mercmobily]

I am a little lost... Isn't the new supa-dupa version of app-route supposed to make atomic changes to the data, so that there is only one notification for all changes? I really though that with P2 the whole "ordering nightmare" would be over?

Second... what does your patching component do...?

[iSuslov]

As I understand, technically it will be one update, but the way binding works is for every updated property property-changed event will be emitted. Since we are in single threaded environment, this events are ordered somehow. Right now according to implementation, the order is pretty random.
My patch overrides setProperties method of app-route specifically, assures that data is updated before tail and sets state to inactive during the update.

[mercmobily]

@iSuslov Ivan your PR was closed: #222 Did you give up on making one up?

[mercmobily]

My final workaround works like this:

  // ****************************************************************************
  // FINAL WORKAROUND FOR https://github.com/PolymerElements/app-route/issues/164
  // "borrowed" from Gigsnet's code
  // ****************************************************************************

  static get observers () {
    return [
      '_subRouteChanged(subRoute.*, subRoute)',
      '_pageSubRouteChanged(pageSubRoute.*,pageSubRoute)'
    ]
  }

  _afterFirstSlash (s) {
    if (!s) return ''
    return s.split('/')[1]
  }
  // subRoute has changed. Work out what page it changed for, and update that page's
  // own's subRoute
  _subRouteChanged (subRouteObj) {
    var subRoute = subRouteObj.base
    if (!subRoute) return
    if (this._protectPageSubRoute) return

    var page = this._afterFirstSlash(subRoute.prefix)
    if (page) {
      this.set('pageSubRoute.' + page, JSON.parse(JSON.stringify(this.subRoute)))
    }
  }
  // This is triggered when a sub-page changes its route. Then the question is:
  // do we propagate it up to the app's `subRoute` property, which is the tail?
  // The answer is YES
  _pageSubRouteChanged (pageSubRouteObj) {
    var pageSubRoute = pageSubRouteObj.base
    if (!this.subRoute) return
    var page = this._afterFirstSlash(this.subRoute.prefix)
    if (page) {
      this._protectPageSubRoute = true
      this.set('subRoute', JSON.parse(JSON.stringify(pageSubRoute[page])))
      this._protectPageSubRoute = false
    }
  }

Then in your program you'd go:

  <app-route
      route="{{route}}"
      pattern="[[rootPath]]:page"
      data="{{routeData}}"
      tail="{{subRoute}}"
  </app-route>
  ...
  <my-add-edit-jobs route="{{pageSubRoute.add-edit-jobs}}" name="add-edit-jobs"></my-add-edit-jobs>

This is basically to 1) Provide each page its own subroute object to avoid overlaps 2) Allow pages to actually change route and know that changes will be propagated upwards

SURELY this isn't a good way to go about it... but it's better than anything else I've tried.