Skip to content

fix: qp-only transition during initial transition#307

Open
buschtoens wants to merge 2 commits into
emberjs:masterfrom
buschtoens:patch-2
Open

fix: qp-only transition during initial transition#307
buschtoens wants to merge 2 commits into
emberjs:masterfrom
buschtoens:patch-2

Conversation

@buschtoens

Copy link
Copy Markdown

Seems to fix emberjs/ember.js#18577.

image

@jelhan

jelhan commented Oct 29, 2020

Copy link
Copy Markdown

Please see my comment here: emberjs/ember.js#18577 (comment) Not sure if this is another bug or if this fix does not address the root cause.

Comment thread lib/router/router.ts
// method (default is replaceState).
let newTransition = new InternalTransition(this, undefined, undefined);
let newTransition = new InternalTransition(this, undefined, newState);
newTransition.queryParamsOnly = true;

@buschtoens buschtoens Oct 30, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
newTransition.queryParamsOnly = true;
newTransition.queryParamsOnly = true;
this.setupContexts(newState, newTransition);

Some other tests started breaking with the undefined -> newState change in L123, because this apparently leaks through Routes which haven't yet been passed through setupContexts(...) and thus did not have their setup(...) hook called.

Ultimately this causes Route#finalizeQueryParamChange to fail, as it expects route.controller to be defined.

@rwjblue

rwjblue commented Nov 6, 2020

Copy link
Copy Markdown
Member

This fix looks good conceptually, but we need to fixup the tests and add a specific test for the scenario described in emberjs/ember.js#18577

@wagenet

wagenet commented Feb 9, 2022

Copy link
Copy Markdown
Member

@buschtoens status?

@Mikek2252

Copy link
Copy Markdown

@buschtoens @rwjblue Is this solution still good? if so is there anything i can do to help get this merged? Happy to write the test for it if i can be pushed in the correct general direction, im not overly familiar with this code.

@vstefanovic97

Copy link
Copy Markdown

Hey, I'm still seeing this problem in 2025, anything I can do to help?

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

Successfully merging this pull request may close these issues.

Redirecting a transition to same route with different query params does not work unless transition is aborted before

6 participants