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

Redirecting during prefetch undermines Ember routing behavior #101

Open
nickiaconis opened this issue Feb 13, 2018 · 5 comments
Open

Redirecting during prefetch undermines Ember routing behavior #101

nickiaconis opened this issue Feb 13, 2018 · 5 comments

Comments

@nickiaconis
Copy link
Owner

nickiaconis commented Feb 13, 2018

What's the issue?

The downstream case works fine—the prefetch hooks of routes farther from the root than a route that starts a new transition during its prefetch hook will not be called because the current transition enters the aborted state. There is no desire to preserve the model of downstream routes because they are never encountered.

Conversely, the upstream case should, but does not, account for preservation of the model. Ember's routing system establishes that redirection in a downstream route does not affect the resolution of upstream routes' models. However, when an app starts a new transition from within a prefetch hook, it invalidates all of the upstream routes' models. This causes unnecessary API requests as the app churns through the upstream routes' prefetch hooks multiple times.

I propose we disallow—or at the very least discourage—redirects during the prefetch hook.

What are some arguments against this?

Arguments that support creating new transitions during the prefetch hook likely stem from the desire to fail early. This is a commendable goal, but there are other ways to achieve early failure, such as moving the redirect logic into the route closest to the root where information is still sufficiently available to make the call whether to redirect or not. The redirect logic can then make use of the traditional model/redirect hooks, ensuring it does not invalidate the models of upstream routes, and giving upstream routes the opportunity to use the traditional hooks as well.

How do we teach this?

This will need to be documented in the README.

Ideally, we can prevent this behavior by calling the prefetch hook with a modified context that blocks access to the transition API or throws an error when an app attempts to use it. This will require a major version increment. Leading up to this, a deprecation can be added that warns of this breaking change and encourages application developers to disuse the transition API in the prefetch hook.

@nickiaconis
Copy link
Owner Author

@stefanpenner @rwjblue I'd like to get your opinions on this

@nickiaconis nickiaconis changed the title Transitioning during prefetch undermines Ember routing behavior Redirecting during prefetch undermines Ember routing behavior Feb 13, 2018
@stefanpenner
Copy link
Collaborator

@nickiaconis its not super clear to me that we should allow/encourage transitions during prefetch. That seems suspect to me. Could you share some use-cases/motivations that require this?

@nickiaconis
Copy link
Owner Author

My intention is actually the exact opposite: to disallow/discourage transitions during prefetch. Sorry for the confusion @stefanpenner. Maybe including some specific examples in the description would have helped drive that fact home.

@stefanpenner
Copy link
Collaborator

My intention is actually the exact opposite: to disallow/discourage transitions during prefetch.

As you can likely tell by my above comment, this direction sounds very reasonable to me.

Would also like to hear @rwjblue's thoughts.

@chriskrycho
Copy link

If anyone else happens to run into this: in at least some cases, prefetch hook invocations which include transitionTo without aborting the current transition will fail on Ember 3.6+ (i.e. with the updated, public RouterService). The failure mode is that the transitions simply never resolve and the app hangs.

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

No branches or pull requests

3 participants