[BUG] v1.6.x broke redirect in child router config

[3cp]

I'm submitting a bug report

• Library Version:
  1.6.1

• Please tell us about your environment:
  OSX 10.13

• Node Version:
  8.11.2

• NPM Version:
  6.1.0

• JSPM OR Webpack AND Version
  NA

• Browser:
  all

• Language:
  ESNext

Current behavior:
With latest v1.6.1, when a route page /child has additional child router config, the redirect in config doesn't work properly.

export class Child {
  configureRouter(config, router) {
    this.router = router;
    config.mapUnknownRoutes('./not-found.html');

    config.map([{
      route: 'test',
      title: 'test',
      name: 'test',
      nav: true,
      moduleId: './test.html'
    }, {
      route: '',
      // this redirects "/child" to "/child/test"
      // but in router 1.6.1, it instead redirects to "/test" (not-found)
      redirect: 'test'
    }]);
  }
}

Expected/desired behavior:
rjs-bundle is not up to date to router 1.6.x, so I built a demo app for the bug https://github.com/huochunpeng/router-bug

• What is the expected behavior?
  Keep the old behaviour, redirect to the same level of route.

• What is the motivation / use case for changing the behavior?
  :-( It broke my production app, I need to pin aurelia-router to 1.5.0 to bypass this regression.

[bobum]

I can confirm this behavior in our app as well

[3cp]

The new implementation of handling child route config "redirect" triggered a timing bug on router baseUrl.

router/src/navigation-plan.js

Lines 28 to 41 in 0a66d74

	if ('redirect' in config) {
	let router = instruction.router;
	return router._createNavigationInstruction(config.redirect)
	.then(newInstruction => {
	let params = Object.keys(newInstruction.params).length ? instruction.params : {};
	let redirectLocation = router.generate(newInstruction.config.name, params, instruction.options);
	if (instruction.queryString) {
	redirectLocation += '?' + instruction.queryString;
	}
	return Promise.resolve(new Redirect(redirectLocation));
	});
	}

The new implementation use router.generate(...) to generate url, replaced the old manual resolve. Which is good thing.

The issue is when router.generate(...) got called here, the router._refreshBaseUrl() was NOT called yet. The result is the child router doesn't have a proper baseUrl at this point of time, manifested as missing baseUrl part in redirectLocation.

router/src/navigation-instruction.js

Line 201 in 0a66d74

router._refreshBaseUrl();

I am not clear about the whole picture of how router works, not sure how to make a fix.
cc @EisenbergEffect @davismj

[EisenbergEffect]

@davismj Can you look into this. Appears to be a rather serious couple of issues in the latest router release.

[fragsalat]

👍 Got this problem as well on main router after updating from 1.5.0 to 1.6.x

[davismj]

On it.

[balazsmeszegeto]

I was about to submit the very same issue

[josundt]

Any progress on this?
Upgrading to 1.6.x is no option in my current project till this is OK

[stefan505]

Guys, this issue is very serious. Its completely broken my sub-routers + defaults strategy.

As a long-time Aurelia user / supporter I am very concerned about the recent router problems.

[3cp]

@stefan505 for temporary fix, I use this in my package.json

"aurelia-router": "1.5.0",
"aurelia-templating-router": "1.3.2", // I need to do this for yarn. Didn't test on npm.

The regression is from #515, which can be reverted.

[3cp]

@EisenbergEffect @davismj can we have a hot-fix reverting #515 before further investigation?

It seems many users are waiting for the fix urgently for production apps.

[EisenbergEffect]

Both @davismj and I are on vacation right now. When Matt gets back, this is on the top of his list to address. In the mean time, I recommend rolling back via package.json as described above. We apologize for this. We will get it fixed soon.

[davismj]

Closed via #610