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.

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

Merged
merged 3 commits into from
Jan 12, 2016

Conversation

johanbrook
Copy link
Contributor

Fixes #23.

@stubailo I'm throwing inside of the applyMixins function instantly if the mixin in the forEach callback isn't returning a plain object.

Error in methodWithFaultySchemaMixin method: The function 'nonReturningFunction' didn't return the options object

resp.

Error in methodWithFaultySchemaMixin method: One of the mixins didn't return the options object

  

@stubailo
Copy link
Contributor

Hmm, this is making me think that all mixins should have a well-defined name property. Also, it's weird that the stack trace in this error will actually show the validated-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?

@stubailo
Copy link
Contributor

I'm thinking at the least the error message should include the name of the ValidatedMethod in which the mixin was used, if such exists.

@johanbrook
Copy link
Contributor Author

Hmm, this is making me think that all mixins should have a well-defined name property.

I hear you, but it might take the simplicity with "it's just functions" away.

I'm thinking at the least the error message should include the name of the ValidatedMethod in which the mixin was used, if such exists.

Yes, that's valid :) I'll fix.

At least the trace will point to the location where the ValidatedMethod was instantiated. And it might perhaps be possible to pass the mixin in question as a parameter to the Error thrown, as an extra data item?

@stubailo
Copy link
Contributor

Nah I think you're right - for now just adding the name of the method will be fine.

@johanbrook
Copy link
Contributor Author

@stubailo Pushed.

As you can see in the tests, the error messages are now:

Error in methodWithFaultySchemaMixin method: The function 'nonReturningFunction' didn't return the options object

resp.

Error in methodWithFaultySchemaMixin method: One of the mixins didn't return the options object

@stubailo
Copy link
Contributor

Looks great!

stubailo pushed a commit that referenced this pull request Jan 12, 2016
Throw error when one of the mixins doesn't return options object.
@stubailo stubailo merged commit 7e61f38 into meteor:master Jan 12, 2016
@johanbrook
Copy link
Contributor Author

⚡ 😄

@johanbrook johanbrook deleted the fix/throw-error-mixin branch January 12, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should throw error when mixin forgets to return new options object
2 participants