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

Routing issue after upgrading from Angular 10 to Angular 11 #39952

Closed
mdudek opened this issue Dec 3, 2020 · 11 comments
Closed

Routing issue after upgrading from Angular 10 to Angular 11 #39952

mdudek opened this issue Dec 3, 2020 · 11 comments
Labels
area: router P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version router: config matching/activation/validation router: lazy loading state: confirmed
Milestone

Comments

@mdudek
Copy link

mdudek commented Dec 3, 2020

Affected Package

@angular/router

Description

Lazy loaded module routing flow ends with 'RouteConfigLoadEnd' event, next expected state 'RoutesRecognized' is not logged. The lazy loaded component is not applied to the outlet, there is no error logged in console.

btw: I'm using webpack dev stack not the Angular CLI

🌍 Your Environment

Angular Version:

11.0.3
@sonukapoor sonukapoor added area: router area: upgrade Issues related to AngularJS → Angular upgrade APIs labels Dec 3, 2020
@ngbot ngbot bot modified the milestone: needsTriage Dec 3, 2020
@mdudek
Copy link
Author

mdudek commented Dec 7, 2020

I'm open to provide access (e.g. Remote desktop) to my development machine to be able to debug the issue. The project is relatively large and needs also a server side (WebApi) to run.

@gkalpak gkalpak removed the area: upgrade Issues related to AngularJS → Angular upgrade APIs label Dec 7, 2020
@atscott atscott added the needs reproduction This issue needs a reproduction in order for the team to investigate further label Dec 8, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Dec 8, 2020
@atscott
Copy link
Contributor

atscott commented Dec 8, 2020

@mdudek - Can you first attempt to create a stackblitz reproduction? You should be able to extract the route config you're trying to activate into a demo that reproduces the error. Since the issue is with activating a certain route, it shouldn't take too long to extract the relevant parts.

@mdudek
Copy link
Author

mdudek commented Dec 8, 2020

@atscott - I recently tried to make 'similar' stackblitz project and it works without issues. Hard to say if the problem could be caused by different build stack. The Stackblitz is using Angular cli, my project is based on Webpack dev stack without Angular cli.

@atscott
Copy link
Contributor

atscott commented Dec 8, 2020

@mdudek - I see. Can you debug this a bit further to try to isolate the issue?
RouteConfigLoadEnd happens during the applyRedirects stage. Can you put a breakpoint here to see what the urlAfterRedirects value is that's being sent to the recognize stage? https://github.com/angular/angular/blob/master/packages/router/src/router.ts#L587-L589
It's curious that you report RoutesRecognized not happening, but also that there is no error. I'm wondering if the applyRedirects never finishes. If that's the case, then it might be that lazy loading never completes for one of the configs. That happens here:

if (route._loadedConfig !== undefined) {
return of(route._loadedConfig);
}
return this.runCanLoadGuards(ngModule.injector, route, segments)
.pipe(mergeMap((shouldLoadResult: boolean) => {
if (shouldLoadResult) {
return this.configLoader.load(ngModule.injector, route)
.pipe(map((cfg: LoadedRouterConfig) => {
route._loadedConfig = cfg;
return cfg;
}));
}
return canLoadFails(route);
}));
}

A few other questions:

  • What route are you trying to activate?
  • What is the Routes config for getting to that route?
  • Do any of the Routes have an empty path?
  • Can you send the stackblitz of your attempted reproduction. I know you said it works, but that would at least help clarify where you're seeing the issue.

The ApplyRedirects algorithm was one of the things that did change in v11. I'm working on another fix that will change the approach once again so maybe if the first change is what broke you, this upcoming change will fix it.

@atscott atscott added the needs: clarification This issue needs additional clarification from the reporter before the team can investigate. label Dec 8, 2020
@mdudek
Copy link
Author

mdudek commented Dec 8, 2020

@atscott - I have followed your instructions and I can confirm that applyRedirects never finishes and recognize stage is not hitted.

It seems that lazy loaded module is loaded (I checked that loaded module constructor is called). The mentioned code part (Lines 327 to 342) is called and 'route._loadedConfig = cfg; ' is called.

There are the answers to your questions:

  1. The testing route is 'https://localhost:8081/a'
  2. You can see the routes config in reproduction stackblitz https://stackblitz.com/edit/angular11-lazy-loading

@atscott
Copy link
Contributor

atscott commented Dec 8, 2020

@mdudek Hmmm... I don't really see what could be the issue. Are you able to create a minimal github reproduction? Using your project as a base, try removing all the unneeded parts until you have just the skeleton reproduction. You should be able to remove the need for the WebApi server by faking your server calls with just an rxjs Observable. Remote desktop isn't a good option for debugging.

@mdudek
Copy link
Author

mdudek commented Dec 9, 2020

@atscott - I have successfully created minimal skeleton project to demonstrate the routing issue.

Sample Project: Angular11RoutingIssue.zip

  1. Extract to folder
  2. npm install
  3. npm run start
  4. open http://localhost:8081/
  5. check router log in browser console. The last logged stage is 'RouteConfigLoadEnd'.

@atscott
Copy link
Contributor

atscott commented Dec 9, 2020

@mdudek - Awesome! Thanks for the reproduction! I did a bit of debugging and couldn't quite pin down what was causing this. All of the execution in applyRedirects looks correct, but for some reason one of the observables doesn't emit.

I was able to confirm that my pending work would fix this issue though. I'm hoping to get this submitted soon. Thanks for your patience!

@atscott atscott added regression Indicates than the issue relates to something that worked in a previous version state: confirmed and removed needs: clarification This issue needs additional clarification from the reporter before the team can investigate. needs reproduction This issue needs a reproduction in order for the team to investigate further labels Dec 9, 2020
@ngbot ngbot bot modified the milestones: Backlog, needsTriage Dec 9, 2020
@atscott atscott added the P2 The issue is important to a large percentage of users, with a workaround label Dec 9, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Dec 9, 2020
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 11, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 14, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Dec 14, 2020
…th parents

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410
atscott added a commit to atscott/angular that referenced this issue Jan 5, 2021
…th parents (angular#40029)

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410

PR Close angular#40029
atscott added a commit to atscott/angular that referenced this issue Jan 5, 2021
…th parents (angular#40029)

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410

PR Close angular#40029
atscott added a commit to atscott/angular that referenced this issue Jan 5, 2021
…th parents (angular#40029)

There are two parts to this commit:
1. Revert the changes from angular#38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in angular#38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes angular#39952
Fixes angular#10726
Closes angular#30410

PR Close angular#40029
josephperrott pushed a commit that referenced this issue Jan 5, 2021
…th parents (#40029) (#40315)

There are two parts to this commit:
1. Revert the changes from #38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in #38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.

Fixes #39952
Fixes #10726
Closes #30410

PR Close #40029

PR Close #40315
@atscott
Copy link
Contributor

atscott commented Jan 6, 2021

Hi @mdudek, could you try updating to 11.0.6 and see if this resolves your issue?

@mdudek
Copy link
Author

mdudek commented Jan 8, 2021

Hi @atscott , I can confirm that my migrated project is working without issues with version 11.0.6.

@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 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: router P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version router: config matching/activation/validation router: lazy loading state: confirmed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants