googlearchive / more-routing

144 stars 30 forks source link

history.pushState is called without doing anything #49

Open tghosgor opened 9 years ago

tghosgor commented 9 years ago

There was a very weird behavior about HTML history after I have converted to Polymer 1.0. I have come a long way down to replace and intercept the history.pushState call and the cause seems that more-routing does a push.

After I do load a page, the library pushes a history which seems to be identical to the actual page. So after I load a page I am able to click the back button and apparently nothing changes. Why is more-routing doing this? In more complex situations it completely messes the history.

Here is the call path on a fresh page load:

Something pushed state. Event {arguments: Arguments[3]}
(anonymous function) @ page:53
(anonymous function) @ page:42
navigateToUrl @ path.html:27
Driver.navigateToParts @ driver.html:48
_navigateToParams @ route.html:207
(anonymous function) @ emitter.html:69
__notify @ emitter.html:68
params.forEach.Object.defineProperty.set @ params.html:50
processPathParts @ route.html:157
manageRoute @ driver.html:37
getRouteByPath @ routing.html:54
registerNamedRoute @ routing.html:83
Polymer._identityChanged @ more-route.html:123
Polymer.routingReady @ more-route.html:100
MoreRouting.ContextAware._makeRoutingReady @ more-route-context-aware.html:45
MoreRouting.ContextAware.ready @ more-route-context-aware.html:20
Polymer.Base._addFeature._invokeBehavior @ polymer-micro.html:275
(anonymous function) @ polymer-micro.html:268
Polymer.Base._addFeature._doBehavior @ polymer-micro.html:267
Polymer.Base._addFeature._readySelf @ polymer-mini.html:83
Polymer.Base._addFeature._ready @ polymer-mini.html:70
Polymer.Base._addFeature._readyClients @ polymer-mini.html:76
Polymer.Base._addFeature._ready @ polymer-mini.html:68
Polymer.Base._addFeature._tryReady @ polymer-mini.html:59
Polymer.Base._addFeature._initFeatures @ polymer.html:2782
Polymer.Base.createdCallback @ polymer-micro.html:106

If I use MoreRouting.navigateTo, this happens:

First:

Something pushed state. Event {arguments: Arguments[3]}
(anonymous function) @ (index):53
(anonymous function) @ (index):42
navigateToUrl @ path.html:27
Driver.navigateToParts @ driver.html:48
navigateTo @ route.html:96
navigateTo @ routing.html:94
Polymer.clickHandler @ custom-pushbutton.html:43
(anonymous function) @ polymer.html:348

Second:

Something pushed state. Event {arguments: Arguments[3]}
(anonymous function) @ (index):53
(anonymous function) @ (index):42
navigateToUrl @ path.html:27
Driver.navigateToParts @ driver.html:48
_navigateToParams @ route.html:207
(anonymous function) @ emitter.html:69
__notify @ emitter.html:68
Object.defineProperty.set @ params.html:50
processPathParts @ route.html:157
_matchingRoutes @ driver.html:103
setCurrentPath @ driver.html:62
_read @ path.html:36
navigateToUrl @ path.html:28
Driver.navigateToParts @ driver.html:48
navigateTo @ route.html:96
navigateTo @ routing.html:94
Polymer.clickHandler @ custom-pushbutton.html:43
(anonymous function) @ polymer.html:348

As you can see there is still a duplicate pushing.

P.S. Obviously, I am using the html5 history driver. I started using <more-route context ...> (emphasis on context) after the upgrade to Polymer 1.0.

tghosgor commented 9 years ago

I guess it all comes down to line 39 in route.html

var params = MoreRouting.Params(namedParams(this.compiled), this.parent && this.parent.params);
  params.__subscribe(this._navigateToParams.bind(this)); // <--- this line, commenting it seems to fix the problem
  Object.defineProperty(this, 'params', {
    get: function() { return params; },
    set: function() { throw new Error('Route#params cannot be overwritten'); },
  });

I don't know the internals enough to say what this line is needed for but apparently it doesn't do its job very well.

__subscribe says that:

  /**
   * Registers a callback that will be called each time any parameter managed by
   * this object (or its parents) have changed.
   *
   * @param {!Function} callback
   * @return {{close: function()}}
   */
  __subscribe: {

So why are we automatically calling navigate when parameters managed by a <more-route context ...> changes? Do I need to do a two-way binding and change the parameter inside the polymer component so that it will propagate the change to more-route's params attribute? And then more-route will do a history.pushState with the resolved URL? How will that work if the URL has some static parts that cannot be changed with a variable anyway?

Well it still doesn't explain the history.pushState call on a fresh page load. It has a one-way binding so more-route shouldn't even see a change on parameters:

<template>
  <more-route context name="test" params="{{params}}"></more-route>

  <x-test id$="[[params.id]]"></x-test>
</template>
<script>
  Polymer({
    is: 'x-test-page',
  });
</script>

Please note that I recently had started using more-route elements with context attribute after upgrading to Polymer 1.0 after which this problem started.

barnomics commented 9 years ago

:+1:

Having a very similar issue. Commenting out line:39 in route.html seems to do the trick.

For example,

  1. Be on /!#/main
  2. Follow a link to ... /#!/details/:param1/:param2
  3. Browser back button history will now look like this (going backwards)
    • /#!/details/:param1/:param2
    • /#!/details/ (this shouldn't be in history)
    • /#!/details/:param1/:param2 (this shouldn't be in history)
    • /#!/main