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

Instrument event dispatcher (rebased) #12550

Closed

Conversation

chancancode
Copy link
Member

Rebased version of #12088

@@ -542,6 +553,8 @@ var EmberRouter = EmberObject.extend(Evented, {
emberRouter.didTransition(infos);
};

// TODO
//router.willTransition = this.willTransition.bind(this);
Copy link
Member

Choose a reason for hiding this comment

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

What is this comment about?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, I think this was us saying we should just replace the line below with a bind since that's exactly what it was doing, but maybe we didn't want to change unrelated code in the commit

@chancancode chancancode force-pushed the instrument-event-dispatcher branch 2 times, most recently from 3b52aeb to 5740168 Compare November 3, 2015 20:19
@rwjblue
Copy link
Member

rwjblue commented Nov 4, 2015

Looks like this needs a rebase.

@mixonic
Copy link
Sponsor Member

mixonic commented Nov 5, 2015

Seems inoffensive, but does lack any tests for the action and routing instrumentation.

Godhuda and others added 4 commits November 4, 2015 16:43
This commit adds instrumentation for interaction events handled by
components. The `before` hook of the instrumentation runs right before
the component’s event handler is invoked, and the `after` hook runs
right after it is invoked.
This commit instruments closure actions.
This instruments the start and end of top-level routing transitions. It
does not run for redirects or intermediate transitions.

There are two events: `router.transition.url` (payload contains the URL)
and `router.transition.named` (payload contains the route name).

The current implementation does not properly handle top-level
transitions that never complete at all; this requires a fix to
router.js.
@rwjblue
Copy link
Member

rwjblue commented Apr 10, 2016

@rwjblue rwjblue closed this Apr 10, 2016
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.

3 participants