Core: Fallback to typeof obj in QUnit.objectType.
#1199
Conversation
test/main/utilities.js
Outdated
| try { | ||
| var async = new Function( "return async function( ) { };" )( ); | ||
| objects.push( async ); | ||
| } catch ( e ) { } |
There was a problem hiding this comment.
why this try / catch + wrapping the function forms in the Function ctor? ES6 compat?
This might escalate in a weird way, as we might also have method definitions, generators, classes, async generators...
There was a problem hiding this comment.
@leobalter - The same tests are ran on IE, Phantom, possibly Node 6, etc where async function() { } would be a parse error. The intent of the try / catch is to ensure that when async function() { } is not valid syntax, the tests for the plain function still runs.
There was a problem hiding this comment.
I got it! Thanks for the clarification!
btw eval allows a shorter syntax, but does the same:
new Function( "return async function( ) { };" )( ) => eval( "async function( ) {};" )
There was a problem hiding this comment.
@leobalter - HA! Yes, I started by trying to use eval, but I do not think that it allows returning the eval'ed value. This is important for us, because we have to ensure that having the syntax does not throw during evaluation of the whole file (and therefore miss all these tests).
Have I missed something? Is there a better way?
test/main/utilities.js
Outdated
| validateObjectType( [ "", "foo", String( "bar" ) ], "string" ); | ||
| validateObjectType( [ null ], "null" ); | ||
| validateObjectType( [ true, false, Boolean( true ) ], "boolean" ); | ||
| validateObjectType( [ [], new Array( ) ], "array" ); |
There was a problem hiding this comment.
the literal and ctor forms of Array are well tested on JS (Test262), same for primitive Number, String and Boolean types.
There was a problem hiding this comment.
Awesome! Do you think I should just remove the non-primitive versions here then?
There was a problem hiding this comment.
I'm +1 to keep only the primitive and literal values here
There was a problem hiding this comment.
Sounds good, updated.
test/main/utilities.js
Outdated
| validateObjectType( buildFunctions(), "function" ); | ||
| validateObjectType( typeof Symbol === "function" ? [ Symbol( "HI!" ) ] : [ ], "symbol" ); | ||
| validateObjectType( typeof Map === "function" ? [ new Map( ) ] : [ ], "map" ); | ||
| validateObjectType( typeof Set === "function" ? [ new Set( ) ] : [ ], "set" ); |
There was a problem hiding this comment.
where is the ordinary object? {}
test/main/utilities.js
Outdated
| validateObjectType( [ [], new Array( ) ], "array" ); | ||
| validateObjectType( [ new Date( ) ], "date" ); | ||
| validateObjectType( [ /foo/, new RegExp( "foo" ) ], "regexp" ); | ||
| validateObjectType( [ new Date( ) ], "date" ); |
There was a problem hiding this comment.
Whoops! Thats what I get for copy and pasting 😝
rwjblue
force-pushed
the
default-to-typeof
branch
3 times, most recently
from
22a8e96
to
08e1fae
Compare
Prior to these changes, `QUnit.objectType(async function() { });`
would return `undefined` (because
`Object.prototype.toString.call(async function() {})` equals
`[object AsyncFunction])`).
This updates `QUnit.objectType` to fallback to returning `typeof obj`
if no match was found, and adds explicit tests for the various expected
inputs and outputs.
|
Updated for feedback, and also updated |
There was a problem hiding this comment.
Looks good to me. Thanks for adding tests (honestly forgot we exposed this as public API).
@leobalter will wait a bit before merging in case you want to do another review.
This is a cherry-pick of qunitjs/qunit#1199. Change-Id: Ibc3a101575bfb57780e8838bc8b9e4ba4fe514f4
Prior to these changes,
QUnit.objectType(async function() { });would returnundefined(becauseObject.prototype.toString.call(async function() {})equals[object AsyncFunction])).This updates
QUnit.objectTypeto fallback to returningtypeof objif no match was found, and adds explicit tests for the various expected inputs and outputs.I discovered during the investigation of #1197 which is failing in [email protected] due to
QUnit.objectTypenot being aware of async functions (but master was fixed by a refactor done in #1188).