Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router): correctly deactivate children with componentless parent #40196

Closed
wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Dec 18, 2020

During route activation, a componentless route will not have a context created
for it, but the logic continues to recurse so that children are still
activated. This can be seen here:

if (future.component) {
// If we have a normal route, we need to go through an outlet.
const context = parentContexts.getOrCreateContext(future.outlet);
this.activateChildRoutes(futureNode, currNode, context.children);
} else {
// if we have a componentless route, we recurse but keep the same outlet map.
this.activateChildRoutes(futureNode, currNode, parentContexts);
}

The current deactivation logic does not currently account for componentless routes.

This commit adjusts the deactivation logic so that if a context cannot
be retrieved for a given route (because it is componentless), we
continue to recurse and deactivate the children using the same
parentContexts in the same way that activation does.

Fixes #20694
Fixes #40196

regular presubmit and global
additional integration tests. Compared with results from clean client and a second run from this PR, they have the same number of failures so these are likely pre-existing and/or flakes. Pending code approval, all g3 signs seem to indicate that this should be okay to merge.

@atscott atscott added action: review The PR is still awaiting reviews from at least one requested reviewer area: router target: patch This PR is targeted for the next patch release labels Dec 18, 2020
@ngbot ngbot bot modified the milestone: Backlog Dec 18, 2020
@google-cla google-cla bot added the cla: yes label Dec 18, 2020
@atscott atscott requested a review from mhevery December 18, 2020 21:43
During route activation, a componentless route will not have a context created
for it, but the logic continues to recurse so that children are still
activated. This can be seen here:
https://github.com/angular/angular/blob/362f45c4bf1bb49a90b014d2053f4c4474d132c0/packages/router/src/operators/activate_routes.ts#L151-L158

The current deactivation logic does not currently account for componentless routes.

This commit adjusts the deactivation logic so that if a context cannot
be retrieved for a given route (because it is componentless), we
continue to recurse and deactivate the children using the same
`parentContexts` in the same way that activation does.

Fixes angular#20694
@atscott atscott added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 6, 2021
atscott added a commit that referenced this pull request Jan 6, 2021
…40196)

During route activation, a componentless route will not have a context created
for it, but the logic continues to recurse so that children are still
activated. This can be seen here:
https://github.com/angular/angular/blob/362f45c4bf1bb49a90b014d2053f4c4474d132c0/packages/router/src/operators/activate_routes.ts#L151-L158

The current deactivation logic does not currently account for componentless routes.

This commit adjusts the deactivation logic so that if a context cannot
be retrieved for a given route (because it is componentless), we
continue to recurse and deactivate the children using the same
`parentContexts` in the same way that activation does.

Fixes #20694

PR Close #40196
@atscott atscott closed this in 13020f9 Jan 6, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: router cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
2 participants