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

[4.0.0.beta-17] ActivatedRoute.queryParams break after navigation #16736

Closed
h4rm opened this issue Dec 14, 2018 · 19 comments

Comments

8 participants
@h4rm
Copy link

commented Dec 14, 2018

Bug Report

Ionic version:
4.0.0.beta-17 and 4.0.0.beta-18

Current behavior:
Subscriptions on ActivatedRoute.queryParams do not fire after navigating away from and returning to that page. This is particullary problematic for base pages of tabs that are cached. The problem does not occur on sub-pages because they are re-initialized i.e. onInit fires again and again (e.g. on speaker-detail in ionic-conference app).

Expected behavior:
Navigation should not interfere with the queryParams subscription.

Steps to reproduce:

  1. Clone ionic-conference-app
  2. Add code below
  3. Test with "Test" button on speakers list. Observe console log of queryParams. Navigate to speaker detail and back. Try "Test" button again and observe no console output.
  4. Test same with Test button speaker detail page. Navigate from base to detail and back and to detail again - logs are fired.

Related code:

speaker-list.html

<ion-content class="outer-content">
   <!-- TEST BUTTON -->
  <ion-button (click)="test()">Test</ion-button>

  <ion-list>
...

in speaker-list.ts modify

...
export class SpeakerListPage implements OnInit {
  speakers: any[] = [];

  constructor(
    public actionSheetCtrl: ActionSheetController,
    public confData: ConferenceData,
    public inAppBrowser: InAppBrowser,
    private route: ActivatedRoute,
    private router: Router
  ) {}

  ngOnInit() {
    console.log('Init');
    this.route.queryParams.subscribe(test => console.log(test));
  }

  test() {
    this.router.navigate(['./'], { queryParams: { test: Math.random() }, relativeTo: this.route });
  }
...

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.5.0 (C:\Users\Ferdinand\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework               : @ionic/angular 4.0.0-beta.17
   @angular-devkit/build-angular : 0.10.7
   @angular-devkit/schematics    : 7.1.3
   @angular/cli                  : 7.1.3
   @ionic/angular-toolkit        : 1.2.0

Cordova:

   cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
   Cordova Platforms     : android 7.1.4
   Cordova Plugins       : cordova-plugin-ionic-keyboard 2.1.3, cordova-plugin-ionic-webview 2.2.5, (and 8 other plugins)

System:

   NodeJS : v10.11.0 (C:\Program Files\nodejs\node.exe)
   npm    : 6.4.1
   OS     : Windows 10

@ionitron-bot ionitron-bot bot added the triage label Dec 14, 2018

@h4rm h4rm changed the title [4.0.0.beta-17ActivatedRoute.queryParams break after navigation [4.0.0.beta-17] ActivatedRoute.queryParams break after navigation Dec 14, 2018

@h4rm

This comment has been minimized.

Copy link
Author

commented Dec 14, 2018

Problem persists if this.route.queryParams.subscribe(test => console.log(test)); is moved to ionViewDidEnter. The whole observable seems to be corrupted.

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

Thanks for your issue! Could you please create a repo? On my app (I also copied your snippet) this is working without problems.

UPDATE: Ah I think you mean you let the component in your stack and open it again?

@paulstelzer paulstelzer added needs: reply and removed triage labels Dec 14, 2018

@h4rm

This comment has been minimized.

Copy link
Author

commented Dec 14, 2018

The problem is that the base page of the tab is cached (as it should) and when one navigates away and back to that page, the cache is retrieved but somehow the queryParams of that ActivatedRoute is not firing anymore, or maybe ActivatedRoute is not restored correctly.

I have the feeling that it might be related to #16534 where ActivatedRoute also introduces problems when the correct href is rendered. Although in that case, ActivatedRoute is sometimes not correctly restored when using relative navigation, which is not the case here.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply labels Dec 14, 2018

@blackfan23

This comment has been minimized.

Copy link

commented Dec 14, 2018

@paulstelzer
I provided a repo here with the changes described by @h4rm

https://github.com/blackfan23/conference_app_beta_nav_bug

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

I think the problem of the issue is the following:

Ionic puts the page into the stack, means the component will be not destroyed, instead it's just hidden (Read more here https://medium.com/@paulstelzer/ionic-4-and-the-lifecycle-hooks-4fe9eabb2864).

That's the reason why ngOnInit() will not be fired and I think it's the same why the router not working as you expected because on going back to the page, the page will be shown again (not created, it was already created - it will only be shown). And I think this leads that the queryParams not working as you expected.

A workaround would be to avoid the ionic stack functionality and just navigate by root. But as written here #16534 (comment) - we are aware of this problem, but it's not trivial :(

BTW, in your use case you stay on the same page. I had the same issue on my app, too. I decided to not use queryParams and relative routes. Instead I am using it only on the first load and then save it in a form (I need this form, so in my case it was already there). Works great :)

@h4rm

This comment has been minimized.

Copy link
Author

commented Dec 14, 2018

Hey Paul,

thanks for your remarks, What exactly do you mean by "navigate by root". If it meants that we avoid relative routes as used above, I have also tried to navigate via absolute roots

    this.router.navigate(['app', 'tabs', { outlets: { speakers: ['speakers'] } }], { queryParams: { test: Math.random() } });

and the problem persists.

Also, I understand that ionic only hides the base page and shows it again when navigating back. Unfortunately that does not explain why the first subscription in ngOnInit() does not fire again as the page component still exists in the background the the subscription should be still valid.

EDIT: I came to the same conclusion that using queryParams only on initial load might work in same cases but e.g. when you use "test" params on the base page, navigate to a child page, put a link there back to the base page with another test=Math.Random(), then the change is not propagated.

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

As said you are the special case (like in my app) that you are stay on the same page. I presented my workaround in my comment above.

The first subscription should work, I get the params on first load in my app.

@h4rm

This comment has been minimized.

Copy link
Author

commented Dec 14, 2018

Okay, we found a viable work-around.

If we put the the queryParams subscription into a seperate service

import { Injectable } from '@angular/core';
import { ActivatedRoute } from '@angular/router';

@Injectable({
  providedIn: 'root',
})
export class TestService {
  constructor(private route: ActivatedRoute) {
    this.route.queryParams.subscribe(test => console.log(test));
  }
}

it will react to all param changes as expected. In particular, in the example above "when you use "test" params on the base page, navigate to a child page, put a link there back to the base page with another test=Math.Random()" - the queryParams correctly fires.
It does not fire, if the subscription is done in the ngOnInit which is the core problem that I do not understand.

So summa sumarum: queryParams subscriptions in angular components are getting corrupted while queryParams subscriptions outside of these components work as intended.

EDIT: I guess the injected ActivatedRoute in the component stays somewhere open, the subscriptions will also be open, but upon navigating away and back, this injected route: ActivatedRoute is not used again, but instead a new ActivatedRoute is created and put into its place (I don't know the deep details of injection of angular).

@KevinKelchen

This comment has been minimized.

Copy link

commented Dec 14, 2018

This sounds similar to some of what I've observed in my issue (#16516).

The observable subscriptions do not fire when revisiting a Page that is cached in the stack. This creates a problem for me because I'm trying to forward navigate to a Page that gets re-used because it's in the stack and matches Ionic's URL uniqueness rules (which excludes query parameters). However, as it's a forward navigation, I have new navigation parameters I'm storing in a shared service that is keyed off of a URL query parameter. Because the new query parameter isn't getting pushed to the observable subscription the Page doesn't grab the new navigation data from the service and retains the old data.

When I debugged a little it seemed that the observables in the ActivatedRoute that are having the new values pushed into them have no subscribers, so perhaps they are not the same observable instances originally subscribed to in the Page's ngOnInit. Sounds similar to what @h4rm describes.

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

@KevinKelchen I think your's could also be #16744

@KevinKelchen

This comment has been minimized.

Copy link

commented Dec 14, 2018

Thanks for the quick response, @paulstelzer!

I don't think I share the same issue as expressed in 16744. The issue I mention with the ion-back-button in 16516 (arguably there are two separate but very related issues mentioned in there) is that it doesn't work like the browser's history stack, Android back button, nor Ionic 3 when you use it to navigate back after having forward navigated to the same/cached Page in the stack.

So for me, the new query parameter value is present in the URL and it appears that the router is trying to set the observable values but the subscriptions aren't firing.

@daem0ndev

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2018

I can confirm this issue as this was quite a blocker for us, the problem is that the OutletInjector class inside ion-router-outlet only provides the correct ActivatedRoute during initial component initialization, but this is not the correct ActivatedRoute when re-activating a component thats already in the ion-router-outlet stack. The fact of the matter is, there is no easy way to correct this issue from Ionic's side because you cant change the injected activated route after the fact. In other words, the activated route thats injected on a component is only correct if the component is being created by the factory, NOT if we are reactivating a component thats already created, and navigating BACK to that component is actually a whole new navigation and therefor new ActivatedRoute object.

I implemented a very crude workaround to this issue because we have a tight deadline to release our app. The main gist of the solution is that I created a service that always maintains the current activated route as well as the component that is activated and exposes this via an observable, and then on my page components I use this service to get the latest (and correct) instance of ActivatedRoute as it changes over time (i.e. when navigating BACK to a component on the stack) and then you can correctly switchMap over to the queryParamMap on that new ActivatedRoute.

@biesbjerg

This comment has been minimized.

Copy link

commented Dec 20, 2018

@daem0ndev can you share your workaround service?

@daem0ndev

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

@biesbjerg for sure, do you have a repo with this issue that I can simply push a branch to?

@KevinKelchen

This comment has been minimized.

Copy link

commented Jan 2, 2019

Are there any thoughts from the team on when this might be fixed? Prior to Final? Seems pretty important for using the correct navigation parameters if the recommendation and default is to use cached Pages.

@biesbjerg

This comment has been minimized.

Copy link

commented Jan 4, 2019

@daem0ndev thanks, and sorry for getting back to you so late!

I ended up using this workaround: #15959 (comment)

@leopangchan

This comment has been minimized.

Copy link

commented Jan 23, 2019

This issue seems to be resolved with the service in #15959 (comment), but I have used a slightly approach to solve it with absolute link, and I'd like to share it with everyone here. This approach is similar to what #15959 (comment) suggested. I'd like to provide more detail to it.

My problem: I have a page which navigates to another page via a path relative to the first page. When I navigate back to the first page, subscription to (ActivatedRoute) route.queryParams or route.params didn't trigger.

My workaround: I navigate to the second page via (NavController) navCtrl.navigateRoot(), so that the first page is always destroyed in the stack. When I visit it back from the second page, ngOnInit and the subscription inside it are always triggered.

However, I do hope that the Ionic team can resolve this as soon as possible.

@mhartington

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Hey everyone. We just merged in #17888 which fixes this. Thanks for all your patience here 😄

@liamdebeasi liamdebeasi added this to Backlog 🤖 in Ionic Core via automation Mar 29, 2019

@liamdebeasi liamdebeasi moved this from Backlog 🤖 to Done 🎉 in Ionic Core Mar 29, 2019

@ionitron-bot

This comment has been minimized.

Copy link

commented Apr 28, 2019

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 28, 2019

Kiku-git added a commit to Kiku-git/ionic that referenced this issue May 16, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.