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

add _matchedRouteName to context #337

Merged
merged 1 commit into from
May 22, 2017
Merged

add _matchedRouteName to context #337

merged 1 commit into from
May 22, 2017

Conversation

netroy
Copy link
Contributor

@netroy netroy commented Mar 15, 2017

sometimes route paths can be very long & convoluted (legacy apis), exposing the layer
name provides a saner alternative for analytics

this is an extension to #236

/cc @alexmingoia @jergason

sometimes route paths can be very long & convoluted, exposing the route
name provides a saner alternative analytics
@jergason
Copy link
Collaborator

Is there a reason to make this backwards-incompatible? Why not have both the route and the name?

@netroy
Copy link
Contributor Author

netroy commented Mar 16, 2017

@jergason it is backwards-compatible.. the _matchedRoute property is still there

@jergason
Copy link
Collaborator

Ah crud, I read it too quickly. Thanks for correcting me.

@netroy
Copy link
Contributor Author

netroy commented Apr 4, 2017

/cc @jbielick

@jbielick jbielick self-assigned this Apr 5, 2017
@Mousius
Copy link

Mousius commented Apr 20, 2017

I was looking for something along these lines to identify the route for stats purposes. In #341 it was suggested to use _matchedRoute but that's not quite as clean for a stats counter. If these things are features they should be documented and not considered private. Maybe adding ctx.routePath and ctx.routeName as well as the backwards compatible ctx._matchedRoute?

@jbielick jbielick merged commit ef4f275 into ZijianHe:master May 22, 2017
jbielick added a commit that referenced this pull request May 23, 2017
@netroy netroy deleted the context-route-name branch May 23, 2017 13:47
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.

4 participants