es-shims / es5-shim

ECMAScript 5 compatibility shims for legacy (and modern) JavaScript engines
MIT License
7.12k stars 898 forks source link

Decide what to do about "\u180E" / ᠎ #196

Open ljharb opened 10 years ago

ljharb commented 10 years ago

From https://github.com/es-shims/es5-shim/pull/194#issuecomment-31375159:

Here's a proposal by google to change that character from the "Zs" to "Cf" which might explain why they treat it as such: http://www.unicode.org/L2/L2013/13004-vowel-sep-change.pdf

Let's keep an eye on that proposal and follow up with TC39 if that character needs to be removed from the spec in ES6 (it's unlikely the ES5 spec will change)

Qantas94Heavy commented 9 years ago

Note that as of Unicode 6.3, "\u180E" is now "Cf" instead of "Zs".

Unicode 3.0 (the version ES5 requires support for) also treats it as "Cf", however ES6 requires support for Unicode 5.1, which defines it as "Zs". Should this be removed?

tmedwards commented 4 years ago

Indeed. I recently ran across an issue where the es5-shim <String>.trim() was causing errors—in combination with other code, not by itself—because of the erroneous inclusion of \u180E. Having to work around that was annoying.

It's not part of the ES5 spec, via Unicode 3, and should not be included in es5-shim.

If es6-shim needs it, then that is something that it should worry about, not es5-shim. Even then, I'd be wary about adding it, since it was dropped from Zs in subsequent versions of Unicode. Yes, you should want to follow the spec as closely as possible, but on the other hand you should also want to (a) not shim unnecessarily and (b) definitely not break more recent versions.

ljharb commented 4 years ago

If you're using es5-shim, you should be using es6-shim as well - do you see that issue when using both?

tmedwards commented 4 years ago

First. That doesn't necessarily follow. If you're transpiling down to ES5, for whatever reason, and don't need the ES6 APIs, then you aren't going to need the ES6 polyfill.

That said. Yes, the project in question does use both libraries—in order: es5-shim → es6-shim (and none of the shams). Also yes, I was seeing the issue in that configuration.

My resolution was to remove \u180E from the vendored copy of es5-shim's <String>.trim() test, so it wasn'tunnecessarily polyfilling over native implementations—i.e., it still polyfills missing or very broken <String>.trim() implementations, just not those simply not considering \u180E part of Zs.

For a one-off update on my end, it's not the end of the world, however, I'd rather not have to remember to do that every time a substantive update to es5-sim happens and I have to update my vendored copy.

ljharb commented 4 years ago

It follows because es6 changes some of the es5 APIs, and they’ll be incorrect if you omit the es6-shim.

I agree, that shouldn’t be needed, and es5-shim should be both 1) ensuring everything in ES5 is compliant, but also 2) not overwriting things that are correct in later specs.