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

Deferred: behavior different from ECMAScript Promise for objects with [[Call]] #3596

Closed
akihikodaki opened this issue Mar 31, 2017 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@akihikodaki
Copy link

@akihikodaki akihikodaki commented Mar 31, 2017

Description

jQuery's Deferred is expected to be a close approximation to Promise of ECMAScript 2016 Language Specification. Promise defined in ECMAScript 2016 tests onFulfilled, onRejected and then are objects with [[Call]].
However, isFunction, used in the Deferred, tests toStringTag, which is irrelevant from the definition. As the result, Deferred shows different behaviors for some objects such as GeneratorFunction.

Link to test case

https://jsbin.com/civabuxabe/1/edit?js,console
Expected [object Generator] { ... }, but results in 0.

jQuery.Deferred()
  .resolve(0)
  .then(function *() {})
  .then(console.log);
@akihikodaki akihikodaki mentioned this issue Mar 31, 2017
4 of 4 tasks complete
@gibson042
Copy link
Member

@gibson042 gibson042 commented Mar 31, 2017

Confirmed, but I would classify this as a bug in jQuery.isFunction rather than Deferreds, and consider improper detection of async functions to be an even bigger flaw. I'd love to switch to typeof, but doubt we actually can, especially in a minor release... even lodash still goes by toStringTag.

@akihikodaki
Copy link
Author

@akihikodaki akihikodaki commented Apr 1, 2017

lodash says:

Checks if value is classified as a Function object.

However, the standard requires the callbacks are functions (objects implementing [[Call]]). jQuery.isFunction and _.isFunction is inappropriate for the purpose.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Apr 1, 2017

It can be rather difficult to perfectly determine if an object has a [[Call]] internal method, and there are some tough edge cases (none of which we properly detect now):

  • In IE9, which we still support, XML host objects have callable properties like getElementsByTagName for which typeof returns "unknown".
  • In modern browsers, document.all is callable but has typeof "undefined".

The intent of jQuery.isFunction is to serve as a reasonable approximation of ECMAScript IsCallable, and we're not going out of our way to catch every possible trick but I for one want it to recognize generators and async functions. So maybe we are at the point where typeof makes the most sense. @jquery/core would anyone else like to weigh in?

@akihikodaki
Copy link
Author

@akihikodaki akihikodaki commented Apr 1, 2017

It can be rather difficult to perfectly determine if an object has a [[Call]] internal method, and there are some tough edge cases (none of which we properly detect now):

I was not aware of these cases. By the way, I tested the second case. document.all was not callable on Firefox 52. It was callable on Chromium 57, but jQuery.isFunction(document.all) returned false. I'm still investigating those cases.

The intent of jQuery.isFunction is to serve as a reasonable approximation of ECMAScript IsCallable

A test suggests it is different from ECMAScript IsCallable.

	obj = document.createElement( "object" );

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

obj = document.createElement( "object" );

It is callable. We may need to clarify the expected behavior of jQuery.isFunction. I have opened an issue. jquery/api.jquery.com#1034

@akihikodaki
Copy link
Author

@akihikodaki akihikodaki commented Apr 1, 2017

So maybe we are at the point where typeof makes the most sense. @jquery/core would anyone else like to weigh in?

Opened #3601.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Apr 2, 2017

If we pursue a fix for #3600 (which I'd like to do), then the only requirement to resolve this issue will be new unit tests.

@gibson042 gibson042 added Deferred and removed Core Needs review labels Apr 2, 2017
gibson042 added a commit to gibson042/jquery that referenced this issue Apr 8, 2017
@gibson042 gibson042 self-assigned this Apr 8, 2017
@gibson042 gibson042 added this to the 3.3.0 milestone Apr 8, 2017
gibson042 added a commit that referenced this issue Apr 24, 2017
@gibson042 gibson042 mentioned this issue Jan 28, 2018
2 of 4 tasks complete
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants