Skip to content

Fix for aux routes with empty base route - #25585

Closed
VamsiVempati wants to merge 6 commits into
angular:masterfrom
VamsiVempati:fix-for-aux-routes-with-empty-base-route
Closed

Fix for aux routes with empty base route#25585
VamsiVempati wants to merge 6 commits into
angular:masterfrom
VamsiVempati:fix-for-aux-routes-with-empty-base-route

Conversation

@VamsiVempati

@VamsiVempati VamsiVempati commented Aug 21, 2018

Copy link
Copy Markdown

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Navigating to an auxiallry route on a lazy loaded empty base route throws the following error:

Uncaught (in promise): TypeError: Cannot read property 'routes' of undefined

#12842

Issue Number: #12842

export const routes: Routes = [
  {
    path: 'login',
    component: LoginModalComponent,
    outlet: 'modal'
  },
  {
    path: '',
    children: [
        {
            path: '',
            loadChildren: './Main/main.module#MainModule',
        },
    ]
  }
]

What is the new behavior?

Navigating to an auxiallry route on a lazy loaded empty base route does not throw any error and navigates successfully

Does this PR introduce a breaking change?

[ ] Yes
[x ] No

Other information

@VamsiVempati
fix(router): lazy load auxillary roots with empty base route
83af226 
Add a null check to see if the modules are lazy loaded before returning the routes

Closes angular#12842
@VamsiVempati
fix(router): Remove non null assertion operator as it is replaced by …
4eaee39 
…null check

Remove non null assertion operator as it is replaced by null check

Fixes angular#12842
@VamsiVempati
fix(router): Fixes lint in tests for recognize
785ee13 
Fixes lint in tests for recognize
@VamsiVempati
fix(router): Fix lint in tests for recognize
420736a 
Fix lint in tests for recognize
@VamsiVempati
fix(router): Fix lint on tests for recognize
cc857c0 
Fix lint on tests for recognize
@VamsiVempati
fix(router): xdescribed new test as the test suite seems to be sporadic
34cccef 
xdescribed new test as the test suite seems to be sporadic
@nasreddineskandrani nasreddineskandrani mentioned this pull request Jan 20, 2019
Closed
@ngbot ngbot Bot added this to the needsTriage milestone Jan 24, 2019
@maxime1992

maxime1992 commented May 23, 2019

Copy link
Copy Markdown

Encountered the bug with a lazy loaded module and an aux route, I tried to apply that directly into the node_modules folder and it didn't solve the issue.

It's true that it wasn't crashing anymore but the main outlet (the one with path: '') wasn't loaded at all.

On the other hand, I did the same test with the following PR and it worked perfectly: #25483

@atscott

atscott commented Aug 5, 2020

Copy link
Copy Markdown
Contributor

Closing as this doesn't fix the underlying problem. This change only prevents the error from occurring when attempting to activate the auxiliary route with loadChildren. The required fix would be to ensure the module is actually loaded in apply_redirects before proceeding to the next stage in the router.

@atscott atscott closed this Aug 5, 2020
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 5, 2020

Copy link
Copy Markdown

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 Sep 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

area: router cla: yes

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

5 participants

@VamsiVempati @maxime1992 @atscott @kara @googlebot