From 1138c99ec376d9b5ab4bf8fda286a6c79155e2d6 Mon Sep 17 00:00:00 2001
From: Ben Lesh <ben@benlesh.com>
Date: Fri, 4 Nov 2016 10:41:03 -0700
Subject: [PATCH] fix(Symbol.iterator): will not polyfill Symbol iterator
 unless Symbol 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
---
 spec/symbol/iterator-spec.ts | 122 +++++++++++++++++++++++++++--------
 src/symbol/iterator.ts       |  47 +++++++-------
 2 files changed, 120 insertions(+), 49 deletions(-)

diff --git a/spec/symbol/iterator-spec.ts b/spec/symbol/iterator-spec.ts
index 77a7273bce..7a8eb2b2ee 100644
--- a/spec/symbol/iterator-spec.ts
+++ b/spec/symbol/iterator-spec.ts
@@ -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');
+        });
+      });
+    });
   });
 });
\ No newline at end of file
diff --git a/src/symbol/iterator.ts b/src/symbol/iterator.ts
index 5855e431c7..3b689a7c89 100644
--- a/src/symbol/iterator.ts
+++ b/src/symbol/iterator.ts
@@ -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';
     }
-}
\ No newline at end of file
+    return '@@iterator';
+  }
+}
+
+export const $$iterator = symbolIteratorPonyfill(root);
\ No newline at end of file