Core: jQuery.isFunction returns false for Function objects with custom @@toStringTag

[akihikodaki]

Description

jQuery.isFunction returns false for Function objects with custom @@toStringTag. That is undesirable in some case such as GeneratorFunction.
This flaw was pointed out at #3597 (comment).

Link to test case

https://jsbin.com/xocanozuye/1/edit?html,js,console

console.log(jQuery.isFunction(function *(){}));

[Krinkle]

@gibson042 From a quick check seems like patching $.type() to align more with typeof should otherwise be harmless. We've already been going in that direction anyways.

As of 3.2, we already do this for most cases. We now only use class2type[toString] for values where typeof returns function or object.

If we remove function from that, reducing use of class2type to object only, should do the trick.

Although we may need to find another way to support Android 2.3's RegExp values in that case.

type: function( obj ) {
	if ( obj == null ) {
		return obj + "";
	}

	// Support: Android <=2.3 only (functionish RegExp)
	return typeof obj === "object" || typeof obj === "function" ?
		class2type[ toString.call( obj ) ] || "object" :
		typeof obj;
},
isFunction: function( obj ) {
	return jQuery.type( obj ) === "function";
},

[mgol]

We don't support Android 2.3 anymore, 4.0 is our minimum. -- Michał Gołębiowski

[akihikodaki]

That breaks for some cases such as document.createElement( "object" ). It is/was actually a function (callable object), but jQuery.isFunction( document.createElement( "object" ) ) is expected to be false.

	obj = document.createElement( "object" );

	// Firefox says this is a function
assert.ok( !jQuery.isFunction( obj ), "Object Element" );

jquery/test/unit/core.js

Line 442 in ac9e301

obj = document.createElement( "object" );

On the other hand, the documentation says:

Determine the internal JavaScript [[Class]] of an object.

https://github.com/jquery/api.jquery.com/blob/eceefcf142534cb51ae443157a4752c9357f3ebd/entries/jQuery.type.xml#L10

Though [[Class]] does no longer exist since ECMAScript 2015, it is similar to @@toStringTag and people may consider jQuery.type derives from that. The change regarding GeneratorFunction as function breaks this expectation.

We need to make clear what we expect for jQuery.isFunction and jQuery.type rather than considering only GeneratorFunction to prevent such confusions.

[gibson042]

I'd rather deprecate jQuery.type than try to keep up with external changes. jQuery.isFunction doesn't need to rely on it, and shouldn't do so any longer IMO.

[mgol]

@gibson042 if all jQuery.isFunction is going to do is checking if typeof is "function" then we should deprecate it as well.

[gibson042]

I'm fine with deprecating it, but unfortunately it's a little more than just typeof because typeof document.createElement("object") === "function" in Firefox.

[mgol]

typeof document.createElement("object") === "function" in Firefox

Not just in Firefox, in Chrome it's "function", too. And in both of them it's changed to "object" in a beta version. I couldn't find any other browser (among these we support) where it's "function" so it will soon stop being an issue. Unless we want to be extra-safe here that nothing DOM-based returns true in jQuery.isFunction.

What's funny is in Chrome it used be "object" in versions <=46.

[akihikodaki]

It's a DOM specific problem, so why not check if it is an element in the functions which deals with DOM? It is not reasonable to do that in jQuery.isFunction.

[dmethvin]

I'm with @gibson042 that we should discourage the use of these utility functions and not set the expectation they'll be updated for bizarre usage. If someone passes a generator function to jQuery they're doing something mighty strange. DO NOT TAUNT HAPPY FUN JQUERY!

[Krinkle]

If we plan to deprecate these methods, that means people will eventually need to migrate use to typeof, isArray, and other native checks. In that case I propose we keep them unchanged and deprecate them in their current form. There's little point in improving them further to allow new usage of patterns only to require migration later. It's certainly less risky in terms of back-compat and more predictable / easier to document ([[Class]] / @@toStringTag).

[akihikodaki]

There's little point in improving them further to allow new usage of patterns only to require migration later.

Closing this issue.