Fix signatures of multiple functions #1067
Conversation
|
|
entries/callbacks.fireWith.xml
Outdated
| <argument name="args" optional="true"> | ||
| <desc>An argument, or array of arguments, to pass to the callbacks in the list.</desc> | ||
| <argument name="args" optional="true" type="Array"> | ||
| <desc>An array of arguments to pass to the callbacks in the list. If omitted or undefined, no arguments will be passed.</desc> |
There was a problem hiding this comment.
Hey @dratini0 , thanks for the PR! I don't think this change is correct, though. Looks to me like args can be "Anything" — an argument or array of arguments like the doc originally stated.
args = args || [];
args = [ context, args.slice ? args.slice() : args ];
queue.push( args );
There was a problem hiding this comment.
Take a look at the output of this fiddle then: https://jsfiddle.net/48Lmdmmf/
I have missed line 208, and assumed this was the intended behaviour. However, now I see that we are looking at a bug in jQuery, not in the documentation. I will put a pull request through shortly for this:
args = [ context, args.slice ? args.slice() : [args] ];I will also update this PR accordingly.
This reverts commit d24199e.
|
LGTM. @kswedberg? |
|
LGTM, too, @gibson042 . You wanna merge the PR? I would do it, but I can't seem to successfully run |
|
Same, but I'll give it a shot and 🤞 |
|
thank you, @gibson042 ! |
Following #1066, I have found several functions without a return type. I have assigned appropriate types to these. I have additionally figured out the signature of callbacks.callWith, and fixed that as well. The documentation had a mistake in it, it stated that if the argument is not an array, the callbacks will receive a single argument, but this is not true, it is in fact passed as the second argument of apply.
Finally, I have not assigned a return type to jQuery.error, as it never returns therefore has no return type.
I agree to the CLA. Do I need any paperwork? EDIT: Figured it out.