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

Guards and Resolvers don't wait for APP_INITIALIZER, if RouterModule initialNavigation: 'enabled' is set #1623

Closed
felixfiskare opened this issue Mar 26, 2020 · 22 comments

Comments

@felixfiskare
Copy link

🐞 bug report

Affected Package

@angular/router/RouterModule
@angular/core/APP_INITIALIZER

Is this a regression?

Yes, this used to work all the way up until Angular 8.2.14.
This bug started to appear once I updated to Angular 9 (9.0.7)
EDIT: I updated to Angular 9.1, but the problem persists.

Description

After Upgrading I noticed, that some parts of my application wouldn't start any longer, because the configuration they are depending on is empty.
I used to load this configuration from a config file on my server. I'm using APP_INITIALIZER for this. Upon further investigation I found out, that my guards and resolvers run before my APP_INITIALIZER promise resolves.
Finally I could pin-point this error to app-routing.module.ts. When I uncomment initialNavigation: 'enabled' then the guards and resolvers wait till the APP_INITIALIZER promise resolve.
Once enabled, the guards and resolvers run immediately and don't wait for APP_INITIALIZER.

@NgModule({
  imports: [
    RouterModule.forRoot(routes, {
    // initialNavigation: 'enabled'
})
  ],
  exports: [RouterModule]
})
export class AppRoutingModule { }

🔬 Minimal Reproduction

Open the console to see when different parts of the application get executed.
MinimalGuard and ResolverService should get executed after AppInitializerService Promise resolved, but unfortunately they get executed before.

https://stackblitz.com/github/felixfiskare/ResolverAndGuardBeforeInit

https://github.com/felixfiskare/ResolverAndGuardBeforeInit

🔥 Exception or Error

guardAndResolverBeforeAppInit

🌍 Your Environment

Angular Version:

 ng version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 9.0.7
Node: 13.11.0
OS: linux x64

Angular: 9.0.7
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.7
@angular-devkit/build-angular     0.900.7
@angular-devkit/build-optimizer   0.900.7
@angular-devkit/build-webpack     0.900.7
@angular-devkit/core              9.0.7
@angular-devkit/schematics        9.0.7
@ngtools/webpack                  9.0.7
@schematics/angular               9.0.7
@schematics/update                0.900.7
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.41.2
@felixfiskare
Copy link
Author

I just found out, that initialNavigation: 'enabled' was added automatically while upgrading from angular 8 to angular 9

@atscott
Copy link

atscott commented Apr 7, 2020

I just found out, that initialNavigation: 'enabled' was added automatically while upgrading from angular 8 to angular 9

Can you clarify this in the context of marking this as a regression? I tested the stackblitz demo, going all the way back to Angular 6.0.0 and the behavior remained the same: when initialNavigation is set to 'enabled', the guards and resolvers run before APP_INITIALIZER.

@atscott
Copy link

atscott commented Apr 7, 2020

I've looked into the docs and the router_module and determined that guards and resolvers being run before the user's own APP_INITIALIZER is working as intended.

  • The router has its own APP_INITIALIZER.
  • All APP_INITIALIZERS are run in parallel.
  • When using initialNavigation: 'enabled' the router's APP_INITIALIZER triggers a navigation. This means that if your initial navigation depends on your own APP_INITILIZER being completed first, you cannot use initialNavigation: 'enabled'.
  • ApplicationRef.bootstrap (which happens after APP_INITIALIZER completion) calls all APP_BOOTSTRAP_LISTENERs, which triggers initial navigation if using initialNavigation: 'legacy_enabled'
  • All of this also seems to follow the documented behavior for initialNavigation:

'enabled' - The initial navigation starts before the root component is created. The bootstrap is blocked until the initial navigation is complete. This value is required for server-side rendering to work.
'legacy_enabled'- (Default, for compatibility.) The initial navigation starts after the root component has been created. The bootstrap is not blocked until the initial navigation is complete.

That said, if initialNavigation: 'enabled' was added automatically when upgrading as you say, this is a bug in the migration since the initialNavigation options are not equivalent.

@felixfiskare
Copy link
Author

I just found out, that initialNavigation: 'enabled' was added automatically while upgrading from angular 8 to angular 9

Can you clarify this in the context of marking this as a regression? I tested the stackblitz demo, going all the way back to Angular 6.0.0 and the behavior remained the same: when initialNavigation is set to 'enabled', the guards and resolvers run before APP_INITIALIZER.

The error appeared after I upgraded to Angular 9. Only later I found out, that 'initialNavigation' was set automatically during the upgrade.

Therefore I agree with you, that this would classify as a migration bug.

I think the value was set, because I'm using SSR.
How am I supposed to have working guards/resolvers AND SSR? The documentation states (as you quoted as well), that 'initialNavigation: enabled' is needed for SSR.

@atscott
Copy link

atscott commented Apr 8, 2020

@felixfiskare - Can you update the configuration you're loading to be an Observable such as a ReplaySubject or BehaviorSubject? Whatever depends on those can then read from the Observable instead of attempting to access a value before the value is resolved. If that solution doesn't make sense, can you provide some more details how your app is set up and what depends on the data being retrieved in the APP_INITIALIZER? A new stackblitz to demonstrate the issue would help as well - the current one is just showing execution order, rather than demonstrating the issue you're experiencing.

@felixfiskare
Copy link
Author

Thank you for that pointer @atscott and sorry for the delay. It took some time to encapsulate everything in observables, but now everything works!
So I guess, the only thing left to do is to emit a warning when upgrading to angular 9, that initialNavigation: 'enabled' will be set and which consequence this could have.

@CaerusKaru
Copy link
Member

@felixfiskare This is caused by the NgUniversal schematics, we added it to avoid flicker when bootstrapping on the client. We'll move this issue over to angular/universal so that we can track a proper resolution. This may be a warning or a prompt, or possibly a link to documentation about certain pitfalls with adding initialNavigation, like the one you encountered here.

@alan-agius4 alan-agius4 transferred this issue from angular/angular Apr 15, 2020
@CaerusKaru CaerusKaru self-assigned this Jun 14, 2020
@CaerusKaru CaerusKaru added this to the 10.1 milestone Jun 14, 2020
@CaerusKaru
Copy link
Member

First phase of this is tracked in #1730, we'll add this section once it's merged.

@BruneXX
Copy link

BruneXX commented Sep 11, 2020

Hi Guys, is this fix currently merged into v10.1.x ? thanks!

@AceVentura
Copy link

@BruneXX I tried with 10.1.1 and 10.1.2 and the issue still remains

@BruneXX
Copy link

BruneXX commented Sep 18, 2020

Hi @AceVentura Sad to hear, I think we'll need to wait for this one.. Thanks

@nicolae-olariu
Copy link

Hi, any update of when is this going to be merged? Thanks!

@Abdulrrahman
Copy link

Hi, I'm facing the same issue here too

@nils-thomann
Copy link

Hi @CaerusKaru,
I'm facing the same issue. I need to wait for the APP_INITIALIZER before the initial navigation on the SSR. I've seen a few workarounds by manually call the router.initialNavigation() function at the end of the APP_INITIALIZER function, but unfortunately this doesn't work for me. Are there some updates on a potential fix? Or do you know a workaround for this issue? I'm currently using Angular 9 but an upgrade to Angular 10 or 11 would be possible. Thanks!

@AceVentura
Copy link

@nils-thomann the issue is still there (v11.1.1). Your only choice is to change your project to not be dependent on APP_INITIALIZER. That's what I've done.

@nils-thomann
Copy link

Hi @AceVentura, thanks for your response. Do you have some recommendations were to do stuff that has to be done before the initial navigation without using the APP_INITIALIZER provider? Or if you have a code example that you could show me it would be even better. Thanks for your help :)

@AceVentura
Copy link

@nils-thomann, I believe that really depends on your project. For example, in my project I have dynamic routes, which I only know by doing an API Call.
So now I have a few assumptions in my routes (so, now it's not 100% dynamic, but like 80%) and I have a Resolver that makes the same call that I used to do in APP_INITIALIZER.

@davidediak
Copy link

Yep, same issue here.
I "resolved" by adding an "if isPlatformBrowser(this.platformId)" in each guard...

@Seemspyo
Copy link

Seemspyo commented Apr 7, 2021

@nguniversal/common@11.2.1 still has no luck.

This is big issue for my case since I generally use APP_INITIALIZER for ensure auth(or user) states.

I tried wrapping all routes like

{ path: "", component: RootComponent, canActivate: [ EnsureAuthGuard ], children: [
  { path: "", component: SomeComponent, canActivate: [ DoSomethingWithAuthGuard ] } // errors or always not accessable.
] }

but as mentioned angular issue, this is currently not working since all guard are runs in parallel. CanLoad and CanActivateChild are the same.

Since initialNavigation: 'enabledBlocking'(same as 'enabled' but 'enabled' deprecated since v11) is required for server-side rendering, as v11 it displays an warning when initialNavigation is not enabled or enabledBlocking.

So currently, only solution is resolve required states before all guards run.

constructor(
  private auth: EnsureAuthGuard
) { }

async canActivate() {
  await this.auth.ensureAuth();
  // ... now auth state surely resolved ....
}

Hope v12 can resolve this.

@nhanpdnguyen
Copy link

nhanpdnguyen commented Aug 29, 2021

Hi guys, here's the workarounds I'm applying on my project (v11). It works with SRR as well. Hope it helps someone

  • There will be a ReplaySubject userInfoSubject to inform the authentication status
  • APP_INITIALIZER will call AuthenticationService to check for authentication status and emit the retrieved authentication status by userInfoSuject.next(userInfo). Note that whenever the userInfo changes (by sign in, sign up...) we need to emit the authentication status too
  • CanActivate will subscribe to this replay subject
// app.module.ts
function initializeApp(
    authenticationService: AuthenticationService,
): () => Promise<any> | undefined {
    return () => {
        if (typeof window !== 'undefined') {
            return authenticationService.getUserInfo().toPromise();
        }
        return;
    };
}
// authentication.service.ts
export class AuthenticationService {
    userInfo: UserLoginInfo | null | undefined;
    userInfoSubject = new ReplaySubject<UserLoginInfo | null>(1);

    constructor(
        @Inject(PLATFORM_ID) private platformId: Record<string, unknown>,
        @Optional()
        @Inject('authenticatedUserData')
        public authenticatedUserDataFromSsr: UserLoginInfo | null | undefined,
        private httpClient: HttpClient,
        private transferStateService: TransferStateService,
    ) {
        if (isPlatformServer(this.platformId)) {
            this.transferStateService.set<UserLoginInfo | null | undefined>(
                'authenticatedUserData',
                authenticatedUserDataFromSsr
            );
        }
        this.userInfo = this.transferStateService.get('authenticatedUserData');
        if (this.isFetched(this.userInfo)) {
            this.userInfoSubject.next(this.userInfo);
        }
    }

    isFetched(data: any): boolean {
        return typeof data !== 'undefined';
    }

    getUserInfo(): Observable<UserLoginInfo | null | undefined> {
        if (this.isFetched(this.userInfo)) {
            return of(this.userInfo);
        }
        return this.httpClient.get<{ user: UserLoginInfo | null }>('/userData').pipe(
            map((response) => response.user),
            catchError((err) => {
                this.userInfoSubject.next(null);
                return of(null);
            }),
            tap((userInfo) => {
                this.userInfo = userInfo;
                this.userInfoSubject.next(userInfo);
            })
        );
    }
...
}
// authentication.guard.ts
export class AuthenticationGuard implements CanActivate {
    constructor(private authenticationService: AuthenticationService, private router: Router) {}

    canActivate(
        route: ActivatedRouteSnapshot,
        state: RouterStateSnapshot
    ): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
        return this.authenticationService.userInfoSubject.pipe(
            map((userInfo) => {
                if (userInfo) {
                    return true;
                }
                return this.router.parseUrl('/');
            })
        );
    }
}

@alan-agius4
Copy link
Collaborator

As per above comments, this is working as intended.

@alan-agius4 alan-agius4 closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2023
@alan-agius4 alan-agius4 removed this from the 10.1 milestone May 23, 2023
@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 Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests