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

[Bug] <RouterService>#transitionTo incorrectly re-runs the the model hook when query params do not change #19497

Open
NullVoxPopuli opened this issue Apr 16, 2021 · 13 comments

Comments

@NullVoxPopuli
Copy link
Sponsor Contributor

🐞 Describe the Bug

  • have queryParams
  • configure the query params to refeshModel: true
  • navigate to a child route with the router service's transitionTo
  • notice that the parent route's model hook re-runs

🔬 Minimal Reproduction

Twiddle

😕 Actual Behavior

the parent model hook is re-running (in the application route in the repro)

🤔 Expected Behavior

the parent model hook should not run (as with the transitionToRoute behavior)

🌍 Environment

  • Ember: - Octane+
  • Node.js/npm: - n/a
  • OS: - n/a
  • Browser: - n/a
@trevordevore
Copy link

trevordevore commented Apr 17, 2021

I came across a behavior which may be related to this bug. The context is as follows:

parent route
-- child route
---- component

The parent route's controller has a queryParam. The component needs to update that queryParam and uses this.router.transitionTo({ queryParams: { prop: value }}); to do so. The queryParam prop is being modified, which is different than this bug report, but the child route model hook is still being triggered.

To work around the issue I now call this.transitionToRoute({ queryParams: { prop: value }}) on the child route's controller. The model hook is no longer run again in this case.

@btecu
Copy link
Contributor

btecu commented Apr 26, 2021

I'm running into this issue as well. Seems to be happening when migrating to the new class syntax.

@rtablada
Copy link
Contributor

rtablada commented Jun 4, 2021

We actually had this come up. I accidentally use this as a behavior to refresh a route without realizing this was going on until I called it twice... 🤦🏽

This is documented in the transitionTo behavior but would be nice to not have default queryParam values pop up.

@ro0gr
Copy link
Contributor

ro0gr commented Jun 5, 2021

This is one of the aged issues #16349

@panthony
Copy link

panthony commented Dec 16, 2021

I think I came across the same issue while migrating to ember 3.24 with native class syntax:

parent route
-- child route
----- component

The component was using the router service to update query params of the child route:

this.router.transitionTo({ queryParams: {} })

Or even simply the route:

this.router.transitionTo(newRoute)

But the parent route model's hook was re-run.

Delegating the update to the child route's controller using transitionToRoute solved this issue.

@IgnaceMaes
Copy link

IgnaceMaes commented Jul 8, 2022

We're also running into this issue when refactoring the deprecated this.transitionToRoute call in Controllers to this.router.transitionTo.

As this issue causes a lot of extra unnecessary model calls and slows down our app it's a critical issue.

A temporary workaround seems to be returning the previous model state like so:

model(params, transition) {
  super.model(...arguments);

  // Reuse the previous model state when redirecting to, or redirected back from a child route
  if (isRedirectFromOrToChildRoute(transition)) {
    return this.modelFor(this.routeName);
  }

  // Redacted ...
}

// With isRedirectFromOrToChildRoute e.g.
isRedirectFromOrToChildRoute(transition) {
  const routeNameFromStripped = transition.from?.name?.replace('.index', ''); // Can be undefined when it's the first page accessed
  if (routeNameFromStripped) {
    const routeNameToStripped = transition.to.name.replace('.index', '');
    const isChildRoute = routeNameFromStripped.startsWith(routeNameToStripped);
    const isParentRoute = routeNameToStripped.startsWith(routeNameFromStripped);
    const isSameRoute = routeNameFromStripped === routeNameToStripped;

    return !isSameRoute && (isChildRoute || isParentRoute);
  }

  return false;
}

But as it's such a core functionality of routing this seems like a hacky approach ...

@abueloshika
Copy link

I'm experiencing the same issue on ember 4.6.0

I've confirmed that if I remove the refreshModel: true from the queryParam the parent hook doesn't fire on transition.

@Techn1x
Copy link

Techn1x commented Dec 21, 2022

Also experiencing this issue on Ember 3.28.11, Ember 4.4.4 and Ember 4.9.2

Having to stick with Controller.transitionToRoute for certain calls to avoid slow page transitions

@Techn1x
Copy link

Techn1x commented Nov 16, 2023

I have confirmed the router.transitionTo problem still exists with Ember 4.12

This is now actually a problem as transitionToRoute is removed in ember 5, and I would like to update..

@nickschot
Copy link
Contributor

nickschot commented Feb 29, 2024

This seems like it may be related to #16349, #20038 and #20512 as well and indeed still occurs in ember 5.

@panthony
Copy link

panthony commented May 29, 2024

In the process of moving toward ember 5 (at least trying), the fact that transitionToRoute is removed is a blocker as it was my unique workaround for that bug.

The fix suggested by IgnaceMaes do not take around for redirection between siblings route (parent.childa -> parent.childb) that produces the bug too.

I'm also afraid of skipping a required model update 🤷🏻

@kategengler
Copy link
Member

There are unfortunate differences between transitionTo on the router service and what route#transitionTo or controller#transitionToRoute did.

As a workaround, I suggest getting to that original transitionTo either through the route or by using getOwner(this).lookup('router:main').transitionTo(... ... technically private but works.

@panthony
Copy link

panthony commented Jun 13, 2024

@kategengler Thanks for your suggestion! I went with the following workaround:

  RouterService.reopen({
    legacyTransitionTo: function () {
      return getOwner(this)
        .lookup('router:main')
        .transitionTo(...arguments);
    },

This way I can use the router service where needed with legacyTransitionTo() like I would with transitionTo:

export default class MyComponent extends Component {
  @service router;

  @action 
   doX() {
       this.router.legacyTransitionTo(/* ... */);
   }
}

Once that bug is resolved I'll just have to swap & replace legacyTransitionTo by transitionTo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests