Incorrect routing when using navigationStrategy callback

[WouterOuwens]

Currently, I'm looking into dynamic routing in aurelia. To achieve this, I mapped all routes using a wildcard and I'm using the navigationStrategy property to determine the correct module id (as suggested in https://github.com/aurelia/router/issues/129).

configureRouter(config: RouterConfiguration, router: Router): void {
    config.map([
        { route: "**", nav: false, navigationStrategy: this.navigateToModule, settings: this }
    ]);

    router.handleUnknownRoutes(this.unknownRoutes);
}

private navigateToModule(instruction: NavigationInstruction): void {

// some logic to determine a moduleId
instruction.config.moduleId = moduleId; // A valid view-model
}

The first navigation is correct and loads the correct viewmodel, as expected. However, when I now navigate to another route in the same Router (with another moduleId), it always navigates to the current active viewmodel (instead of the new one). The navigationStrategy callback is called witch each navigation, as expected.

The activate() function of the current (incorrect) viewmodel is called, while the navigationInstruction.config.moduleId in the activate() function contains the new moduleId (as set in the navigateToModule function). When refreshing the site, the correct viewmodel is loaded.

The code is running on the current Aurelia release (beta.1.1.0), using Typescript. I'm encountering this in all browsers. Is this expected behavior?

I also tried to map the routes using the handleUnknownRoutes (as sugested in https://github.com/aurelia/router/issues/15). This is working as expected. However, I prefer to use the described way, since it is possible to use to settings property in the router better (there is no reference to the original class in the handleUnknownRoutes callback) and it is possible to use multiple wildcards and redirect them to other functions.

[bryanrsmith]

@WouterOuwens Are you able to provide a plunker or link to a repo that demonstrates the problem?

[EisenbergEffect]

Closing this issue since it was not reproducible and the OP didn't respond after several months.

[Roustalski]

@WouterOuwens How did you get around this issue? I am running up against the same problem. I'll see if I can create a reproducible scenario, forked from the TS skeleton, for the team.

[Roustalski]

OK, I have a reproducible scenario: https://github.com/Roustalski/skeleton-navigation

cd path/to/skeleton-navigation-typescript npm install && jspm install gulp watch

Click on child router, then modify the route URL: http://localhost:9001/#/child-router/1 http://localhost:9001/#/child-router/2 http://localhost:9001/#/child-router/3 etc...

Breakpoint on child-router.ts::12 shows the module ID being swapped per logic, but the view/vm pair remains on the first module loaded for that instruction.

[WouterOuwens]

I was unable to reproduce it in the skeleton. In the end, I used another approach.

For my application, it was easier to route it with a simple custom router. The main navigation is still aurelia, but some "inner navigation" is a custom router. This solution gave me better control over the whole (custom) navigation pipeline.

[StrahilKazlachev]

@EisenbergEffect Can this be reopened? Here is a simple repro - skeleton-esnext. All route configuration related code is in app.js.

[StrahilKazlachev]

Ok, so I managed to make it work, in a way. The first time navigationStrategy is called navigationInstruction.config does not have viewPorts set. On subsequent calls viewPorts is set and its moduleId is set to the moduleId from the first call. So viewPorts.default.moduleId also has to be set to the moduleId we want. If this is how it should work, it has to be added to the docs about Dynamically Specify Route Components.

[doronguttman]

I had the same issue. Based on @StrahilKazlachev's comment, I have added the following line in my navigationStrategy code: delete instruction.config.viewPorts;

here's my complete code:

private dynamicRouter(instruction: NavigationInstruction) {
  let route = /\/(.*)(\/)?/.exec(instruction.fragment);
  if (!route || !route[1] || route[1].length === 0) return;

  instruction.config.moduleId = `views/${route[1]}/view-${route[1]}`;
  instruction.config.href = instruction.fragment;
  delete instruction.config.viewPorts;
}

I must say, though, this looks like a hack and I would not expect to have to do that.

[thomas-darling]

@EisenbergEffect @davismj We're seeing this too, and it's definitely a bug.

Basically, we have a route, /flights/:destination, with a navigation strategy, which sets navigationInstruction.config.moduleId dynamically based on the type of place referenced by the destination parameter in the URL.

This works correctly on the first navigation to that route. However, if you then, while on that route, try to navigate to the same route, but with a place of a different type, things break. The navigation strategy is correctly executed, and a new moduleId is assigned to the navigation instruction config, but as illustrated in the screenshot below, it still ends up reusing the previous viewport instruction, and thus breaks things.

For example, on the line with the breakpoint, it fails to realize that the moduleId was changed, which means it doesn't set the activation strategy to replace, and therefore never actually loads the new module - as you can see in the watches at the top-right, the new moduleId is in fact assigned to config, but nextViewPortConfig still has the old value.

[jonathaneckman]

I have an issue open in aurelia-open-id-connect that may be this same problem. Check out the issue there for a more detailed explanation.

The gist is that a navigation instruction is added like so:

routerConfiguration.mapRoute({
      name: 'logInRedirectCallback',
      navigationStrategy: (instruction: NavigationInstruction) => {
        // do stuff
      },
      route: 'configured-route',
});

While routing, I get the following error. Stepping through, I can see it is because moduleId not undefined.

ERROR [app-router] TypeError: Cannot read property 'trim' of undefined
ERROR [app-router] Router navigation failed, and no previous location or fallbackRoute could be restored.

[davismj]

Let me know if you can set up a repro @jonathaneckman.

[jonathaneckman]

@davismj here is a repro. If a navigationStrategy must also include a moduleId, then this is a bug in the plugin I'm using. If not, its a bug in the router. You can see this in my configureRouter.

In the example, I am statically assigning the route for logInRedirectCallback. In the plugin, this is dynamically provided by a config.

[SlowFourierTransform]

Our team hit this issue as well. The suggestion delete instruction.config.viewPorts; suggested https://github.com/aurelia/router/issues/289#issuecomment-297816986 does solve the problem for us.

[bigopon]

@jonathaneckman Thanks for your repo. Here https://github.com/aurelia/router/blob/9ba6ef338e4e4f5e21758b245d7ebf82fb364982/src/router.ts#L726-L730 is where the issue seems to be.

After evaluating navigationStrategy, current it assumes moduleId is available on config object and continues to process. Which will later fails at https://github.com/aurelia/templating-router/blob/6209bde7e8cdc35ee511b9b0cec576174dfba56c/src/route-loader.ts#L40-L46

if (moduleId === null) {
  viewModel = EmptyClass;
} else {
  // this requires container of router has passes a certain point
  // where a view model has been setup on the container
  // it will fail in enhance scenario because no viewport has been registered
  moduleId = relativeToFile(moduleId, Origin.get(router.container.viewModel.constructor).moduleId);

since it expect moduleId on that line would be a string, if not null. But it's undefined instead

Maybe we can do a check for undefined and normalized to null, so that it will ignore the instruction and process the one intended by devs, for example: navigate to home route like in @jonathaneckman 's repro

WRT deleting the viewports from previous instruction, I'll need to investigate a bit more 😃

cc @davismj @fkleuver @jwx

[davismj]

I wrote a blog last year talking about navigation strategy and recommending against it, while providing alternative strategies: http://davismj.me/blog/dynamic-routing/. I'd be curious to learn if there was some use case I hadn't accounted for.

@bigopon this might just be a documentation issue. As I wrote in the blog, I recommend against using the navigationStrategy outlined in the official docs. Maybe we need to revise that section?

@JamesFenton @jonathaneckman what's going on in navigationStrategy? before we dig into a PR I'm just curious if there's a better or easier way.