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

Navigate to home after login does not work with Angular 11 #576

Closed
1 of 4 tasks
csname1910 opened this issue Nov 20, 2020 · 6 comments
Closed
1 of 4 tasks

Navigate to home after login does not work with Angular 11 #576

csname1910 opened this issue Nov 20, 2020 · 6 comments
Labels

Comments

@csname1910
Copy link

csname1910 commented Nov 20, 2020

I'm submitting a...

  • Bug report
  • Feature request
  • Documentation issue or request
  • Question

Current behavior

After upgrade to Angular 11 the login button does not work anymore.
URL in Chrome is updated to 'home', but it stays on the login screen.
You have to hit F5 for refresh to get to the home screen.

Expected behavior

After pressing login button the home screen should appear.

Minimal reproduction of the problem with instructions

ngx new
ng update @angular/cli @angular/core
npm start

The problem seems to be a change in shouldReuseRoute, see here:

https://update.angular.io/?l=3&v=10.0-11.0

The bug can be fixed by changing shouldReuseRoute to:

public shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean {
  return future.routeConfig === curr.routeConfig || (curr.url.length > 0 ? curr.data.reuse : future.data.reuse);
}

But this is probably not a good way to fix it.

Environment



ngX-Rocket: 9.1.0
Node.js: v12.18.0
Npm: 6.14.8
OS: win32 x64 10.0.19042

Generated project options:
{
  "generator-ngx-rocket": {
    "version": "9.1.0",
    "props": {
      "location": "path",
      "strict": false,
      "skipInstall": false,
      "skipQuickstart": false,
      "initGit": true,
      "usePrefix": true,
      "appName": "testnol",
      "target": [
        "web"
      ],
      "pwa": false,
      "ui": "material",
      "layout": "side-menu",
      "auth": true,
      "lazy": false,
      "angulartics": false,
      "languages": [
        "en-US"
      ],
      "tools": [
        "prettier",
        "hads"
      ],
      "utility": [],
      "deploy": "none",
      "projectName": "testnol",
      "packageManager": "npm",
      "mobile": [],
      "desktop": []
    }
  }
}

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

Angular CLI: 11.0.2
Node: 12.18.0
OS: win32 x64

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

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1001.7
@angular-devkit/build-angular   0.1001.7
@angular-devkit/core            10.1.7
@angular-devkit/schematics      11.0.2
@angular/cdk                    10.2.7
@angular/flex-layout            10.0.0-beta.32
@angular/material               10.2.7
@schematics/angular             11.0.2
@schematics/update              0.1100.2
rxjs                            6.6.3
typescript                      4.0.5

Others:

@athisun
Copy link
Contributor

athisun commented Nov 23, 2020

I've just run into this issue as well. Forgive me if I'm wrong, but the change documented on https://update.angular.io/?l=3&v=10.0-11.0 seems to suggest that we should be able to solve this problem by changing shouldReuseRoute to:

public shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean {
  return future.routeConfig === curr.routeConfig || curr.data.reuse;
}

Notice that future.data.reuse is now curr.data.reuse.

Quote for reference:

If you use the Router's RouteReuseStrategy, the argument order has changed. When calling RouteReuseStrategy#shouldReuseRoute previously when evaluating child routes, they would be called with the future and current arguments swapped. If your RouteReuseStrategy relies specifically on only the future or current snapshot state, you may need to update the shouldReuseRoute implementation's use of future and current ActivateRouteSnapshots.

@csname1910
Copy link
Author

I've just run into this issue as well. Forgive me if I'm wrong, but the change documented on https://update.angular.io/?l=3&v=10.0-11.0 seems to suggest that we should be able to solve this problem by changing shouldReuseRoute to:

public shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean {
  return future.routeConfig === curr.routeConfig || curr.data.reuse;
}

This does not work, because the arguments are not always swapped, but only for child routes.
You have to change like this:

public shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean {
  return future.routeConfig === curr.routeConfig || (curr.url.length > 0 ? curr.data.reuse : future.data.reuse);
}

But this looks like a hack, there should be a better way.

@athisun
Copy link
Contributor

athisun commented Nov 23, 2020

@csname1910 Yes, you're right. I've worked with your fix and it seems to break for me in cases where we navigate from a child route with a reusable component (i.e. shell) to a route with non-reused components (e.g. from home to login). For now I've simplified to the following, which fixes navigation but likely breaks component reuse:

return future.routeConfig === curr.routeConfig;

@csname1910
Copy link
Author

@csname1910 Yes, you're right. I've worked with your fix and it seems to break for me in cases where we navigate from a child route with a reusable component (i.e. shell) to a route with non-reused components (e.g. from home to login). For now I've simplified to the following, which fixes navigation but likely breaks component reuse:

return future.routeConfig === curr.routeConfig;

Yes, this breaks component reuse, so the ShellComponent is always recreated. This is bad, because if you use a MatSidenav in ShellComponent it is always closed if you switch to another route.

I could not reproduce your problem with navigating from home to login with my fix.
For me it does work.

@athisun
Copy link
Contributor

athisun commented Nov 24, 2020

For reference, this is the original issue: angular/angular#18374

@csname1910 Does atscott's solution work for you? https://stackblitz.com/edit/angular-ivy-ksf9cm

// Reuse the route if the RouteConfig is the same, or if both routes use the
// same component, because the latter can have different RouteConfigs.
return future.routeConfig === curr.routeConfig ||
    (!!future.routeConfig?.component &&
      future.routeConfig?.component === curr.routeConfig?.component);

@csname1910
Copy link
Author

// Reuse the route if the RouteConfig is the same, or if both routes use the
// same component, because the latter can have different RouteConfigs.
return future.routeConfig === curr.routeConfig ||
    (!!future.routeConfig?.component &&
      future.routeConfig?.component === curr.routeConfig?.component);

@athisun This does work for me, thank you!

@sinedied sinedied added the bug label Jan 12, 2021
sinedied added a commit that referenced this issue Jan 13, 2021
sinedied added a commit that referenced this issue Jan 13, 2021
sinedied added a commit that referenced this issue Jan 14, 2021
ci-rebot pushed a commit that referenced this issue Jan 14, 2021
# [9.2.0](9.1.0...9.2.0) (2021-01-14)

### Bug Fixes

* app not loading with Electron v11 ([1444bf0](1444bf0))
* incorrect hads version ([ba20ff1](ba20ff1))
* rename env script to fix execution in some environments (closes [#575](#575)) ([ec93fdb](ec93fdb))
* update electron templates ([3433465](3433465))
* update generator dependencies ([bda29d9](bda29d9))
* update jest config ([4b4a357](4b4a357))
* update packages and fix peer dependencies ([64098dc](64098dc))
* update RouteReusableStrategy for Angular 11 ([#576](#576)) ([3077f5b](3077f5b))

### Features

* add brazilian portuguese language ([1a6d728](1a6d728))
* enable webpack 5 ([8592a71](8592a71))
* migrate to @ngneat/until-destroy (fix [#577](#577)) ([3a77fcb](3a77fcb))
* update editorconfig (fixes [#580](#580)) ([9f036b3](9f036b3))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants