fix: update location when reusing parents

Fixes #428
vaadin · Feb 24, 2020 · 74b2155 · 74b2155
1 parent bfa377c
commit 74b2155
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 11 deletions.
diff --git a/src/router.js b/src/router.js
@@ -556,22 +556,18 @@ export class Router extends Resolver {
         const isFound = (matchedPath === contextAfterRedirects.pathname);
 
         // Recursive method to try matching more child and sibling routes
-        const findNextContextIfAny = (context, parent) => {
-          let prevResult = undefined;
-          if (parent === undefined) {
-            parent = context.route;
-          } else {
-            prevResult = null;
-          }
+        const findNextContextIfAny = (context, parent = context.route, prevResult) => {
           return context.next(undefined, parent, prevResult).then(nextContext => {
             if (nextContext === null || nextContext === notFoundResult) {
               // Next context is not found in children, ...
               if (isFound) {
                 // ...but original context is already fully matching - use it
                 return context;
-              } else {
+              } else if (parent.parent !== null) {
                 // ...and there is no full match yet - step up to check siblings
-                return findNextContextIfAny(context, context.route.parent);
+                return findNextContextIfAny(context, parent.parent, nextContext);
+              } else {
+                return nextContext;
               }
             }
 
@@ -674,8 +670,19 @@ export class Router extends Resolver {
       }
     }
     // execute onBeforeEnter when NOT skipping attach
-    for (let i = newContext.__divergedChainIndex; !newContext.__skipAttach && i < newChain.length; i++) {
-      callbacks = this.__runOnBeforeEnterCallbacks(callbacks, newContext, {prevent, redirect}, newChain[i]);
+    if (!newContext.__skipAttach) {
+      for (let i = 0; i < newChain.length; i++) {
+        if (i < newContext.__divergedChainIndex) {
+          if (i < previousChain.length && previousChain[i].element) {
+            previousChain[i].element.location = createLocation(newContext, previousChain[i].route);
+          }
+        } else {
+          callbacks = this.__runOnBeforeEnterCallbacks(callbacks, newContext, {prevent, redirect}, newChain[i]);
+          if (newChain[i].element) {
+            newChain[i].element.location = createLocation(newContext, newChain[i].route);
+          }
+        }
+      }
     }
     return callbacks.then(amendmentResult => {
       if (amendmentResult) {

diff --git a/test/router/parent-layouts.spec.html b/test/router/parent-layouts.spec.html
@@ -96,6 +96,28 @@
         expect(third.firstElementChild.tagName).to.match(/x-d/i);
       });
 
+      it('should update parent location when reusing layout', async() => {
+        router.setRoutes([
+          {path: '/a', component: 'x-a', children: [
+            {path: '/b', component: 'x-b'},
+            {path: '/c', component: 'x-c'},
+            {path: '/([de])', component: 'x-d'}
+          ]}
+        ]);
+
+        await router.render('/a/b');
+        expect(outlet.lastElementChild.location.pathname).to.equal('/a/b');
+
+        await router.render('/a/c');
+        expect(outlet.lastElementChild.location.pathname).to.equal('/a/c');
+
+        await router.render('/a/d');
+        expect(outlet.lastElementChild.location.pathname).to.equal('/a/d');
+
+        await router.render('/a/e');
+        expect(outlet.lastElementChild.location.pathname).to.equal('/a/e');
+      });
+
       it('should remove nested route components when the parent route is navigated to', async() => {
         router.setRoutes([
           {path: '/a', component: 'x-a', children: [
@@ -208,6 +230,49 @@
         checkOutlet(['x-fallback']);
       });
 
+      it('should take next routes as fallback when grandchildren do not match', async() => {
+        router.setRoutes([
+          {path: '/a', component: 'x-a', children: [
+            {path: '/b', component: 'x-b', children: [
+              {path: '/b', component: 'x-c'}
+            ]},
+          ]},
+          {path: '/a/b/d', component: 'x-fallback'}
+        ], true);
+
+        await router.render('/a/b/d');
+
+        verifyActiveRoutes(router, ['/a/b/d']);
+        checkOutlet(['x-fallback']);
+      });
+
+      it('should throw not found when neither children nor siblings match', async() => {
+        // Ensure outlet is clean
+        const childNodes = Array.from(outlet.childNodes);
+        outlet.textContent = '';
+
+        router.setRoutes([
+          {path: '/a', component: 'x-a', children: [
+            {path: '/b', component: 'x-b', children: [
+              {path: '/b', component: 'x-c'}
+            ]},
+          ]},
+          {path: '/a/b/d', component: 'x-fallback'}
+        ], true);
+
+        const onError = sinon.spy();
+        await router.render('/a/b/e').catch(onError);
+
+        expect(outlet.childNodes.length).to.equal(0);
+        const error = onError.args[0][0];
+        expect(error).to.be.an('error');
+        expect(error.message).to.match(/page not found/i);
+
+        // Restore previous outlet content
+        outlet.textContent = '';
+        childNodes.forEach(childNode => outlet.appendChild(childNode));
+      });
+
       it('redirect property amends previous path', async() => {
         router.setRoutes([
           {path: '/a', component: 'x-a', children: [