fix(router): pass outlet context to split to fix empty path named out…#67806
Draft
atscott wants to merge 1 commit intoangular:mainfrom
Draft
fix(router): pass outlet context to split to fix empty path named out…#67806atscott wants to merge 1 commit intoangular:mainfrom
atscott wants to merge 1 commit intoangular:mainfrom
Conversation
…lets
The `split` helper function in `packages/router/src/utils/config_matching.ts` was blind to the current outlet being processed. When encountering an empty path named outlet in the config, it would assume it needed to pull it in as a synthetic empty group, even if we were already in the process of resolving that very outlet!
When navigating to `/(secondary:component-copy)` with this config:
```typescript
{
path: '',
component: MainLayout,
children: [
{ path: '', outlet: 'secondary', component: SecondaryComponent, children: [{path: 'component-copy'}] }
]
}
```
The router uses `MainLayout` as a pass-through and calls `split` on its children with segments `['component-copy']`.
`split` uses the `containsEmptyPathMatchesWithNamedOutlets` helper to determine if there are any candidate empty path named outlets to pull in. Because of this, it sees `{ path: '', outlet: 'secondary' }` and says: "Ah, an empty path named outlet! I must pull it in!"
Rather than falling through to standard segment matching, it returns `UrlSegmentGroup(segments: [], children: {secondary: emptyGroup})`.
The router then tries to process `primary` (with `[]` segments) and fails because the config only has `secondary`. It also tries to process `secondary` with the `emptyGroup`. While `{ path: '', outlet: 'secondary' }` matches the empty group, its child `{ path: 'component-copy' }` fails to match because the `emptyGroup` has no segments! So both branches fail, resulting in a `NoMatch` error for the entire navigation!
Pulling in empty path named outlets IS desired when they act as siblings to segments we are matching. This has worked before and continues to work!
```typescript
{
path: 'a',
children: [
{ path: 'b', component: ComponentB },
{ path: '', component: ComponentC, outlet: 'aux' }
]
}
```
When navigating to `a/b`, `split` sees segments `['b']` and the `aux` empty path. It pulls in `aux` so it gets instantiated alongside `b`. This is correct!
If we have a named outlet with a non-empty path under an empty path parent:
```typescript
{
path: '',
component: MainLayout,
children: [
{ path: 'component-copy', outlet: 'secondary', component: ComponentE }
]
}
```
When we navigate to `/(secondary:component-copy)`:
- `split` uses `containsEmptyPathMatchesWithNamedOutlets` to see if there are any empty path named outlets. Since it only sees `path: 'component-copy'`, it returns `false`.
- It falls through to standard segment matching, which finds `component-copy` in the segments array and activates it flawlessly!
This worked perfectly before the fix because it didn't use `containsEmptyPathMatchesWithNamedOutlets`.
The fix passes the **current active outlet context** into `split`. If `split` finds an empty path named outlet that matches the outlet we are already processing, it ignores it as a pull-in candidate.
When evaluating `MainLayout` children for `secondary`:
- URL Segments left to process: `['component-copy']`
- Current Outlet: `secondary`
- `childConfig`: `[{ path: '', outlet: 'secondary' }]`
Previously, `split` saw the empty path and pulled it in as a synthetic empty group, breaking matching. Now, since `getOutlet(r) === outlet` (both are `secondary`), the fix ignores it. Instead of returning empty segments, it **falls through to standard segment matching**, which successfully find the `component-copy` segment!
When evaluating `ComponentA` children for `primary`:
- URL Segments left to process: `['b']`
- Current Outlet: `primary`
- `childConfig`: `[{ path: 'b' }, { path: '', outlet: 'aux' }]`
Since `getOutlet(aux) !== primary`, the fix **does not ignore it**. `split` pulls in `aux: emptyGroup` as a sibling, instantiating `ComponentC` alongside `ComponentB`. This preserves correct behavior for auxiliary outlets!
fixes angular#67708
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…lets
The
splithelper function inpackages/router/src/utils/config_matching.tswas blind to the current outlet being processed. When encountering an empty path named outlet in the config, it would assume it needed to pull it in as a synthetic empty group, even if we were already in the process of resolving that very outlet!When navigating to
/(secondary:component-copy)with this config:The router uses
MainLayoutas a pass-through and callsspliton its children with segments['component-copy'].splituses thecontainsEmptyPathMatchesWithNamedOutletshelper to determine if there are any candidate empty path named outlets to pull in. Because of this, it sees{ path: '', outlet: 'secondary' }and says: "Ah, an empty path named outlet! I must pull it in!" Rather than falling through to standard segment matching, it returnsUrlSegmentGroup(segments: [], children: {secondary: emptyGroup}). The router then tries to processprimary(with[]segments) and fails because the config only hassecondary. It also tries to processsecondarywith theemptyGroup. While{ path: '', outlet: 'secondary' }matches the empty group, its child{ path: 'component-copy' }fails to match because theemptyGrouphas no segments! So both branches fail, resulting in aNoMatcherror for the entire navigation!Pulling in empty path named outlets IS desired when they act as siblings to segments we are matching. This has worked before and continues to work!
When navigating to
a/b,splitsees segments['b']and theauxempty path. It pulls inauxso it gets instantiated alongsideb. This is correct!If we have a named outlet with a non-empty path under an empty path parent:
When we navigate to
/(secondary:component-copy):splitusescontainsEmptyPathMatchesWithNamedOutletsto see if there are any empty path named outlets. Since it only seespath: 'component-copy', it returnsfalse.component-copyin the segments array and activates it flawlessly!This worked perfectly before the fix because it didn't use
containsEmptyPathMatchesWithNamedOutlets.The fix passes the current active outlet context into
split. Ifsplitfinds an empty path named outlet that matches the outlet we are already processing, it ignores it as a pull-in candidate.When evaluating
MainLayoutchildren forsecondary:['component-copy']secondarychildConfig:[{ path: '', outlet: 'secondary' }]Previously,
splitsaw the empty path and pulled it in as a synthetic empty group, breaking matching. Now, sincegetOutlet(r) === outlet(both aresecondary), the fix ignores it. Instead of returning empty segments, it falls through to standard segment matching, which successfully find thecomponent-copysegment!When evaluating
ComponentAchildren forprimary:['b']primarychildConfig:[{ path: 'b' }, { path: '', outlet: 'aux' }]Since
getOutlet(aux) !== primary, the fix does not ignore it.splitpulls inaux: emptyGroupas a sibling, instantiatingComponentCalongsideComponentB. This preserves correct behavior for auxiliary outlets!fixes #67708