Skip to content

[BUGFIX lts] Update ApplicationInstance#visit to use followRedirects() - #20574

Merged
kategengler merged 3 commits into
emberjs:mainfrom
davidtaylorhq:follow-redirects
Mar 17, 2026
Merged

[BUGFIX lts] Update ApplicationInstance#visit to use followRedirects()#20574
kategengler merged 3 commits into
emberjs:mainfrom
davidtaylorhq:follow-redirects

Conversation

@davidtaylorhq

Copy link
Copy Markdown
Contributor

The current implementation of Transition#followRedirects() is identical to the logic here: it catches rejections, and then checks for any other activeTransition. Therefore, this commit introduces no change in behavior.

But, if/when emberjs/router.js#335 lands, it will means we can take advantage of that fix for ApplicationInstance#visit.

@wagenet

wagenet commented Aug 2, 2024

Copy link
Copy Markdown
Member

The router.js fix has finally landed. I've pushed the version update to this branch as well.

@wagenet wagenet added the bug label Aug 2, 2024
@wycats

wycats commented Sep 5, 2025

Copy link
Copy Markdown
Member

Is anything blocking this?

@kategengler

Copy link
Copy Markdown
Member

Tests were failing and it has not been updated since

@MrChocolatine

Copy link
Copy Markdown
Contributor

Would it be possible to move forward with this PR please?

@davidtaylorhq @wagenet

Thank you.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

@MrChocolatine we're mid pulling router_js in to the repo: #21211 which would include any changes to router_js, and then this PR becomes very tiny!! 🎉

@MrChocolatine

Copy link
Copy Markdown
Contributor

@NullVoxPopuli , that's great. Once #21211 is merged, do you know for which version it will be available? Eventually backported to previous 6.x versions?

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

if it merged nowish, it'd be in 6.12 LTS, but if it merges after 6.12 is out, it'd go in to 7 -- personally, I'd like to even delay the 6.12 release so we can get more bugfixes in -- the community has blessed us with so many bugfixes the last few weeks <3

@MrChocolatine

Copy link
Copy Markdown
Contributor

I do agree, I think bug fixes are more important.

@kategengler

Copy link
Copy Markdown
Member

If it merged now, it would be in 7.0

We're not delaying releases. The the point of a release train is that there are always more releases. Anything important enough will be backported.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

gotchya -- I feel like I need a app/dashboard for our release train 🙈 haha

@MrChocolatine

Copy link
Copy Markdown
Contributor

In v7, oh okay.

emberjs/ember-test-helpers#332 exists since 2018, and this fix (this PR) since 2023. It would have been great to make it available for all the versions since 2023...

@kategengler

Copy link
Copy Markdown
Member

@davidtaylorhq Any interest in rebasing? If we merge before the router is inlined (asap), this is more easily backported.

davidtaylorhq and others added 2 commits March 17, 2026 13:45
The current implementation of `Transition#followRedirects()` is identical to the logic here: it catches rejections, and then checks for any other `activeTransition`. Therefore, this commit introduces no change in behavior.

But, if/when emberjs/router.js#335 lands, it will means we can take advantage of that fix for ApplicationInstance#visit.
@kategengler kategengler changed the title Update ApplicationInstance#visit to use followRedirects() [BUGFIX lts] Update ApplicationInstance#visit to use followRedirects() Mar 17, 2026
@kategengler
kategengler merged commit 3fb96dd into emberjs:main Mar 17, 2026
36 checks passed
@MrChocolatine

Copy link
Copy Markdown
Contributor

Yey, thanks everyone.

@kategengler

Copy link
Copy Markdown
Member

@MrChocolatine If you have a chance, can you try canary and validate that it fixes that test helpers bug for you?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants