aurelia / router

A powerful client-side router.
MIT License
120 stars 115 forks source link

Missing parentOverrideContext when a nested child route is loaded at first time #548

Open 3cp opened 7 years ago

3cp commented 7 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: When a nested child route is loaded at first time, the parentOverrideContext is missing.

https://gist.run/?id=24bddb1ca8e502bcac280fbb3368dd30

This demo setup a nested route, two routes at top level, plus two nested routes in one of the top level page.

The top app.js module defined a variable face with value ":-)". Due to all components loaded by aurelia-router have access to app.js component through overrideContext chain, the app is able to display the face in every leaf pages.

But there is a problem, when nested page "/two/one" is loaded first time, the face is missing, but if you visit "/two/two" then visit "/two/one" again, the face comes back.

A debug log in two/one.js prints bindingContext and overrideContext in bind() callback, it reveals the parentOverrideContext was missing when page "/two/one" was loaded first time, it comes back to normal when it was loaded second time. screen shot 2017-11-06 at 12 50 50 pm

Expected/desired behavior:

Lichtjaeger commented 6 years ago

I have the same problem. Is there a workaround until this is fixed?

3cp commented 6 years ago

See the PR above for a fix.

3cp commented 6 years ago

In my app, I have to inject a common obj to avoid using binding scope chain.

davismj commented 6 years ago

This is definitely a bug as the same behavior should occur on the first and each subsequent navigation. Thanks for the PR.

@Lichtjaeger If you really need access to a parent scope in a child route, there is likely a better way to approach the problem. Here are two suggestions.

Use a parameterized route (highly recommended)

There's a temptation to jump on child routes when a parameterized route would do. For the above example, the parent and child routers have the following structure:

parent

configureRouter(config) {
  config.map([ { route: 'two', moduleId: 'two' } ])
}

child

  config.map([ 
    { route: 'one', moduleId: 'one' },
    { route: 'two', moduleId: 'two' }
])

This particular example could be rewritten with a parameterized route in a single router:

recommendation

configureRouter(config)
  config.map([ { route: 'two/:type', moduleId: 'two' } ]);
});

In this case, you'll have the exact same types of routes, but all the content would live within the same view / viewModel pair and therefore the same bindingContext. You would not need any messy workarounds requiring diving into the parent binding context.

Use a service pattern

The above strategy works in all the most common cases. If it does not work in your particular case, then consider leveraging the service pattern. The service pattern involves creating a third module that provides common data across your application.

my-service.js

export class DataService {
  constructor() {
    this.foo = 'bar';
  }
}

This only works when the data you want to share is not state data. For example, this is not the pattern to use if you want to share which element is selected in the parent view. (That particular use case is better handled by the parameterized route, though).

Inject the parent (not recommended!)

If all else fails, consider injecting the parent element itself. This strategy requires that the parent view is an application singleton, and it is in general a sloppy and less maintainable solution. It can work, and I've used it once or twice in very special use cases, so I'm including it here:

child.js

// Avoid doing this as much as possible. There's almost always a better way.
@inject(ParentViewModel) 
export class ChildViewModel { 
  constructor(parent) { 
    this.foo = parent.foo;
  }
}
3cp commented 6 years ago

Thanks for all the options @davismj.

I am using option2 "use a service pattern", it looks like the best I can get.

Option1 has one limitation, parameterized routes are not in router.navigation. Hard to build menu for them.

davismj commented 6 years ago

@huochunpeng You're right. Personally, I never use the navModel. It's too limited for nearly every application I build. I've worked on a lot of applications and haven't found a one size fits all solution. Going to continue that conversation here: https://github.com/aurelia/router/issues/90#issuecomment-366575999.

In any case, I recommend building the best routing structure for the router, option 1, and solving the nav model problem differently. It's an easier problem to solve.

3cp commented 6 years ago

I am with you.

I am not sure whether to inflate that thread even more with following very rough idea.

If there is a way to support parameterized routes in router.navigation, even a hint, that will be great to reduce the need of dynamic route.

It could be just static route with dynamic navigableTo, no more need of dynamic route itself (at least for my use case, the only reason I use dynamic route is for the menu links).

{
  route: 'something/:type',
  moduleId: 'something', 
  navigableTo: myLimitedTypes.map(type => {
    return { title: type.label, params: {type: type.value} };
  })
}
Lichtjaeger commented 6 years ago

Thank you @davismj.

In the end, I definitely will use a service because these states will be from a backend logic. The project I'm working on is in a very early stage so I thought using a transparent state that is inherited from parent to child would be easier to debug for now.