paulmillr / es6-shim

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

[Fix] Add non-native symbol check #460

Closed wintlu closed 4 years ago

wintlu commented 4 years ago

Fix issue #459

When we include a Symbol shim before es6-shim, Symbol.iterator key is not correctly recognized, and this caused bunched of other issues. (for example, cannot retrieve correct iterator from ArrayIterator object)

wintlu commented 4 years ago

hi @ljharb, need your help on writing tests (don't quite understand how tests work in this repo):

  1. which file should this symbol test go? can I reuse array.js or do I need to create new file like test/symbol.js?
  2. how do I run test? do I simply run it locally by npm run test, or will the CI run it in real IE11?
  3. how do I write the test? Do I need to create a dummy Symbol and verify on it? something like:
    
    const Symbol = () => {}
    Symbol.iterator = { next: () => {}, done: false };

it('array has non native Symbol.iterator', () => { expect([][Symbol.iterator]).to.NotNull() });

ljharb commented 4 years ago

It's probably better to make a new file, but it's fine to reuse the array file as well. I can reorganize once you've added the test, if needed :-)

To run the tests in IE 11, you'll have to load https://github.com/paulmillr/es6-shim/blob/master/test/index.html in IE 11 yourself, manually.

I'd expect that in an environment with native symbols, the test would just use them, but in an environment without them, it would replicate the conditions that cause #459 - such that the test fails prior to this fix.

wintlu commented 4 years ago

google maps fixed in their code, so we don't need to make this change here, thanks!