erikringsmuth / app-router

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

#79 solving issue when network connection is slow #80

Closed plequang closed 9 years ago

plequang commented 9 years ago

Sure, I can make suggested changes.

Regarding the scenario that leads to this check (I mean importLink.import is not null, even though import has not completed) , you can try the following :

  1. Go to http://localhost:8080/tests/functional-tests/test-big-import.html#/
  2. Set network throttling to 3G
  3. Click on /big-element1 twice The second time you click, importLink.import is not null, but download should not have completed. So instead of relying on importLink.import, it's probably safer to wait for the 'load' event.
moderndeveloperllc commented 9 years ago

@plequang Just curious, is this an issue if the elements have been vulcanized? I gather this is only an issue when loading large scripts sequentially.

plequang commented 9 years ago

@moderndeveloperllc This is an issue as long as you let app-router initiate the import of your elements.

I've tried my scenario with following routes definitions :

<app-route path="/big-element1" import="pages/big-elements-vulcanized.html" element="big-element-1"></app-route>
<app-route path="/big-element2" import="pages/big-elements-vulcanized.html" element="big-element-2"></app-route>

Where big-elements-vulcanized.html is a vulcanized version of my 2 big elements. Under slow network connections, a user might click on different links until the big-elements-vulcanized.html file has been loaded. After 2 clicks, app-router lose the initial previousRoute reference that it was supposed to remove. And importLink.import still gets a non-null value even though the file has not been completely loaded.

This can be solved by importing the html file in the head of the document, but what I'd like to do is to load "on demand" my views. Moreover, the app will be "unresponsive" until all head's imports have been completed.

erikringsmuth commented 9 years ago

@plequang The importLink.loaded check makes sense, but now I'm trying to debug route.importInProgress and I'm not seeing what it does.

plequang commented 9 years ago

@erikringsmuth actually it's my mistake, I should have open another issue for the importLink issue. Initially, issue #79 was about previousRoute not being correctly removed in case of slow import.

The test case is (you will need to use some network throttling):

  1. Go to http://localhost:8080/tests/functional-tests/test-big-import.html#/
  2. Click on /big-element1
  3. Click on /big-element2 before /big-element1 finishes loading.

Normally, previous route content removal is done in activateElement which is run when import is finished. But in this situation, activateElement won't run for '/big-element1', because you switch view before it finishes loading ==> content of route '/ ' won't be removed

If not using core-animated-pages, you will see the content of route '/' and '/big-element2'. If using core-animated-page, the content of '/' will be hidden, but still present in the DOM.

In activateRoute, similar situation was already anticipated and described in the comments :

// update the references to the activeRoute and previousRoute. if you switch between routes quickly you may go to a
// new route before the previous route's transition animation has completed. if that's the case we need to remove
// the previous route's content before we replace the reference to the previous route.

That's why I added the test for route.importInProgress there, if you switch between routes quickly before new route finishes loading.

erikringsmuth commented 9 years ago

Ah yes, got it! I merged in the changes.