PolymerElements / app-route

A modular client-side router
https://www.polymer-project.org/1.0/articles/routing.html
146 stars 66 forks source link

_routePageChanged running twice #129

Open ansarizafar opened 7 years ago

ansarizafar commented 7 years ago

Browsers Affected

Here is my code

<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../bower_components/app-route/app-location.html">
<link rel="import" href="../bower_components/app-route/app-route.html">
<link rel="import" href="../bower_components/app-layout/app-drawer-layout/app-drawer-layout.html">
<link rel="import" href="../bower_components/app-layout/app-drawer/app-drawer.html">
<link rel="import" href="../bower_components/app-layout/app-scroll-effects/app-scroll-effects.html">
<link rel="import" href="../bower_components/app-layout/app-header/app-header.html">
<link rel="import" href="../bower_components/app-layout/app-header-layout/app-header-layout.html">
<link rel="import" href="../bower_components/app-layout/app-toolbar/app-toolbar.html">
<link rel="import" href="../bower_components/paper-icon-button/paper-icon-button.html">
<link rel="import" href="../bower_components/paper-menu/paper-menu.html">
<link rel="import" href="../bower_components/paper-item/paper-item.html">
<link rel="import" href="../bower_components/paper-menu-button/paper-menu-button.html">
<link rel="import" href="../bower_components/neon-animation/neon-animated-pages.html">
<link rel="import" href="../bower_components/neon-animation/neon-animatable.html">
<link rel="import" href="../bower_components/neon-animation/animations/slide-from-left-animation.html">
<link rel="import" href="../bower_components/neon-animation/animations/slide-right-animation.html">

<link rel="import" href="../bower_components/iron-selector/iron-selector.html">
<link rel="import" href="my-icons.html">

<dom-module id="my-app">

  <template>

    <style>
      :host {
        display: block;
        --app-primary-color: #4CAF50;
        --app-secondary-color: black;
      }

      app-header {
        background-color: var(--app-primary-color);
        color: #fff;
      }

      app-header paper-icon-button {
        --paper-icon-button-ink-color: white;
      }

      .drawer-list {
        margin: 0 20px;
      }

      .drawer-list a {
        display: block;
        padding: 0 16px;
        line-height: 40px;
        text-decoration: none;
        color: var(--app-secondary-color);
      }

      .drawer-list a.iron-selected {
        color: black;
        font-weight: bold;
      }

      .drawer-list a.subroute {
        padding-left: 32px;
      }
    </style>

    <app-location route="{{route}}"></app-location>
    <app-route route="{{route}}" pattern="/:page" data="{{routeData}}" tail="{{subroute}}"></app-route>
    <app-drawer-layout fullbleed force-narrow="true">
      Drawer content
      <app-drawer id="menu" swipe-open>
        <app-header>
          <app-toolbar>
            <iron-icon icon="move-to-inbox"></iron-icon> Restock
          </app-toolbar>
        </app-header>
        <iron-selector selected="{{routeData.page}}" attr-for-selected="name" class="drawer-list"
          role="navigation">
          <a name="" drawer-toggle href="/">Home</a>
          <a name="signup" drawer-toggle href="/signup">Create a free account</a>
          <a name="login" drawer-toggle href="/login">Login</a>
          <a name="about" drawer-toggle href="/about">About</a>
        </iron-selector>
      </app-drawer>

      <!-- Main content -->
      <app-header-layout has-scrolling-region>

        <app-header condenses reveals effects="waterfall">
          <app-toolbar>
            <paper-icon-button icon="menu" drawer-toggle></paper-icon-button>
            <div title>Restock</div>
            <paper-menu-button horizontal-offset=- 18>
              <paper-icon-button icon="more-vert" class="dropdown-trigger"></paper-icon-button>
              <paper-menu class="dropdown-content">
                <paper-item>Signup</paper-item>
                <paper-item>Login</paper-item>
                <paper-item>Help</paper-item>
              </paper-menu>
            </paper-menu-button>
          </app-toolbar>
        </app-header>

        <neon-animated-pages selected="[[routeData.page]]" fallback-Selection="notfound" attr-for-selected="name" entry-animation='slide-from-left-animation'
          exit-animation='slide-right-animation'>
          <neon-animatable name=''>
            <home-page name=""></home-page>
          </neon-animatable>
          <neon-animatable name='signup'>
            <signup-page name="signup"></signup-page>
          </neon-animatable>
          <neon-animatable name='login'>
            <login-page name="login"></login-page>
          </neon-animatable>
          <neon-animatable name='notfound'>
            <notfound-page name="notfound"></notfound-page>
          </neon-animatable>
          </neon-animated-page>

      </app-header-layout>

    </app-drawer-layout>
  </template>

  <script>

    Polymer({

      is: 'my-app',
      user: 'Zafar',
      properties: {

      },

     observers: [
        '_routePageChanged(routeData.page)'
      ],

      _routePageChanged: function(page) {
  console.log(page);

switch (page) {
case '':
case 'home':
this.importHref(this.resolveUrl('home.html'), null, null, true);
break;
case 'signup':
this.importHref(this.resolveUrl('signup.html'), null, null, true);
break;
case 'login':
this.importHref(this.resolveUrl('login.html'), null, null, true);
break;
default:
this.importHref(this.resolveUrl('notfound.html'), null, null, true);

}
      },
    });

  </script>

</dom-module>
rictic commented 7 years ago

Can you give more info here? What values of page are you seeing, and in what order? How do these values correspond to the URL?

ansarizafar commented 7 years ago

Whenever I click a link, _routePageChanged function is running twice. For example If I click signup link, The page value signup appears twice in console.

JanGunzenhauser commented 7 years ago

It is running twice because it changes twice, because a click on e.g. /signup link triggers

  1. a select of name="signup" in selected="{{routeData.page}}", because of two-way-binding
  2. a path change because of its href

Try using a one-way-binding like you do on the animated pages.

kshep92 commented 7 years ago

Running a similar example and with the only two-way binding being the {{route}} property the observer fires twice when using hash URLs.

<app-location route="{{route}}" use-hash-as-path></app-location>
<app-route route="[[route]]" pattern="/:page" data="{{data}}" tail="{{tail}}"></app-route>
.
.
.
observers: ['_routePageChange(data.page)'],
_routePageChange: function(e) {   
    console.log('_routePageChange'); //Prints twice for #/home and #/about
}
kshep92 commented 7 years ago

Here's a more complete example. Switched to traditional no-hash routing as well and still getting the observer running twice. My workaround was to listen for the window.location-changed event fired by app-location.

<link rel="import" href="bower_components/app-route/app-location.html" >
<link rel="import" href="bower_components/app-route/app-route.html" >
<dom-module id="blog-app">
    <template>
        <app-location route="{{route}}"></app-location>
        <app-route
            route="{{route}}"
            pattern="/:page"
            data="{{pageData}}"
            tail="{{pageTail}}">
        </app-route>
        <ul>
            <li>
                <a href="/home">Home</a>
            </li>            
            <li>
                <a href="/about">About</a>
            </li>
        </ul>

    </template>
    <script>
        Polymer({
            is: 'example-app',
            properties: {
                pageData: Object
            },
            observers: ['_pageDataChange(pageData.page)'],
            attached: function() {
                window.addEventListener('location-changed', function() {
                    console.log('new location:' + this.pageData.page); //Fires once
                }.bind(this));
            },
            _pageDataChange: function(e) {
                console.log('_pageDataChange'); //Fires twice on route change
            }
        });
    </script>
</dom-module>
virusakos commented 7 years ago

This is happening in Safari as well (and I suppose in all browsers).

Steps to replicate: 1) polymer init with starter-kit 2) Add console.log('_routePageChanged | page = ', page); in _routePageChanged: function(page) in my-app.html

Open Safari, show web inspector and you will immediately see:

_routePageChanged | page = "view1"
_routePageChanged | page = "view1"

Navigate to view2:

_routePageChanged | page = "view2"
_routePageChanged | page = "view2"

etc

What I ended up doing to avoid this issue is this: I defined a new property

pageLoaded: {
  type: String,
  value: ''
}

and changed _routePageChanged: function(page) to

_routePageChanged: function (page) {
  if (this.pageLoaded !== page) {
    this.page = page || 'view1';
    this.pageLoaded = this.page;
  }
}

But it feels 'hacky' and have no idea yet what kind of problems I will be having with this. The event is still firing twice.

abridger commented 7 years ago

Not sure if this is the right strategy, but we had this problem and it was hugely affecting our page tracking. Our _viewChanged observer now wraps a debounce method to collapse multiple requests, which seems to have resolved the problem.

_viewChanged (view) {
        this.debounce('viewChanged', () => {
            // Tracking events
        });
}
jfrazzano commented 7 years ago

This might be incredibly naive, but our response when having similar issues since August was to just take control of the navigator

Store data pieces or at least enough to represent last set of changes and let the data from us, user state, level etc sort of merge into an address

I don't think that what was intended here but there is a lot of memory available In the mac and rewrite back history

Sent from my iPhone

On Dec 15, 2016, at 6:29 AM, Alan Bridger notifications@github.com wrote:

Not sure if this is the right strategy, but we had this problem and it was hugely affecting our page tracking. Our _viewChanged observer now wraps a debounce method to collapse multiple requests, which seems to have resolved the problem.

_viewChanged (view) { this.debounce('viewChanged', () => { // Tracking events }); } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

daniele-orlando commented 7 years ago

What's the status of the fix for this bug? I can still reproduce it on Google Chrome.

Thkasis commented 7 years ago

Any update on this? I seem to have the same issue

jurerick commented 7 years ago

How it is going?

jukbot commented 7 years ago

yep how it's update ?

ghost commented 7 years ago

I have the same buf. I'm fixed this bug by just calling routeChange and format url as i need, it is running only once on change url

<app-location route="{{route}}"></app-location>

 observers: [
         '_routeChanged(route)',
 ],

 _routeChanged: function (r) {
         console.log(r.path);
         var id = r.path.split("/")[2];
         var tab = r.path.split("/")[3];
         if(id  != null)
               this._routeIdChanged(id);
        this._routeTabChanged(tab);
}, 
johnlim commented 6 years ago

Hi, just realised I'm also encountering this issue. Is there any update on this?

kshep92 commented 6 years ago

@johnlim guess not. Maybe in Polymer 3.0? I've always said that Polymer needed a fully featured routing system complete with life cycle hooks (before callback, after callback) and maybe we'll get that in addition to a resolution to this issue.

tekkamanendless commented 6 years ago

I've been using @abridger's debounce workaround for the past year, and that's been good enough for me. If my workflow becomes more complicated that this gets in the way again, maybe I'll poke around and see if I can submit a patch. That's unlikely at this stage, however.

markbrocato commented 6 years ago

+1 We're running into this as well.

HJK181 commented 6 years ago

Still nothing new after more than one and a half year?

sundar-rajavelu commented 6 years ago

Even after all these months, for reasons unknown, the 'routeData.page' is still firing twice. So, I did this: `

<app-location
  route="{{route}}">
</app-location>    

<app-route
  route="{{route}}"
  pattern="/:page"
  data="{{routeData}}" 
  tail="{{subroute}}"
  query-params="{{queryParams}}">
</app-route>

 ....

 static get observers() {
    return [
      '_routePathChanged(route.path)',
    ];
  }

  _routePathChanged(path) {

    console.log('routeData.page: ', this.routeData.page);
    console.log('queryParams: ', this.queryParams);

  }

`

sebs commented 6 years ago
  1. Juli 2016

no time for that

jsilvermist commented 6 years ago

With Polymer 3 I'm finding the pwa-helpers router a sufficient replacement in most cases.

App shell


// https://github.com/Polymer/pwa-helpers/blob/master/router.js
import { installRouter } from 'pwa-helpers/router.js';

constructor() { super();

// Register location changed listeners installRouter((location) => this._locationChanged(location)); }

_locationChanged(location) { // Remove root-path from route const prefixRegex = new RegExp('^' + this.rootPath);

// Get path from router const path = window.decodeURIComponent(location.pathname).replace(prefixRegex, '');

// Parse path to create route this.route = path.split('/').filter((item) => item !== ''); }

> Mixin shared between all routing views
```javascript
_routeChanged(route) {
  // Default to '_default' when `route` array is empty.
  this.page = route.length > 0 ? route[0] : '_default';
}

// Compute subroute for sub-views
// <my-view route="[[_bindSubroute(route, 'foobar')]]"></my-view>
_bindSubroute(route, uri) {
  return route.length > 1 && route[0] === uri ? route.slice(1) : [];
}

// Compute active for sub-views
// <my-view active="[[_bindActive(route, 'foobar')]]"></my-view>
_bindActive(route, uri) {
  return route.length > 0 && route[0] === uri;
}

For query parameters, there's no end of solutions available for parsing location.search so I'll just leave that for a Google search.

sebs commented 6 years ago

This is a close ?