Skip to content

Commit

Permalink
Fix: redirects created invalid URLs
Browse files Browse the repository at this point in the history
…because they’re generated when the router does not yet know its `baseUrl`. We therefore need to infer it from the navigation instruction.
  • Loading branch information
rluba committed Jun 5, 2018
1 parent 78e5ba8 commit 0737128
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
8 changes: 6 additions & 2 deletions src/navigation-commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,12 @@ export class Redirect {
* @param appRouter The router to be redirected.
*/
navigate(appRouter: Router): void {
let navigatingRouter = this.options.useAppRouter ? appRouter : (this.router || appRouter);
navigatingRouter.navigate(this.url, this.options);
if (this.options.useHistory) {
appRouter.history.navigate(this.url, this.options);
} else {
let navigatingRouter = this.options.useAppRouter ? appRouter : (this.router || appRouter);
navigatingRouter.navigate(this.url, this.options);
}
}
}

Expand Down
19 changes: 16 additions & 3 deletions src/navigation-plan.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ export function _buildNavigationPlan(instruction: NavigationInstruction, forceLi

if ('redirect' in config) {
let router = instruction.router;
return router._createNavigationInstruction(config.redirect)
return router._createNavigationInstruction(config.redirect, instruction.parentInstruction)
.then(newInstruction => {
let params = Object.keys(newInstruction.params).length ? instruction.params : {};
let redirectLocation = router.generate(newInstruction.config.name, params, instruction.options);
const baseUrl = getInstructionBaseUrl(instruction);
let redirectLocation = router.generate(newInstruction.config.name, params, instruction.options, baseUrl);

if (instruction.queryString) {
redirectLocation += '?' + instruction.queryString;
}

return Promise.resolve(new Redirect(redirectLocation));
return Promise.resolve(new Redirect(redirectLocation, {useHistory: true}));
});
}

Expand Down Expand Up @@ -111,6 +112,18 @@ export function _buildNavigationPlan(instruction: NavigationInstruction, forceLi
return Promise.resolve(plan);
}

function getInstructionBaseUrl(instruction) {
const instructionBaseUrlParts = [];
instruction = instruction.parentInstruction;

while (instruction) {
instructionBaseUrlParts.unshift(instruction.getBaseUrl());
instruction = instruction.parentInstruction;
}

return instructionBaseUrlParts.join('');
}

function hasDifferentParameterValues(prev: NavigationInstruction, next: NavigationInstruction): boolean {
let prevParams = prev.params;
let nextParams = next.params;
Expand Down
4 changes: 2 additions & 2 deletions src/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ export class Router {
* @param options If options.absolute = true, then absolute url will be generated; otherwise, it will be relative url.
* @returns {string} A string containing the generated URL fragment.
*/
generate(name: string, params?: any, options?: any = {}): string {
generate(name: string, params?: any, options?: any = {}, baseUrlOverride: string = null): string {
let hasRoute = this._recognizer.hasRoute(name);
if ((!this.isConfigured || !hasRoute) && this.parent) {
return this.parent.generate(name, params);
Expand All @@ -279,7 +279,7 @@ export class Router {
}

let path = this._recognizer.generate(name, params);
let rootedPath = _createRootedPath(path, this.baseUrl, this.history._hasPushState, this.history.root);
let rootedPath = _createRootedPath(path, baseUrlOverride ? baseUrlOverride : this.baseUrl, this.history._hasPushState, this.history.root);
if (options.absolute) {
// Hamfisted workaround because history does not expose the origin without the root
let origin = this.history.getAbsoluteRoot();
Expand Down

0 comments on commit 0737128

Please sign in to comment.