Change callback context to deprecate test module properties #136
Conversation
| for accessing properties that exist on the module. | ||
| */ | ||
| _buildDeprecatedContext: function(module, context) { | ||
| var deprecatedContext = Object.create(context); |
There was a problem hiding this comment.
I would prefer to avoid the Object.create(context) call when there are no keysForDeprecation, can you tweak this to be behind a guard?
Something like this slight reordering:
var keysForDeprecation = Object.keys(module);
if (keysForDeprecation.length === 0) {
return this;
}
var deprecatedContext = Object.create(context);
// .... other stuff
There was a problem hiding this comment.
Wait, sorry, I misread. This is only being called when needed (because _contextualizeCallback is called only when needed), but we can still cache the deprecated context (instead of building a new one for every function that is in options).
There was a problem hiding this comment.
Good point, will adjust this to cache the deprecatedContext
|
This looks good, only one thing to tweak (caching the |
| this.foo = 'bar'; | ||
|
|
||
| equal(this.getFoo(), 'bar'); | ||
| ok(!deprecations.length); |
There was a problem hiding this comment.
I am unsure why, but this assertion is failing on ember-1.13. I am rerunning in Travis in case it is a fluke.
There was a problem hiding this comment.
It's possible that 1.13 is throwing a deprecation unrelated to the changes here. Will investigate this before updating again.
trentmwillis
force-pushed
the
callback-context
branch
from
6db0570
to
4a2b736
Compare
|
@rwjblue updated to cache the deprecated context across callbacks and fixed the failure with 1.13. Turns out |
Change callback context to deprecate test module properties
|
Thanks @trentmwillis! |
Addresses #133.
One note: in the case where a user-defined context property collides with a property on the test module, nothing special is done since the user-defined value isn't added to the context until after binding the callbacks.