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

transitionTo returns promise when passed a url #12899

Closed
asakusuma opened this issue Feb 2, 2016 · 12 comments
Closed

transitionTo returns promise when passed a url #12899

asakusuma opened this issue Feb 2, 2016 · 12 comments

Comments

@asakusuma
Copy link
Contributor

In ember canary, route.transitionTo returns a promise when passed a url. According to the docs, it should return a transition object.

From what I can tell, the issue seems to have been introduced in this commit.

I reproduced the issue in this example app.

cc @minasmart @rwjblue

@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2016

Thanks for the report @asakusuma, looks like @GavinJoyce is working on a fix in #12905.

@GavinJoyce
Copy link
Member

I had been digging into the recent changes which reshuffled how the router deals with exceptions but I don't have enough context yet to be productive here.

Sorry, I'm going to close my skeleton PR and find something else to work on.

@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2016

@GavinJoyce - Understood. I'm trying to find a good path forward. I realize this is a fairly bad regression, so I'll try to be quick about it...

@krisselden
Copy link
Contributor

Mobile phone mishap

@krisselden krisselden reopened this Feb 5, 2016
@krisselden
Copy link
Contributor

Was just scrolling somehow registered as a tap on the close button

@stefanpenner
Copy link
Member

Mobile phone mishap

rofl #theWeb

@minasmart
Copy link

@rwjblue & @GavinJoyce the skeleton PR looks good to me. I don't think it breaks exception handling. Did all the tests pass with the change?

@rwjblue
Copy link
Member

rwjblue commented Feb 5, 2016

@minasmart - It breaks most of the tests I added in the original PR.

@minasmart
Copy link

aw 😢

@minasmart
Copy link

I'll spend some time and see if I can maybe come up with some ideas? I feel a little responsible having wrote the code that caused the regression.

@asakusuma
Copy link
Contributor Author

@krisselden briefed me on the issue. I'm going to take a stab at a PR this afternoon and will give an update later today.

@asakusuma
Copy link
Contributor Author

@rwjblue @krisselden @minasmart any objection to making router.transitionTo public? route.transitionTo is public, and it's pretty much just a proxy to router.transitionTo: https://github.com/emberjs/ember.js/blob/master/packages/ember-routing/lib/system/route.js#L1027

Edit: formatting

@rwjblue rwjblue closed this as completed in b1cf8f6 Feb 9, 2016
rwjblue added a commit that referenced this issue Feb 9, 2016
rwjblue pushed a commit that referenced this issue Feb 21, 2016
Ensure transitionTo returns a transition object. Addresses
#12899

(cherry picked from commit b1cf8f6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants