erikringsmuth / app-router

Router for Web Components
https://erikringsmuth.github.io/app-router/
MIT License
610 stars 83 forks source link

Issue when importing route content if network connection is slow #79

Closed plequang closed 9 years ago

plequang commented 9 years ago

Hi,

After submitting issue #77, I've found other problems related to removal of previousRoute content, but in a particular context, i.e. when network connection is slow (3G on a phone for example) and I try to import a large html file.

First issue is when you do not use core-animated-pages. The app-router configuration I'm using is the following :

    <app-router>
      <app-route path="/">
        <template>
          <div>
            <h2>Start page</h2>
          </div>
        </template>
      </app-route>
      <app-route path="/big-element1" import="pages/big-element-1.html" element="big-element-1"></app-route>
      <app-route path="/big-element2" import="pages/big-element-2.html" element="big-element-2"></app-route>
    </app-router>

big-element-1.html and big-element-2.html are 300K custom elements. Using Chrome network throttling set to 3G, navigate to /big-element1, then to /big-element2 before big-element-1.html has been loaded in the browser.

When big-element1 finishes loading, /big-element1 is not the active route anymore and nothing is done. When big-element2 finishes loading, it displays /big-element2 route content, and removes previous route content which is /big-element1. So it ends up by displaying the content of route '/' and '/big-element2'.

With the same scenario but using core-animated-pages, behavior is almost the same, except the content of route '/' is not displayed at the end, because core-animated-pages displays only the selected route. But if you navigate to route '/' again, you will get the route content twice.

Sorry if it is a bit confused. I've test files in my app-router repository fork, maybe it's easier if I send these files in a pull request ?

erikringsmuth commented 9 years ago

Yeah, it'd be helpful to send those in a PR.

I get what you're saying. The code get's an incorrect previousRoute and fails to remove the other content.

plequang commented 9 years ago

OK. I've tried to solve the issue using a boolean value on route object, like this is done for transitions with the boolean transitionAnimationInProgress.

Then I've noted another problem : it seems the import property of the import link can be non null before the load event is fired on the import link. This has lead to situation where app-router tries to activate an element, but an exception is raised in Polymer.

So I've added an additional test in importAndActivate, to be sure app-router activate elements only after the load event has been fired.

I send a pull request with my test files and my attempt to solve the issue. Let me know if you think this looks good.

erikringsmuth commented 9 years ago

All merged!