-
Notifications
You must be signed in to change notification settings - Fork 30
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
Throw error when one of the mixins doesn't return options object. #24
Conversation
Hmm, this is making me think that all mixins should have a well-defined Thoughts? |
I'm thinking at the least the error message should include the |
I hear you, but it might take the simplicity with "it's just functions" away.
Yes, that's valid :) I'll fix. At least the trace will point to the location where the |
Nah I think you're right - for now just adding the name of the method will be fine. |
@stubailo Pushed. As you can see in the tests, the error messages are now:
resp.
|
Looks great! |
Throw error when one of the mixins doesn't return options object.
⚡ 😄 |
Fixes #23.
@stubailo I'm throwing inside of the
![](https://camo.githubusercontent.com/f79211df976f17f689afcdad9cf11b6090266511c4c76958d1c3b5c7370ce90d/687474703a2f2f7777772e636f64657265766965776875622e636f6d2f736974652f6769746875622d6261722e706e67)
applyMixins
function instantly if the mixin in theforEach
callback isn't returning a plain object.name
property. Also, it's weird that the stack trace in this error will actually show thevalidated-method
package, so in the worst case you'll get "One of the mixins didn't return an options object" and the trace will be from this package. I guess you will be able to use that to figure out which method it is.Thoughts?
name
of the ValidatedMethod in which the mixin was used, if such exists.I hear you, but it might take the simplicity with "it's just functions" away.
As you can see in the tests, the error messages are now:
resp.