[FEATURE ember-improved-instrumentation] Rebase + Tests #13017
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
Tests seem broken |
|
Yea, there is some weird stuff around route transitions where certain events are fired in some cases and not others. |
chadhietala
force-pushed
the
instrument
branch
2 times, most recently
from
1f0bc7d to
2ba1336
Compare
chadhietala
force-pushed
the
instrument
branch
from
2ba1336 to
364fc53
Compare
chadhietala
force-pushed
the
instrument
branch
from
364fc53 to
c89f137
Compare
|
@chancancode You ok merging this for the time being and then come back to router instrumentation? Can squash if everything else looks good. |
|
☔ The latest upstream changes (presumably #13100) made this pull request unmergeable. Please resolve the merge conflicts. |
This was referenced Apr 10, 2016
|
@chadhietala - We will definitely need to fix the failing tests before merging (I haven't looked into what exactly is wrong yet). |
|
Also, does this need to live on or is it replaced by #13279? |
|
Yeah the comments left on #13279 would also apply to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebase and Adds Test For #12550. However this just introduces action instrumentation. There are still issues around instrumenting the router. I have saved off the diff for the time being. https://gist.github.com/chadhietala/2b3065f3da3d8ae6e2ff