diff --git a/FEATURES.md b/FEATURES.md index a84d60e5d0a..dc4082c798a 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -45,3 +45,10 @@ for a detailed explanation. ], dedupedComments: Ember.computed.uniqBy('comments', 'id') ``` + +* `ember-improved-instrumentation` + + Adds additional instrumentation to Ember: + + - `interaction.` for events handled by a component. + - `interaction.ember-action` for closure actions. diff --git a/features.json b/features.json index ab63d7069ec..de6cc760119 100644 --- a/features.json +++ b/features.json @@ -9,6 +9,7 @@ "ember-htmlbars-local-lookup": true, "ember-application-engines": null, "ember-glimmer": null, - "ember-runtime-computed-uniq-by": null + "ember-runtime-computed-uniq-by": null, + "ember-improved-instrumentation": null } } diff --git a/packages/ember-metal/lib/instrumentation.js b/packages/ember-metal/lib/instrumentation.js index ca9337686ae..6b045b3bfed 100644 --- a/packages/ember-metal/lib/instrumentation.js +++ b/packages/ember-metal/lib/instrumentation.js @@ -1,4 +1,5 @@ import Ember from 'ember-metal/core'; +import isEnabled from 'ember-metal/features'; /** The purpose of the Ember Instrumentation module is @@ -105,14 +106,26 @@ export function instrument(name, _payload, callback, binding) { } } +var flaggedInstrument; +if (isEnabled('ember-improved-instrumentation')) { + flaggedInstrument = instrument; +} else { + flaggedInstrument = function(name, payload, callback) { + return callback(); + }; +} +export { flaggedInstrument }; + function withFinalizer(callback, finalizer, payload, binding) { + let result; try { - return callback.call(binding); + result = callback.call(binding); } catch(e) { payload.exception = e; - return payload; + result = payload; } finally { - return finalizer(); + finalizer(); + return result; } } @@ -151,7 +164,9 @@ export function _instrumentStart(name, _payload) { var timestamp = time(); for (i = 0, l = listeners.length; i < l; i++) { listener = listeners[i]; - listener.after(name, timestamp, payload, beforeValues[i]); + if (typeof listener.after === 'function') { + listener.after(name, timestamp, payload, beforeValues[i]); + } } if (STRUCTURED_PROFILE) { diff --git a/packages/ember-routing-htmlbars/lib/keywords/closure-action.js b/packages/ember-routing-htmlbars/lib/keywords/closure-action.js index 1cebabdca49..9aa76722bb0 100644 --- a/packages/ember-routing-htmlbars/lib/keywords/closure-action.js +++ b/packages/ember-routing-htmlbars/lib/keywords/closure-action.js @@ -1,13 +1,15 @@ import { Stream } from 'ember-metal/streams/stream'; import { read, - readArray + readArray, + labelFor } from 'ember-metal/streams/utils'; import symbol from 'ember-metal/symbol'; import { get } from 'ember-metal/property_get'; import { labelForSubexpr } from 'ember-htmlbars/hooks/subexpr'; import EmberError from 'ember-metal/error'; import run from 'ember-metal/run_loop'; +import { flaggedInstrument } from 'ember-metal/instrumentation'; export const INVOKE = symbol('INVOKE'); export const ACTION = symbol('ACTION'); @@ -59,7 +61,7 @@ export default function closureAction(morph, env, scope, params, hash, template, valuePath = read(hash.value); } - return createClosureAction(target, action, valuePath, actionArguments); + return createClosureAction(this, target, action, valuePath, actionArguments); }, function() { return labelForSubexpr(params, hash, 'action'); }); @@ -70,7 +72,7 @@ export default function closureAction(morph, env, scope, params, hash, template, return s; } -function createClosureAction(target, action, valuePath, actionArguments) { +function createClosureAction(stream, target, action, valuePath, actionArguments) { var closureAction; if (actionArguments.length > 0) { @@ -83,7 +85,10 @@ function createClosureAction(target, action, valuePath, actionArguments) { args[0] = get(args[0], valuePath); } - return run.join(target, action, ...args); + let payload = { target, args, label: labelFor(stream) }; + return flaggedInstrument('interaction.ember-action', payload, () => { + return run.join(target, action, ...args); + }); }; } else { closureAction = function(...args) { @@ -91,7 +96,10 @@ function createClosureAction(target, action, valuePath, actionArguments) { args[0] = get(args[0], valuePath); } - return run.join(target, action, ...args); + let payload = { target, args, label: labelFor(stream) }; + return flaggedInstrument('interaction.ember-action', payload, () => { + return run.join(target, action, ...args); + }); }; } diff --git a/packages/ember-routing-htmlbars/tests/helpers/closure_action_test.js b/packages/ember-routing-htmlbars/tests/helpers/closure_action_test.js index 19f002ed35e..c400f39beb2 100644 --- a/packages/ember-routing-htmlbars/tests/helpers/closure_action_test.js +++ b/packages/ember-routing-htmlbars/tests/helpers/closure_action_test.js @@ -1,8 +1,14 @@ import run from 'ember-metal/run_loop'; import compile from 'ember-template-compiler/system/compile'; import EmberComponent from 'ember-views/components/component'; +import EmberView from 'ember-views/views/view'; import { computed } from 'ember-metal/computed'; import { INVOKE } from 'ember-routing-htmlbars/keywords/closure-action'; +import { subscribe, unsubscribe } from 'ember-metal/instrumentation'; +import buildOwner from 'container/tests/test-helpers/build-owner'; +import { OWNER } from 'container/owner'; +import ComponentLookup from 'ember-views/component_lookup'; +import EventDispatcher from 'ember-views/system/event_dispatcher'; import { runAppend, @@ -11,25 +17,149 @@ import { import { registerKeyword, resetKeyword } from 'ember-htmlbars/tests/utils'; import viewKeyword from 'ember-htmlbars/keywords/view'; +import isEnabled from 'ember-metal/features'; -var innerComponent, outerComponent, originalViewKeyword; +var innerComponent, outerComponent, originalViewKeyword, owner, view, dispatcher; + +function buildResolver() { + let resolver = { + resolve() { }, + expandLocalLookup(fullName, sourceFullName) { + let [sourceType, sourceName ] = sourceFullName.split(':'); + let [type, name ] = fullName.split(':'); + + if (type !== 'template' && sourceType === 'template' && sourceName.slice(0, 11) === 'components/') { + sourceName = sourceName.slice(11); + } + + if (type === 'template' && sourceType === 'template' && name.slice(0, 11) === 'components/') { + name = name.slice(11); + } + + + let result = `${type}:${sourceName}/${name}`; + + return result; + } + }; + + return resolver; +} + +function registerTemplate(moduleName, snippet) { + owner.register(`template:${moduleName}`, compile(snippet, { moduleName })); +} + +function registerComponent(name, factory) { + owner.register(`component:${name}`, factory); +} + +function appendViewFor(template, moduleName='', hash={}) { + let view = EmberView.extend({ + template: compile(template, { moduleName }), + [OWNER]: owner + }).create(hash); + + runAppend(view); + + return view; +} -import isEnabled from 'ember-metal/features'; if (!isEnabled('ember-glimmer')) { // jscs:disable +let subscriber; QUnit.module('ember-routing-htmlbars: action helper', { setup() { originalViewKeyword = registerKeyword('view', viewKeyword); + owner = buildOwner({ + _registryOptions: { + resolver: buildResolver() + } + }); + owner.registerOptionsForType('component', { singleton: false }); + owner.registerOptionsForType('view', { singleton: false }); + owner.registerOptionsForType('template', { instantiate: false }); + owner.register('component-lookup:main', ComponentLookup); + dispatcher = EventDispatcher.create(); + dispatcher.setup(); }, teardown() { runDestroy(innerComponent); runDestroy(outerComponent); + runDestroy(view); + runDestroy(owner); resetKeyword('view', originalViewKeyword); + if (subscriber) { + unsubscribe(subscriber); + } + owner = view = null; + runDestroy(dispatcher); } }); +if (isEnabled('ember-improved-instrumentation')) { + QUnit.test('action should fire interaction event', function(assert) { + assert.expect(2); + + subscriber = subscribe('interaction.ember-action', { + before() { + assert.ok(true, 'instrumentation subscriber was called'); + } + }); + + registerTemplate('components/inner-component', ''); + registerComponent('inner-component', EmberComponent.extend({ + actions: { + fireAction() { + this.attrs.submit(); + } + } + })); + + registerTemplate('components/outer-component', '{{inner-component submit=(action outerSubmit)}}'); + registerComponent('outer-component', EmberComponent.extend({ + innerComponent, + outerSubmit() { + assert.ok(true, 'action is called'); + } + })); + + view = appendViewFor(`{{outer-component}}`); + + view.$('#instrument-button').trigger('click'); + }); + + QUnit.test('instrumented action should return value', function(assert) { + assert.expect(1); + + var returnedValue = 'Chris P is so krispy'; + + registerTemplate('components/inner-component', ''); + registerComponent('inner-component', EmberComponent.extend({ + actions: { + fireAction() { + var actualReturnedValue = this.attrs.submit(); + assert.equal(actualReturnedValue, returnedValue, 'action can return to caller'); + } + } + })); + + registerTemplate('components/outer-component', '{{inner-component submit=(action outerSubmit)}}'); + registerComponent('outer-component', EmberComponent.extend({ + innerComponent, + outerSubmit() { + return returnedValue; + } + })); + + view = appendViewFor(`{{outer-component}}`); + + view.$('#instrument-button').trigger('click'); + }); +} + QUnit.test('action should be called', function(assert) { assert.expect(1); diff --git a/packages/ember-views/lib/views/states/has_element.js b/packages/ember-views/lib/views/states/has_element.js index d1cda52a926..f2eb7e9ea8b 100644 --- a/packages/ember-views/lib/views/states/has_element.js +++ b/packages/ember-views/lib/views/states/has_element.js @@ -2,6 +2,7 @@ import _default from 'ember-views/views/states/default'; import assign from 'ember-metal/assign'; import jQuery from 'ember-views/system/jquery'; import run from 'ember-metal/run_loop'; +import { flaggedInstrument } from 'ember-metal/instrumentation'; /** @module ember @@ -46,11 +47,13 @@ assign(hasElement, { }, // Handle events from `Ember.EventDispatcher` - handleEvent(view, eventName, evt) { + handleEvent(view, eventName, event) { if (view.has(eventName)) { // Handler should be able to re-dispatch events, so we don't // preventDefault or stopPropagation. - return run.join(view, view.trigger, eventName, evt); + return flaggedInstrument(`interaction.${eventName}`, { event, view }, () => { + return run.join(view, view.trigger, eventName, event); + }); } else { return true; // continue event propagation } diff --git a/packages/ember-views/tests/system/event_dispatcher_test.js b/packages/ember-views/tests/system/event_dispatcher_test.js index 314e5bfad68..4167efe9f32 100644 --- a/packages/ember-views/tests/system/event_dispatcher_test.js +++ b/packages/ember-views/tests/system/event_dispatcher_test.js @@ -16,10 +16,13 @@ import { runAppend, runDestroy } from 'ember-runtime/tests/utils'; import { registerKeyword, resetKeyword } from 'ember-htmlbars/tests/utils'; import viewKeyword from 'ember-htmlbars/keywords/view'; +import { subscribe, unsubscribe } from 'ember-metal/instrumentation'; + var owner, view, originalViewKeyword; var dispatcher; import isEnabled from 'ember-metal/features'; + if (!isEnabled('ember-glimmer')) { // jscs:disable @@ -47,6 +50,59 @@ QUnit.module('EventDispatcher', { } }); +if (isEnabled('ember-improved-instrumentation')) { + QUnit.test('should instrument triggered events', function() { + let clicked = 0; + + run(function () { + view = View.create({ + click(evt) { + clicked++; + }, + + template: compile('

hello

') + }).appendTo(dispatcher.get('rootElement')); + }); + + view.$().trigger('click'); + + equal(clicked, 1, 'precond - The click handler was invoked'); + + let clickInstrumented = 0; + let clickSubscriber = subscribe('interaction.click', { + before() { + clickInstrumented++; + equal(clicked, 1, 'invoked before event is handled'); + }, + after() { + clickInstrumented++; + equal(clicked, 2, 'invoked after event is handled'); + } + }); + + let keypressInstrumented = 0; + let keypressSubscriber = subscribe('interaction.keypress', { + before() { + keypressInstrumented++; + }, + after() { + keypressInstrumented++; + } + }); + + try { + view.$().trigger('click'); + view.$().trigger('change'); + equal(clicked, 2, 'precond - The click handler was invoked'); + equal(clickInstrumented, 2, 'The click was instrumented'); + strictEqual(keypressInstrumented, 0, 'The keypress was not instrumented'); + } finally { + unsubscribe(clickSubscriber); + unsubscribe(keypressSubscriber); + } + }); +} + QUnit.test('should dispatch events to views', function() { var receivedEvent; var parentMouseDownCalled = 0; diff --git a/packages/ember/tests/view_instrumentation_test.js b/packages/ember/tests/view_instrumentation_test.js index 124ac230b9e..48d3b96f749 100644 --- a/packages/ember/tests/view_instrumentation_test.js +++ b/packages/ember/tests/view_instrumentation_test.js @@ -27,6 +27,7 @@ import isEnabled from 'ember-metal/features'; if (!isEnabled('ember-glimmer')) { // jscs:disable +let subscriber; QUnit.module('View Instrumentation', { setup() { run(function() { @@ -45,6 +46,9 @@ QUnit.module('View Instrumentation', { }, teardown() { + if (subscriber) { + unsubscribe(subscriber); + } run(App, 'destroy'); App = null; Ember.TEMPLATES = {}; @@ -53,7 +57,7 @@ QUnit.module('View Instrumentation', { QUnit.test('Nodes without view instances are instrumented', function(assert) { var called = false; - var subscriber = subscribe('render', { + subscriber = subscribe('render', { before() { called = true; }, @@ -64,7 +68,6 @@ QUnit.test('Nodes without view instances are instrumented', function(assert) { called = false; handleURL('/posts'); assert.ok(called, 'instrumentation called on transition to non-view backed route'); - unsubscribe(subscriber); }); }