search

paulmillr / es6-shim

ECMAScript 6 compatibility shims for legacy JS engines
http://paulmillr.com
MIT License
3.11k stars 387 forks source link

Question: When is Set/Map not? #382

Closed Xotic750 closed 6 years ago

Xotic750 commented 8 years ago

I am writing some type checking routines isSet and isMap among others. I have them using Object#toString when toStringTag is not available, .and when it is available then I'm using the size getter to perform a check. This is working nicely across realm and on all environments that I have tested on. I then threw es6-shim into the mix, which has now caused me some dilemma. On ES5 environments the shim is used, it walks, talks, eats, sleeps, and even farts like a Duck :) but it's DNA is not of a Duck, Object#toString is [object Object]. Ok, so the alternative method of using size says that it is a Duck, and I thought "that's ok then". But using this method without es6-shim then fails due to bugs in old FireFox where size is a function rather than a getter. So I'm wondering what thoughts are regarding type checkers and shims which implement a Duck in all but DNA. Should these checkers support es shims or should they count a shim as not a real Duck and say false. I'm not asking for code, just a perspective from shim makers as to whether isSet should return true for the Set shim in an ES5 environment or even a Set patch in an ES6 environment where the constructor has been replaced?

ljharb commented 8 years ago

You can go one of two ways - either isSet only returns true for a set that has native behavior (which is impractical to determine in JS), or, assume that if Set exists, cache the Set.prototype.size getter and use that (an old Firefox Set is not actually a Set if "size" is a non-getter).

Xotic750 commented 8 years ago

Thanks for that perspective, my head is still in dilemma about what to do about it, how much to check (is a buggy Set not a Set?). Seems likely that there is a need to drop Object#toString checking for Map and Set and be more duck-typed. Maybe sleeping on it will clear my view.

ljharb commented 8 years ago

Yes, Object#toString is not reliable whatsoever in an environment with Symbol.toStringTag, so it's best to avoid it except as an optimization in older engines.

Xotic750 commented 8 years ago

Yeah, I was only using it where toStringTag didn't exist, but it didn't exist in the buggy Firefox. So this begs the question of how much of an implementation to test before you say "this is a Set/Map", sure I could require that es6-shim is loaded, but unlike es5-shim (which I fully recommend/require) these shims still feel like they are in their infancy, and some even require true ES5 environments. It feels a bit of a "between a rock and a hard place" situation.

ljharb commented 8 years ago

es6-shim should still be used on every engine since there are tons of bugs in all of them with their implementations.

Xotic750 commented 8 years ago

Based on what you've said, I am going back over all my work to add it in. It's going to throw up some issues I'm sure. :)

cscott commented 8 years ago

The only really reliable mechanism is to compile a list of bugs on each platform, and test for them individually. That happens to be exactly what es6-shim is doing -- it then loads a shim to fix those bugs when it detects them.

But there are certainly ES6 features which are unshimmable. Our goal is that correctly written ES6 code which uses only the shimmable subset of ES6 will run correctly on every platform. If you use unshimmable features we can't make any guarantees. (ToStringTag is an unshimmable feature.)

Xotic750 commented 8 years ago

I'm currently moving away from toStringTag checks in all of my code, and adding es6-shim to all projects. Often it means implementing more costly code, but nothing that I'm working on needs to be so performant. :)

Xotic750 commented 6 years ago

Closing this, I'm very happy that the shim implements Map and Set as per spec, and that my isMap and isSet are happy. I look forward to a new release that includes the MapIterator and SetIterator changes that are sitting in the master. Thanks.