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

[Breaking] RouteInfo Type And Transition.(from|to) #252

Merged
merged 9 commits into from
Sep 19, 2018

Conversation

chadhietala
Copy link
Collaborator

This introduces the public RouteInfo type and from and to fields on the Transition as described in emberjs/rfcs#95.

This PR also:

  • [Breaking] removes all underscore methods from the Route type.
  • [Breaking] Renames HandlerInfo -> RouteInfo in many places.

@chadhietala chadhietala force-pushed the route-info-type branch 2 times, most recently from 8131acb to 05f8ab4 Compare September 18, 2018 15:45
Copy link
Collaborator

@ef4 ef4 left a comment

Choose a reason for hiding this comment

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

It's a big diff and I can't pretend to have followed every bit of it, but it all seems reasonable on first read.

@rwjblue
Copy link
Collaborator

rwjblue commented Sep 19, 2018

How do you plan to mitigate the breakage on the Ember side when we land this there?

@chadhietala
Copy link
Collaborator Author

chadhietala commented Sep 19, 2018

@rwjblue all of this should absorbed by Ember. With the private infos the only thing that might likely troll users is that I renamed the handle property to route, but other than that the structure should be the same. This is a private API, I can defineProperty and delegate if need be.

@chadhietala chadhietala merged commit e853a63 into master Sep 19, 2018
@chadhietala chadhietala deleted the route-info-type branch September 19, 2018 19:40
@ef4
Copy link
Collaborator

ef4 commented Sep 20, 2018

This will likely break liquid-outlet. But that’s fine, the whole point was creating public API to replace the private one it’s using

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.

None yet

3 participants