Skip to content

Commit

Permalink
fix(Symbol.iterator): will not polyfill Symbol iterator unless Symbol…
Browse files Browse the repository at this point in the history 
… exists (#2082)

- adds more robust tests
- removes strange logic added to support es6-shim, as no documentation or reasoning could be found to support its existence
- ponyfills Symbol rather than polyfills... meaning the module just returns something that can be used as the symbol, but
  if `Symbol` exists as a function, like it should, and it does not have `Symbol.iterator`, only then will it polyfill it

BREAKING CHANGE: RxJS will no longer polyfill `Symbol.iterator` if `Symbol` does not exist. This may break code that inadvertently relies on this behavior
  • Loading branch information
@benlesh @jayphelps
benlesh authored and jayphelps committed Nov 4, 2016
1 parent b5f8ab3 commit 1138c99
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 49 deletions.
122 changes: 95 additions & 27 deletions spec/symbol/iterator-spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,32 +1,100 @@
import {expect} from 'chai';

import {root} from '../../dist/cjs/util/root';
import {$$iterator} from '../../dist/cjs/symbol/iterator';
import { expect } from 'chai';
import { $$iterator, symbolIteratorPonyfill } from '../../dist/cjs/symbol/iterator';

describe('iterator symbol', () => {
it('should exist in the proper form', () => {
const Symbol = root.Symbol;
if (typeof Symbol === 'function') {
if (Symbol.iterator) {
expect($$iterator).to.equal(Symbol.iterator);
} else if (root.Set && typeof (new root.Set()['@@iterator']) === 'function') {
// FF bug coverage
expect($$iterator).to.equal('@@iterator');
} else if (root.Map) {
// es6-shim specific logic
let keys = Object.getOwnPropertyNames(root.Map.prototype);
for (let i = 0; i < keys.length; ++i) {
let key = keys[i];
if (key !== 'entries' && key !== 'size' && root.Map.prototype[key] === root.Map.prototype['entries']) {
expect($$iterator).to.equal(key);
break;
it('should exist', () => {
expect($$iterator).to.exist;
});
});

describe('symbolIteratorPonyfill', () => {
describe('when root.Symbol is a function', () => {
describe('and Symbol.iterator exists', () => {
it('should return Symbol.iterator', () => {
const FakeSymbol = function () { /* lol */ };
(<any>FakeSymbol).iterator = {};
const result = symbolIteratorPonyfill({ Symbol: FakeSymbol });
expect(result).to.equal((<any>FakeSymbol).iterator);
});
});

describe('and Symbol.iterator does not exist', () => {
it('should use Symbol to create an return a symbol and polyfill Symbol.iterator', () => {
const SYMBOL_RETURN = {};
let passedDescription: string;
const root = {
Symbol: function (description) {
passedDescription = description;
return SYMBOL_RETURN;
}
};

const result = symbolIteratorPonyfill(root);
expect(result).to.equal(SYMBOL_RETURN);
expect((<any>root.Symbol).iterator).to.equal(SYMBOL_RETURN);
});
});
});

describe('when root.Symbol is NOT a function', () => {
describe('and root.Set exists with an @@iterator property that is a function (Mozilla bug)', () => {
it ('should return "$$iterator"', () => {
const result = symbolIteratorPonyfill({
Set: function FakeSet() {
this['@@iterator'] = function () { /* lol */ };
}
}
} else if (typeof Symbol.for === 'function') {
expect($$iterator).to.equal(Symbol.for('iterator'));
}
} else {
expect($$iterator).to.equal('@@iterator');
}
});
expect(result).to.equal('@@iterator');
});
});

describe('root.Set does not exit or does not have an @@iterator property', () => {
describe('but Map exists and a random key on Map.prototype that matches Map.prototype.entries (for es6-shim)', () => {
it('should return the key that matches the "entries" key on Map.prototype, but not return "size"', () => {
function FakeMap() { /* lol */ }
function fakeMethod() { /* lol */ }
FakeMap.prototype['-omg-lol-i-can-use-whatever-I-want-YOLO-'] = fakeMethod;
FakeMap.prototype.entries = fakeMethod;
FakeMap.prototype.size = fakeMethod;
const root = {
Map: FakeMap
};

const result = symbolIteratorPonyfill(root);
expect(result).to.equal('-omg-lol-i-can-use-whatever-I-want-YOLO-');
});
});

describe('but Map exists and no other key except "size" on Map.prototype that matches Map.prototype.entries (for es6-shim)', () => {
it('should return "@@iterator"', () => {
function FakeMap() { /* lol */ }
function fakeMethod() { /* lol */ }
FakeMap.prototype.entries = fakeMethod;
FakeMap.prototype.size = fakeMethod;
const root = {
Map: FakeMap
};

const result = symbolIteratorPonyfill(root);
expect(result).to.equal('@@iterator');
});
});

describe('if Set exists but has no iterator, and Map does not exist (bad polyfill maybe?)', () => {
it('should return "@@iterator"', () => {
const result = symbolIteratorPonyfill({
Set: function () { /* lol */ }
});
expect(result).to.equal('@@iterator');
});
});

describe('and root.Set and root.Map do NOT exist', () => {
it('should return "@@iterator"', () => {
const result = symbolIteratorPonyfill({});
expect(result).to.equal('@@iterator');
});
});
});
});
});
47 changes: 25 additions & 22 deletions src/symbol/iterator.ts
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
import { root } from '../util/root';

export let $$iterator: any;
export function symbolIteratorPonyfill(root: any) {
const Symbol: any = root.Symbol;

const Symbol: any = root.Symbol;

if (typeof Symbol === 'function') {
if (Symbol.iterator) {
$$iterator = Symbol.iterator;
} else if (typeof Symbol.for === 'function') {
$$iterator = Symbol.for('iterator');
}
} else {
if (root.Set && typeof new root.Set()['@@iterator'] === 'function') {
// Bug for mozilla version
$$iterator = '@@iterator';
} else if (root.Map) {
// es6-shim specific logic
let keys = Object.getOwnPropertyNames(root.Map.prototype);
if (typeof Symbol === 'function') {
if (!Symbol.iterator) {
Symbol.iterator = Symbol('iterator polyfill');
}
return Symbol.iterator;
} else {
// [for Mozilla Gecko 27-35:](https://mzl.la/2ewE1zC)
const { Set } = root;
if (Set && typeof new Set()['@@iterator'] === 'function') {
return '@@iterator';
}
const { Map } = root;
// required for compatability with es6-shim
if (Map) {
let keys = Object.getOwnPropertyNames(Map.prototype);
for (let i = 0; i < keys.length; ++i) {
let key = keys[i];
if (key !== 'entries' && key !== 'size' && root.Map.prototype[key] === root.Map.prototype['entries']) {
$$iterator = key;
break;
// according to spec, Map.prototype[@@iterator] and Map.orototype.entries must be equal.
if (key !== 'entries' && key !== 'size' && Map.prototype[key] === Map.prototype['entries']) {
return key;
}
}
} else {
$$iterator = '@@iterator';
}
}
return '@@iterator';
}
}

export const $$iterator = symbolIteratorPonyfill(root);

0 comments on commit 1138c99

Please sign in to comment.