This snippet of code attempts to shift the callback argument into place when an element is omitted, but it seems to unintentionally break the scenario where you only provide an element.
Possible cases:
mouseChange() - Works fine. Both arguments are undefined, so ultimately the callback is also undefined, and thus it doesn't break due to falsiness checks elsewhere.
mouseChange(element, callback) - Works fine. Both arguments are defined, so no shifting takes place.
mouseChange(callback) - Works fine. Second argument is unset, so the first argument (named element but actually containing the callback here) gets shifted into the callback variable.
mouseChange(element) - This one breaks. Shifting code assumes that since the second argument is undefined, the first argument must be a callback (but it's actually an element!) and shifts it into the callback variable. That variable is now truthy, so later on code passes the truthiness check and tries to call this callback that's actually an element, and it breaks with callback is not a function.
IMO the ideal change would be not to try and magically infer arguments like this, but rather accept an object of optional options. That'd be a breaking change, though.
The quicker (and non-breaking) patch would be to shift the argument only if the 'element' is a function and the callback is unset.
This snippet of code attempts to shift the
callbackargument into place when anelementis omitted, but it seems to unintentionally break the scenario where you only provide an element.Possible cases:
mouseChange()- Works fine. Both arguments are undefined, so ultimately the callback is also undefined, and thus it doesn't break due to falsiness checks elsewhere.mouseChange(element, callback)- Works fine. Both arguments are defined, so no shifting takes place.mouseChange(callback)- Works fine. Second argument is unset, so the first argument (namedelementbut actually containing the callback here) gets shifted into thecallbackvariable.mouseChange(element)- This one breaks. Shifting code assumes that since the second argument is undefined, the first argument must be a callback (but it's actually an element!) and shifts it into thecallbackvariable. That variable is now truthy, so later on code passes the truthiness check and tries to call this callback that's actually an element, and it breaks withcallback is not a function.IMO the ideal change would be not to try and magically infer arguments like this, but rather accept an object of optional options. That'd be a breaking change, though.
The quicker (and non-breaking) patch would be to shift the argument only if the 'element' is a function and the callback is unset.