feat(distinct): remove distinctKey, distinct signature change and perf improvements (#2049)

* feat(distinct): remove `distinctKey`,  `distinct` signature change and perf improvements

- Adds a limited Set ponyfill for runtimes that do not support `Set`
- `distinct` now supports an optional keySelector argument that the user can use to select the value to check distinct on
- `distinctKey` is removed as it is redundant
- `distinct` no longer supports a comparer function argument, as there is little to no use case for such an argument that could not be covered by the keySelector
- updates tests to remove tests that do not make sense and test new functionality

BREAKING CHANGE: `distinctKey` has been removed. Use `distinct`
BREAKING CHANGE: `distinct` operator has changed, first argument is an
optional `keySelector`. The custom `compare` function is no longer
supported.

resolves #2009

* perf(distinct): increase `distinct()` perf by improving deopts

- moves keySelector call to a different function with a try catch to improve V8 optimization
- distinct calls with no keySelector passed now take a fully optimized path, doubling speed again

related #2009

* docs(distinct): update distinct docs to fit new API

Author: benlesh

spec/operators/distinct-spec.ts

@@ -138,46 +138,29 @@ describe('Observable.prototype.distinct', () => {
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
   });
 
-  it('should emit once if comparer returns true always regardless of source emits', () => {
-    const e1 =   hot('--a--b--c--d--e--f--|');
-    const e1subs =   '^                   !';
-    const expected = '--a-----------------|';
-
-    expectObservable((<any>e1).distinct(() => true)).toBe(expected);
-    expectSubscriptions(e1.subscriptions).toBe(e1subs);
-  });
-
-  it('should emit all if comparer returns false always regardless of source emits', () => {
-    const e1 =   hot('--a--a--a--a--a--a--|');
+  it('should distinguish values by key', () => {
+    const values = {a: 1, b: 2, c: 3, d: 4, e: 5, f: 6};
+    const e1 =   hot('--a--b--c--d--e--f--|', values);
     const e1subs =   '^                   !';
-    const expected = '--a--a--a--a--a--a--|';
+    const expected = '--a--b--c-----------|';
+    const selector = (value: number) => value % 3;
 
-    expectObservable((<any>e1).distinct(() => false)).toBe(expected);
+    expectObservable((<any>e1).distinct(selector)).toBe(expected, values);
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
   });
 
-  it('should distinguish values by selector', () => {
-    const e1 =   hot('--a--b--c--d--e--f--|', {a: 1, b: 2, c: 3, d: 4, e: 5, f: 6});
-    const e1subs =   '^                   !';
-    const expected = '--a-----c-----e-----|';
-    const selector = (x: number, y: number) => y % 2 === 0;
-
-    expectObservable((<any>e1).distinct(selector)).toBe(expected, {a: 1, c: 3, e: 5});
-    expectSubscriptions(e1.subscriptions).toBe(e1subs);
-  });
-
-  it('should raises error when comparer throws', () => {
+  it('should raises error when selector throws', () => {
     const e1 =   hot('--a--b--c--d--e--f--|');
     const e1subs =   '^          !         ';
     const expected = '--a--b--c--#         ';
-    const selector = (x: string, y: string) => {
-      if (y === 'd') {
-        throw 'error';
+    const selector = (value: string) => {
+      if (value === 'd') {
+        throw new Error('d is for dumb');
       }
-      return x === y;
+      return value;
     };
 
-    expectObservable((<any>e1).distinct(selector)).toBe(expected);
+    expectObservable((<any>e1).distinct(selector)).toBe(expected, undefined, new Error('d is for dumb'));
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
   });
 
@@ -187,9 +170,8 @@ describe('Observable.prototype.distinct', () => {
     const e2 =   hot('-----------x--------|');
     const e2subs =   '^                   !';
     const expected = '--a--b--------a--b--|';
-    const selector = (x: string, y: string) => x === y;
 
-    expectObservable((<any>e1).distinct(selector, e2)).toBe(expected);
+    expectObservable((<any>e1).distinct(null, e2)).toBe(expected);
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
     expectSubscriptions(e2.subscriptions).toBe(e2subs);
   });
@@ -200,9 +182,8 @@ describe('Observable.prototype.distinct', () => {
     const e2 =   hot('-----------x-#');
     const e2subs =   '^            !';
     const expected = '--a--b-------#';
-    const selector = (x: string, y: string) => x === y;
 
-    expectObservable((<any>e1).distinct(selector, e2)).toBe(expected);
+    expectObservable((<any>e1).distinct(null, e2)).toBe(expected);
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
     expectSubscriptions(e2.subscriptions).toBe(e2subs);
   });
@@ -214,9 +195,8 @@ describe('Observable.prototype.distinct', () => {
     const e2subs =   '^          !         ';
     const unsub =    '           !         ';
     const expected = '--a--b------';
-    const selector = (x: string, y: string) => x === y;
 
-    expectObservable((<any>e1).distinct(selector, e2), unsub).toBe(expected);
+    expectObservable((<any>e1).distinct(null, e2), unsub).toBe(expected);
     expectSubscriptions(e1.subscriptions).toBe(e1subs);
     expectSubscriptions(e2.subscriptions).toBe(e2subs);
   });

spec/operators/distinctKey-spec.ts

@@ -0,0 +1,60 @@
+import {expect} from 'chai';
+import { Set as TestSet, minimalSetImpl } from '../../dist/cjs/util/Set';
+
+describe('Set', () => {
+  if (typeof Set === 'function') {
+    it('should use Set if Set exists', () => {
+      expect(TestSet).to.equal(Set);
+    });
+  }
+});
+
+describe('minimalSetImpl()', () => {
+  it('should provide the minimal Set we require', () => {
+    const MinSet = minimalSetImpl();
+    const test = new MinSet();
+
+    expect(MinSet.prototype.add).to.be.a('function');
+    expect(MinSet.prototype.has).to.be.a('function');
+    expect(MinSet.prototype.clear).to.be.a('function');
+    expect(test.size).to.be.a('number');
+  });
+
+  describe('returned MinimalSet', () => {
+    it('should implement add, has, size and clear', () => {
+      const MinSet = minimalSetImpl();
+      const test = new MinSet();
+
+      expect(test.size).to.equal(0);
+
+      test.add('Laverne');
+      expect(test.size).to.equal(1);
+      expect(test.has('Laverne')).to.be.true;
+      expect(test.has('Shirley')).to.be.false;
+
+      test.add('Shirley');
+      expect(test.size).to.equal(2);
+      expect(test.has('Laverne')).to.be.true;
+      expect(test.has('Shirley')).to.be.true;
+
+      const squiggy = { name: 'Andrew Squiggman' };
+      const identicalSquiggy = { name: 'Andrew Squiggman' }; // lol, imposter!
+
+      test.add(squiggy);
+      expect(test.size).to.equal(3);
+      expect(test.has(identicalSquiggy)).to.be.false;
+      expect(test.has(squiggy)).to.be.true;
+
+      test.clear();
+      expect(test.size).to.equal(0);
+      expect(test.has('Laverne')).to.be.false;
+      expect(test.has('Shirley')).to.be.false;
+      expect(test.has(squiggy)).to.be.false;
+      expect(test.has(identicalSquiggy)).to.be.false;
+
+      test.add('Fonzi');
+      expect(test.size).to.equal(1);
+      expect(test.has('Fonzi')).to.be.true;
+    });
+  });
+});

spec/util/Set-spec.ts

@@ -59,7 +59,6 @@ import './add/operator/defaultIfEmpty';
 import './add/operator/delay';
 import './add/operator/delayWhen';
 import './add/operator/distinct';
-import './add/operator/distinctKey';
 import './add/operator/distinctUntilChanged';
 import './add/operator/distinctUntilKeyChanged';
 import './add/operator/do';