From bed0d1ac3e8ec74e1726549aab91681610253ae8 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 25 Oct 2021 13:41:30 -0600 Subject: [PATCH] Fix: accept Iterable in TrackedMap and TrackedSet - add constructor overloads to match those from Map and Set - add both type and runtime tests to verify this works as expected - convert entire test suite to TS to get type validation in place at least to some degree (partially addresses #32) --- config/tsconfig.addon.json | 9 +++ config/tsconfig.cjs.json | 9 +++ package.json | 4 +- src/-private/map.ts | 33 ++++++++--- src/-private/set.ts | 19 ++++--- tests/dummy/app/{app.js => app.ts} | 2 +- tests/dummy/{app => }/config/environment.d.ts | 1 + tests/{test-helper.js => test-helper.ts} | 4 +- tests/unit/{map-test.js => map-test.ts} | 37 ++++++------ tests/unit/{set-test.js => set-test.ts} | 57 ++++++++++++++----- .../{weak-map-test.js => weak-map-test.ts} | 22 ++++--- .../{weak-set-test.js => weak-set-test.ts} | 23 +++++--- tsconfig.json | 20 +++---- yarn.lock | 12 ++++ 14 files changed, 176 insertions(+), 76 deletions(-) create mode 100644 config/tsconfig.addon.json create mode 100644 config/tsconfig.cjs.json rename tests/dummy/app/{app.js => app.ts} (87%) rename tests/dummy/{app => }/config/environment.d.ts (91%) rename tests/{test-helper.js => test-helper.ts} (64%) rename tests/unit/{map-test.js => map-test.ts} (89%) rename tests/unit/{set-test.js => set-test.ts} (81%) rename tests/unit/{weak-map-test.js => weak-map-test.ts} (85%) rename tests/unit/{weak-set-test.js => weak-set-test.ts} (80%) diff --git a/config/tsconfig.addon.json b/config/tsconfig.addon.json new file mode 100644 index 0000000..92976ce --- /dev/null +++ b/config/tsconfig.addon.json @@ -0,0 +1,9 @@ +{ + "extends": "../tsconfig.json", + // From root of repo, *not* from directory where this tsconfig is located + "include": ["../src"], + "compilerOptions": { + "outDir": "../addon", + "module": "ES2015" + } +} diff --git a/config/tsconfig.cjs.json b/config/tsconfig.cjs.json new file mode 100644 index 0000000..b73e641 --- /dev/null +++ b/config/tsconfig.cjs.json @@ -0,0 +1,9 @@ +{ + "extends": "../tsconfig.json", + "include": ["../src"], + "compilerOptions": { + "outDir": "../commonjs", + "declaration": true, + "module": "CommonJS" + } +} diff --git a/package.json b/package.json index 9f3a3cd..5a813a3 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "test": "tests" }, "scripts": { - "build:ts": "tsc && tsc --declaration --outDir commonjs --module CommonJS", + "build:ts": "tsc --project config/tsconfig.addon.json && tsc --project config/tsconfig.cjs.json", "build": "ember build --environment=production", "lint": "npm-run-all --aggregate-output --continue-on-error --parallel 'lint:!(fix)'", "lint:hbs": "ember-template-lint .", @@ -43,6 +43,7 @@ "@glimmer/component": "^1.0.0", "@types/ember": "^3.16.5", "@types/ember-qunit": "^3.4.13", + "@types/ember-resolver": "^5.0.10", "@types/ember__test-helpers": "^2.0.0", "@types/qunit": "^2.11.1", "@types/rsvp": "^4.0.3", @@ -70,6 +71,7 @@ "eslint": "^7.0.0", "eslint-plugin-ember": "^10.4.1", "eslint-plugin-node": "^11.0.0", + "expect-type": "^0.13.0", "loader.js": "^4.7.0", "npm-run-all": "^4.1.5", "qunit": "^2.17.2", diff --git a/src/-private/map.ts b/src/-private/map.ts index 6dcf42f..fdd1a32 100644 --- a/src/-private/map.ts +++ b/src/-private/map.ts @@ -32,8 +32,19 @@ export class TrackedMap implements Map { } } - constructor(entries?: readonly (readonly [K, V])[] | null) { - this.vals = new Map(entries); + constructor(); + constructor(entries: readonly (readonly [K, V])[] | null); + constructor(iterable: Iterable); + constructor( + existing?: + | readonly (readonly [K, V])[] + | Iterable + | null + | undefined + ) { + // TypeScript doesn't correctly resolve the overloads for calling the `Map` + // constructor for the no-value constructor. This resolves that. + this.vals = existing ? new Map(existing) : new Map(); } // **** KEY GETTERS **** @@ -120,10 +131,9 @@ export class TrackedMap implements Map { // So instanceof works Object.setPrototypeOf(TrackedMap.prototype, Map.prototype); -export class TrackedWeakMap< - K extends object = object, - V = unknown -> implements WeakMap { +export class TrackedWeakMap + implements WeakMap +{ private storages: WeakMap> = new WeakMap(); private vals: WeakMap; @@ -148,8 +158,15 @@ export class TrackedWeakMap< } } - constructor(iterable?: (readonly [K, V][] | null)) { - this.vals = new WeakMap(iterable); + constructor(); + constructor(iterable: Iterable); + constructor(entries: readonly [K, V][] | null); + constructor( + existing?: readonly [K, V][] | Iterable | null | undefined + ) { + // TypeScript doesn't correctly resolve the overloads for calling the `Map` + // constructor for the no-value constructor. This resolves that. + this.vals = existing ? new WeakMap(existing) : new WeakMap(); } get(key: K): V | undefined { diff --git a/src/-private/set.ts b/src/-private/set.ts index 19e2849..e720b1b 100644 --- a/src/-private/set.ts +++ b/src/-private/set.ts @@ -32,8 +32,11 @@ export class TrackedSet implements Set { } } - constructor(values?: readonly T[] | null) { - this.vals = new Set(values); + constructor(); + constructor(values: readonly T[] | null); + constructor(iterable: Iterable); + constructor(existing?: readonly T[] | Iterable | null | undefined) { + this.vals = new Set(existing); } // **** KEY GETTERS **** @@ -45,31 +48,31 @@ export class TrackedSet implements Set { // **** ALL GETTERS **** entries(): IterableIterator<[T, T]> { - getValue(this.collection) + getValue(this.collection); return this.vals.entries(); } keys(): IterableIterator { - getValue(this.collection) + getValue(this.collection); return this.vals.keys(); } values(): IterableIterator { - getValue(this.collection) + getValue(this.collection); return this.vals.values(); } forEach(fn: (value1: T, value2: T, set: Set) => void): void { - getValue(this.collection) + getValue(this.collection); this.vals.forEach(fn); } get size(): number { - getValue(this.collection) + getValue(this.collection); return this.vals.size; } @@ -86,7 +89,7 @@ export class TrackedSet implements Set { // **** KEY SETTERS **** add(value: T): this { - this.dirtyStorageFor(value) + this.dirtyStorageFor(value); setValue(this.collection, null); this.vals.add(value); diff --git a/tests/dummy/app/app.js b/tests/dummy/app/app.ts similarity index 87% rename from tests/dummy/app/app.js rename to tests/dummy/app/app.ts index d8e2088..523bad6 100644 --- a/tests/dummy/app/app.js +++ b/tests/dummy/app/app.ts @@ -1,7 +1,7 @@ import Application from '@ember/application'; import Resolver from 'ember-resolver'; import loadInitializers from 'ember-load-initializers'; -import config from './config/environment'; +import config from 'dummy/config/environment'; export default class App extends Application { modulePrefix = config.modulePrefix; diff --git a/tests/dummy/app/config/environment.d.ts b/tests/dummy/config/environment.d.ts similarity index 91% rename from tests/dummy/app/config/environment.d.ts rename to tests/dummy/config/environment.d.ts index 3a01473..90c27fb 100644 --- a/tests/dummy/app/config/environment.d.ts +++ b/tests/dummy/config/environment.d.ts @@ -13,4 +13,5 @@ declare const config: { podModulePrefix: string; locationType: string; rootURL: string; + APP: Record; }; diff --git a/tests/test-helper.js b/tests/test-helper.ts similarity index 64% rename from tests/test-helper.js rename to tests/test-helper.ts index 0382a84..4eae06f 100644 --- a/tests/test-helper.js +++ b/tests/test-helper.ts @@ -1,5 +1,5 @@ -import Application from '../app'; -import config from '../config/environment'; +import Application from 'dummy/app'; +import config from 'dummy/config/environment'; import { setApplication } from '@ember/test-helpers'; import { start } from 'ember-qunit'; diff --git a/tests/unit/map-test.js b/tests/unit/map-test.ts similarity index 89% rename from tests/unit/map-test.js rename to tests/unit/map-test.ts index a28dfde..a2a0e41 100644 --- a/tests/unit/map-test.js +++ b/tests/unit/map-test.ts @@ -3,14 +3,19 @@ import { TrackedMap } from 'tracked-maps-and-sets'; import { setupRenderingTest } from 'ember-qunit'; import { module, test } from 'qunit'; +import { expectTypeOf } from 'expect-type'; + import { reactivityTest } from '../helpers/reactivity'; import { eachReactivityTest, eachInReactivityTest } from '../helpers/collection-reactivity'; +expectTypeOf>().toMatchTypeOf>(); +expectTypeOf>().not.toMatchTypeOf>(); + module('TrackedMap', function(hooks) { setupRenderingTest(hooks); test('constructor', assert => { - let map = new TrackedMap([['foo', 123]]); + const map = new TrackedMap([['foo', 123]]); assert.equal(map.get('foo'), 123); assert.equal(map.size, 1); @@ -18,10 +23,10 @@ module('TrackedMap', function(hooks) { }); test('works with all kinds of keys', assert => { - let map = new TrackedMap([ + const map = new TrackedMap([ ['foo', 123], [{}, {}], - [() => {}, 'bar'], + [() => {/* no op! */}, 'bar'], [123, true], [true, false], [null, null], @@ -31,7 +36,7 @@ module('TrackedMap', function(hooks) { }); test('get/set', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set('foo', 123); assert.equal(map.get('foo'), 123); @@ -41,7 +46,7 @@ module('TrackedMap', function(hooks) { }); test('has', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); assert.equal(map.has('foo'), false); map.set('foo', 123); @@ -49,12 +54,12 @@ module('TrackedMap', function(hooks) { }); test('entries', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set(0, 1); map.set(1, 2); map.set(2, 3); - let iter = map.entries(); + const iter = map.entries(); assert.deepEqual(iter.next().value, [0, 1]); assert.deepEqual(iter.next().value, [1, 2]); @@ -63,12 +68,12 @@ module('TrackedMap', function(hooks) { }); test('keys', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set(0, 1); map.set(1, 2); map.set(2, 3); - let iter = map.keys(); + const iter = map.keys(); assert.equal(iter.next().value, 0); assert.equal(iter.next().value, 1); @@ -77,12 +82,12 @@ module('TrackedMap', function(hooks) { }); test('values', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set(0, 1); map.set(1, 2); map.set(2, 3); - let iter = map.values(); + const iter = map.values(); assert.equal(iter.next().value, 1); assert.equal(iter.next().value, 2); @@ -91,7 +96,7 @@ module('TrackedMap', function(hooks) { }); test('forEach', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set(0, 1); map.set(1, 2); map.set(2, 3); @@ -110,7 +115,7 @@ module('TrackedMap', function(hooks) { }); test('size', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); assert.equal(map.size, 0); map.set(0, 1); @@ -127,7 +132,7 @@ module('TrackedMap', function(hooks) { }); test('delete', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); assert.equal(map.has(0), false); @@ -139,7 +144,7 @@ module('TrackedMap', function(hooks) { }); test('clear', assert => { - let map = new TrackedMap(); + const map = new TrackedMap(); map.set(0, 1); map.set(1, 2); @@ -263,7 +268,7 @@ module('TrackedMap', function(hooks) { map = new TrackedMap(); get value() { - this.map.forEach(() => {}); + this.map.forEach(() => {/* no op! */}); return 'test'; } diff --git a/tests/unit/set-test.js b/tests/unit/set-test.ts similarity index 81% rename from tests/unit/set-test.js rename to tests/unit/set-test.ts index 5ded8cf..2f82ded 100644 --- a/tests/unit/set-test.js +++ b/tests/unit/set-test.ts @@ -3,40 +3,69 @@ import { TrackedSet } from 'tracked-maps-and-sets'; import { setupRenderingTest } from 'ember-qunit'; import { module, test } from 'qunit'; +import { expectTypeOf } from 'expect-type'; + import { reactivityTest } from '../helpers/reactivity'; import { eachReactivityTest } from '../helpers/collection-reactivity'; +expectTypeOf>().toMatchTypeOf>(); +expectTypeOf>().not.toEqualTypeOf>(); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyFn = (...args: any[]) => any; + module('TrackedSet', function(hooks) { setupRenderingTest(hooks); test('constructor', assert => { - let set = new TrackedSet(['foo', 123]); + const set = new TrackedSet(['foo', 123]); assert.equal(set.has('foo'), true); assert.equal(set.size, 2); assert.ok(set instanceof Set); + + const setFromSet = new TrackedSet(set); + assert.equal(setFromSet.has('foo'), true); + assert.equal(setFromSet.size, 2); + assert.ok(setFromSet instanceof Set); + + const setFromEmpty = new TrackedSet(); + assert.equal(setFromEmpty.has('anything'), false); + assert.equal(setFromEmpty.size, 0); + assert.ok(setFromEmpty instanceof Set); }); test('works with all kinds of values', assert => { - let set = new TrackedSet(['foo', {}, () => {}, 123, true, null]); + const set = new TrackedSet< + string | Record | AnyFn | number | boolean | null + >([ + 'foo', + {}, + () => { + /* no op */ + }, + 123, + true, + null, + ]); assert.equal(set.size, 6); }); test('add/has', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add('foo'); assert.equal(set.has('foo'), true); }); test('entries', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add(0); set.add(2); set.add(1); - let iter = set.entries(); + const iter = set.entries(); assert.deepEqual(iter.next().value, [0, 0]); assert.deepEqual(iter.next().value, [2, 2]); @@ -45,12 +74,12 @@ module('TrackedSet', function(hooks) { }); test('keys', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add(0); set.add(2); set.add(1); - let iter = set.keys(); + const iter = set.keys(); assert.equal(iter.next().value, 0); assert.equal(iter.next().value, 2); @@ -59,12 +88,12 @@ module('TrackedSet', function(hooks) { }); test('values', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add(0); set.add(2); set.add(1); - let iter = set.values(); + const iter = set.values(); assert.equal(iter.next().value, 0); assert.equal(iter.next().value, 2); @@ -73,7 +102,7 @@ module('TrackedSet', function(hooks) { }); test('forEach', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add(0); set.add(1); set.add(2); @@ -92,7 +121,7 @@ module('TrackedSet', function(hooks) { }); test('size', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); assert.equal(set.size, 0); set.add(0); @@ -109,7 +138,7 @@ module('TrackedSet', function(hooks) { }); test('delete', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); assert.equal(set.has(0), false); @@ -121,7 +150,7 @@ module('TrackedSet', function(hooks) { }); test('clear', assert => { - let set = new TrackedSet(); + const set = new TrackedSet(); set.add(0); set.add(1); @@ -230,7 +259,7 @@ module('TrackedSet', function(hooks) { set = new TrackedSet(); get value() { - this.set.forEach(() => {}); + this.set.forEach(() => {/* no-op */}); return 'test'; } diff --git a/tests/unit/weak-map-test.js b/tests/unit/weak-map-test.ts similarity index 85% rename from tests/unit/weak-map-test.js rename to tests/unit/weak-map-test.ts index 3ade9df..6a4ae2e 100644 --- a/tests/unit/weak-map-test.js +++ b/tests/unit/weak-map-test.ts @@ -9,37 +9,41 @@ module('TrackedWeakMap', function(hooks) { setupRenderingTest(hooks); test('constructor', assert => { - let obj = {}; - let map = new TrackedWeakMap([[obj, 123]]); + const obj = {}; + const map = new TrackedWeakMap([[obj, 123]]); assert.equal(map.get(obj), 123); assert.ok(map instanceof WeakMap); }); test('does not work with built-ins', assert => { - let map = new TrackedWeakMap(); + const map = new TrackedWeakMap(); assert.throws( + // @ts-expect-error -- point is testing constructor error () => map.set('aoeu', 123), /Invalid value used as weak map key/ ); assert.throws( + // @ts-expect-error -- point is testing constructor error () => map.set(true, 123), /Invalid value used as weak map key/ ); assert.throws( + // @ts-expect-error -- point is testing constructor error () => map.set(123, 123), /Invalid value used as weak map key/ ); assert.throws( + // @ts-expect-error -- point is testing constructor error () => map.set(undefined, 123), /Invalid value used as weak map key/ ); }); test('get/set', assert => { - let obj = {}; - let map = new TrackedWeakMap(); + const obj = {}; + const map = new TrackedWeakMap(); map.set(obj, 123); assert.equal(map.get(obj), 123); @@ -49,8 +53,8 @@ module('TrackedWeakMap', function(hooks) { }); test('has', assert => { - let obj = {}; - let map = new TrackedWeakMap(); + const obj = {}; + const map = new TrackedWeakMap(); assert.equal(map.has(obj), false); map.set(obj, 123); @@ -58,8 +62,8 @@ module('TrackedWeakMap', function(hooks) { }); test('delete', assert => { - let obj = {}; - let map = new TrackedWeakMap(); + const obj = {}; + const map = new TrackedWeakMap(); assert.equal(map.has(obj), false); diff --git a/tests/unit/weak-set-test.js b/tests/unit/weak-set-test.ts similarity index 80% rename from tests/unit/weak-set-test.js rename to tests/unit/weak-set-test.ts index b01a480..0694db4 100644 --- a/tests/unit/weak-set-test.js +++ b/tests/unit/weak-set-test.ts @@ -9,33 +9,42 @@ module('TrackedWeakSet', function(hooks) { setupRenderingTest(hooks); test('constructor', assert => { - let obj = {}; - let set = new TrackedWeakSet([obj]); + const obj = {}; + const set = new TrackedWeakSet([obj]); assert.equal(set.has(obj), true); assert.ok(set instanceof WeakSet); + + const array = [1, 2, 3]; + const iterable = [array]; + const fromIterable = new TrackedWeakSet(iterable); + assert.equal(fromIterable.has(array), true); }); test('does not work with built-ins', assert => { - let set = new TrackedWeakSet(); + const set = new TrackedWeakSet(); + // @ts-expect-error -- point is testing constructor error assert.throws(() => set.add('aoeu'), /Invalid value used in weak set/); + // @ts-expect-error -- point is testing constructor error assert.throws(() => set.add(true), /Invalid value used in weak set/); + // @ts-expect-error -- point is testing constructor error assert.throws(() => set.add(123), /Invalid value used in weak set/); + // @ts-expect-error -- point is testing constructor error assert.throws(() => set.add(undefined), /Invalid value used in weak set/); }); test('add/has', assert => { - let obj = {}; - let set = new TrackedWeakSet(); + const obj = {}; + const set = new TrackedWeakSet(); set.add(obj); assert.equal(set.has(obj), true); }); test('delete', assert => { - let obj = {}; - let set = new TrackedWeakSet(); + const obj = {}; + const set = new TrackedWeakSet(); assert.equal(set.has(obj), false); diff --git a/tsconfig.json b/tsconfig.json index efb2d01..0bc4397 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,15 +1,10 @@ { "compilerOptions": { "target": "es2015", - "module": "es2015", "allowJs": true, "moduleResolution": "node", "allowSyntheticDefaultImports": true, - "noImplicitAny": true, - "noImplicitThis": true, - "alwaysStrict": true, - "strictNullChecks": true, - "strictPropertyInitialization": true, + "strict": true, "noFallthroughCasesInSwitch": true, "noUnusedLocals": true, "noUnusedParameters": true, @@ -18,9 +13,14 @@ "inlineSourceMap": true, "inlineSources": true, "experimentalDecorators": true, - "outDir": "addon" + "outDir": "addon", + "baseUrl": ".", + "paths": { + "dummy/tests/*": ["tests/*"], + "dummy/config/*": ["tests/dummy/config/*"], + "dummy/*": ["tests/dummy/app/*"], + "tracked-maps-and-sets": ["src"] + } }, - "include": [ - "src/**/*" - ] + "include": ["src/**/*", "tests/**/*"] } diff --git a/yarn.lock b/yarn.lock index 29dcb36..8f87023 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1561,6 +1561,13 @@ "@types/ember-test-helpers" "*" "@types/qunit" "*" +"@types/ember-resolver@^5.0.10": + version "5.0.10" + resolved "https://registry.yarnpkg.com/@types/ember-resolver/-/ember-resolver-5.0.10.tgz#bb265571a9cea9c4e6b095c8318df4e7f7837d5f" + integrity sha512-NTN4blgVUXi0wE6XTsuUjks15cI6em4sdTcgDlE8FvL8gkrxQFLSByyKDTBofHCncM6LwZcHz/eVVm13rwfC2w== + dependencies: + "@types/ember" "*" + "@types/ember-test-helpers@*": version "1.0.10" resolved "https://registry.yarnpkg.com/@types/ember-test-helpers/-/ember-test-helpers-1.0.10.tgz#cbd7f7e73a05f1c08075b99d4906cd5d92f443f0" @@ -5982,6 +5989,11 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2: dependencies: homedir-polyfill "^1.0.1" +expect-type@^0.13.0: + version "0.13.0" + resolved "https://registry.yarnpkg.com/expect-type/-/expect-type-0.13.0.tgz#916646a7a73f3ee77039a634ee9035efe1876eb2" + integrity sha512-CclevazQfrqo8EvbLPmP7osnb1SZXkw47XPPvUUpeMz4HuGzDltE7CaIt3RLyT9UQrwVK/LDn+KVcC0hcgjgDg== + express@^4.10.7, express@^4.17.1: version "4.17.1" resolved "https://registry.yarnpkg.com/express/-/express-4.17.1.tgz#4491fc38605cf51f8629d39c2b5d026f98a4c134"