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.

Sign up for GitHub

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

Jump to bottom

Assert #923

Merged
merged 4 commits into from
Jun 6, 2012
Merged

Assert #923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6d29a7
Guard against undefined mixins
joliss May 31, 2012
b6c1c2f
Use Ember.assert instead of throw
joliss May 31, 2012
d2edc1d
Guard against implicit function application by Ember.assert
joliss May 31, 2012
18ce711
Guard mergeMixins parameters more generally
joliss Jun 2, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/ember-metal/lib/mixin.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function mergeMixins(mixins, m, descs, values, base) {
for(idx=0;idx<len;idx++) { for(idx=0;idx<len;idx++) {


mixin = mixins[idx]; mixin = mixins[idx];
if (!mixin) throw new Error('Null value found in Ember.mixin()'); Ember.assert('Expected hash or Mixin instance, got ' + Object.prototype.toString.call(mixin), typeof mixin === 'object' && mixin !== null && Object.prototype.toString.call(mixin) !== '[object Array]');
Copy link
Member

Choose a reason for hiding this comment

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

One difference between this and the previous code is that the Ember.assert will get stripped in production but the !mixin check won't be. Do you think it matters for us to have a check in production?



if (mixin instanceof Mixin) { if (mixin instanceof Mixin) {
guid = Ember.guidFor(mixin); guid = Ember.guidFor(mixin);
Expand Down Expand Up @@ -224,7 +224,7 @@ function applyMixin(obj, mixins, partial) {


if (desc === REQUIRED) { if (desc === REQUIRED) {
if (!(key in obj)) { if (!(key in obj)) {
if (!partial) throw new Error('Required property not defined: '+key); Ember.assert('Required property not defined: '+key, !!partial);
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.



// for partial applies add to hash of required keys // for partial applies add to hash of required keys
req = writableReq(obj); req = writableReq(obj);
Expand Down Expand Up @@ -311,7 +311,8 @@ function applyMixin(obj, mixins, partial) {
if (META_SKIP[key]) continue; if (META_SKIP[key]) continue;
keys.push(key); keys.push(key);
} }
throw new Error('Required properties not defined: '+keys.join(',')); // TODO: Remove surrounding if clause from production build
Ember.assert('Required properties not defined: '+keys.join(','));
Copy link
Member

Choose a reason for hiding this comment

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

And again.

} }
return obj; return obj;
} }
Expand Down Expand Up @@ -387,6 +388,8 @@ Mixin.prototype.reopen = function() {


for(idx=0;idx<len;idx++) { for(idx=0;idx<len;idx++) {
mixin = arguments[idx]; mixin = arguments[idx];
Ember.assert('Expected hash or Mixin instance, got ' + Object.prototype.toString.call(mixin), typeof mixin === 'object' && mixin !== null && Object.prototype.toString.call(mixin) !== '[object Array]');

if (mixin instanceof Mixin) { if (mixin instanceof Mixin) {
mixins.push(mixin); mixins.push(mixin);
} else { } else {
Expand Down
4 changes: 2 additions & 2 deletions packages/ember-states/lib/state_manager.js
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ Ember.StateManager = Ember.State.extend(


if (initialState) { if (initialState) {
this.transitionTo(initialState); this.transitionTo(initialState);
Ember.assert('Failed to transition to initial state "' + initialState + '"', get(this, 'currentState')); Ember.assert('Failed to transition to initial state "' + initialState + '"', !!get(this, 'currentState'));
} }
}, },


Expand All @@ -414,7 +414,7 @@ Ember.StateManager = Ember.State.extend(
errorOnUnhandledEvent: true, errorOnUnhandledEvent: true,


send: function(event, context) { send: function(event, context) {
Ember.assert('Cannot send event "' + event + '" while currentState is ' + get(this, 'currentState'), get(this, 'currentState')); Ember.assert('Cannot send event "' + event + '" while currentState is null or undefined.', !!get(this, 'currentState'));
this.sendRecursively(event, get(this, 'currentState'), context); this.sendRecursively(event, get(this, 'currentState'), context);
}, },


Expand Down