feature detects for ES6 Array, Math, Number, Object, String; for #1170

[jokeyrhyme]

for #1170

[patrickkettner]

do you mean ECMAScript 6 Array?

[jokeyrhyme]

I'll fix this in a second. :)

[jokeyrhyme]

I noticed that there's a separate feature-detect for String.prototype.contains by @robertkowalski

I have left it in place. However, it's possible that it makes sense to remove it, as the es6string detect covers it as well.

[jokeyrhyme]

I squashed and force pushed. The typo and the unwrapping of unnecessary functions has been done.

[patrickkettner]

bravo, sir. very well done.

I am a bit confused as to the utility of this, though. Why would someone want to know that all properties exist (rather than a specific one?)

[jokeyrhyme]

@patrickkettner I'm just following the pattern of a previous pull request I got through for the ES5 tests: #901

It seemed okay back then to test for them in type-specific batches.

[patrickkettner]

@jokeyrhyme - totally get that, it was more of a topic-of-discussion for @stucox, @ryanseddon, @paulirish, yourself, and anyone else who would like to pitch in an opinion.

[stucox]

Separate detects for every feature would result in LOADS of new detects — and you'd have to include loads to know whether or not to load a shim (which you may well not want to download if it isn't needed). There's both a performance & maintenance overhead in having many separate detects instead of a few broader ones.

I haven't really looked into the available ES6 shims, but for ES5 it was evident that grouping them like this was a good compromise, meaning only a couple of detects were needed to decide which shim/sham may be needed — while not tying Modernizr to a specific set of shims.

[stucox]

@jokeyrhyme How do we expect these to evolve as the drafts change? As you highlighted on the other thread, a few things are still in flux. While we want to keep current, it might be odd if we release a detect which changes what it considers to be a “compliant” implementation too frequently.

We could attach a warning that this is a new spec in flux and our detect may change accordingly?

[jokeyrhyme]

We could always hold off merging this until ES6 goes gold. Also, this won't be in the wild until Modernizr v3 at the earliest if you do merge. What's that timeframe versus ES6 timeframe looking like?

Number.prototype.clz is the only method that has moved in months, and there are a handful of additional properties that we'll need to add to the "es6object" test in a month or so. The methods on the basic types seem to be the most solid parts of ES6 at this point (with Firefox and Chrome actually implementing a good chunk of them in their stable branches).

Anyone trying to detect ES6 right now should know that ES6 itself isn't finished, so our feature detects can't be considered final. If we have an "experimental" flag, that would be great. Is it worth raising that as a separate issue?

An experimental flag might even allow us to avoid future issues like the "flexbox" and "flexwrap" detects, which really should have been merged but couldn't be because of downstream compatibility concerns.

[stucox]

I'm 1 free weekend away from being ready to release a v3 beta.

I don't think we need an "experimental" flag, that's kind of what warnings is for (and was actually one of the proposed use cases for it when we defined the metadata structure).

[jokeyrhyme]

@stucox my bad. I didn't realise there actually already was a warnings metadata field. I'll set this appropriately, hopefully within the next 24 hours.

[jokeyrhyme]

I just updated the notes and warnings metadata fields as suggested.

Let me know if there is anything else found wanting. :)

[stucox]

Excellent stuff.

This looks good to me then. What say you, @patrickkettner? Happy with this approach?

[patrickkettner]

+1

only nit is I think detection should be detect, based on the last what-we-call-the-detect-tests-thingies conversation.

[jokeyrhyme]

@patrickkettner done!

[patrickkettner]

I'm +1

[jokeyrhyme]

Just one last question: do you think I ought to included sub-class tests for the built-ins in this set of detects?
https://github.com/lukehoban/es6features#subclassable-built-ins

[patrickkettner]

sounds useful

[stucox]

Maybe that should be a separate detect (or at least a separate PR). Is it reasonable to assume a browser would make all of these built-ins subclassable at the same time?

[jokeyrhyme]

Perhaps. As I discuss over in the ticket, detecting subclassable would possibly rely on either the es5object detect or an as-yet-unwritten detect for ES6 classes.

It's probably best to tackle these in a separate detect.

[stucox]

I think we’re looking good here then. Unless we want to change all of the clauses to use 'clz32' in Math etc… but that’d make it seriously verbose for this many checks.

Any properties here for which we can imagine some implementation might throw an error for when accessed, rather than just being undefined if not supported? Or any which could possibly have a falsy value when they are supported?

[jokeyrhyme]

@stucox We have the warnings in place. Does this not give us license to ask for forgiveness later? :)
These detects should be safe, from my reading of the drafts. I do expect they'll need some gentle tweaking once ES6 goes gold, anyway.

[stucox]

Yep, sounds like a plan.

Thanks dude 👍