Skip to content

Commit

Permalink
Fix: accept Iterable in TrackedMap and TrackedSet
Browse files Browse the repository at this point in the history
- 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)
  • Loading branch information
chriskrycho committed Oct 25, 2021
1 parent 328fc6d commit bed0d1a
Show file tree
Hide file tree
Showing 14 changed files with 176 additions and 76 deletions.
9 changes: 9 additions & 0 deletions config/tsconfig.addon.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
9 changes: 9 additions & 0 deletions config/tsconfig.cjs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "../tsconfig.json",
"include": ["../src"],
"compilerOptions": {
"outDir": "../commonjs",
"declaration": true,
"module": "CommonJS"
}
}
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 .",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
33 changes: 25 additions & 8 deletions src/-private/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,19 @@ export class TrackedMap<K = unknown, V = unknown> implements Map<K, V> {
}
}

constructor(entries?: readonly (readonly [K, V])[] | null) {
this.vals = new Map(entries);
constructor();
constructor(entries: readonly (readonly [K, V])[] | null);
constructor(iterable: Iterable<readonly [K, V]>);
constructor(
existing?:
| readonly (readonly [K, V])[]
| Iterable<readonly [K, V]>
| 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 ****
Expand Down Expand Up @@ -120,10 +131,9 @@ export class TrackedMap<K = unknown, V = unknown> implements Map<K, V> {
// So instanceof works
Object.setPrototypeOf(TrackedMap.prototype, Map.prototype);

export class TrackedWeakMap<
K extends object = object,
V = unknown
> implements WeakMap<K, V> {
export class TrackedWeakMap<K extends object = object, V = unknown>
implements WeakMap<K, V>
{
private storages: WeakMap<K, TrackedStorage<null>> = new WeakMap();

private vals: WeakMap<K, V>;
Expand All @@ -148,8 +158,15 @@ export class TrackedWeakMap<
}
}

constructor(iterable?: (readonly [K, V][] | null)) {
this.vals = new WeakMap(iterable);
constructor();
constructor(iterable: Iterable<readonly [K, V]>);
constructor(entries: readonly [K, V][] | null);
constructor(
existing?: readonly [K, V][] | Iterable<readonly [K, V]> | 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 {
Expand Down
19 changes: 11 additions & 8 deletions src/-private/set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ export class TrackedSet<T = unknown> implements Set<T> {
}
}

constructor(values?: readonly T[] | null) {
this.vals = new Set(values);
constructor();
constructor(values: readonly T[] | null);
constructor(iterable: Iterable<T>);
constructor(existing?: readonly T[] | Iterable<T> | null | undefined) {
this.vals = new Set(existing);
}

// **** KEY GETTERS ****
Expand All @@ -45,31 +48,31 @@ export class TrackedSet<T = unknown> implements Set<T> {

// **** ALL GETTERS ****
entries(): IterableIterator<[T, T]> {
getValue(this.collection)
getValue(this.collection);

return this.vals.entries();
}

keys(): IterableIterator<T> {
getValue(this.collection)
getValue(this.collection);

return this.vals.keys();
}

values(): IterableIterator<T> {
getValue(this.collection)
getValue(this.collection);

return this.vals.values();
}

forEach(fn: (value1: T, value2: T, set: Set<T>) => 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;
}
Expand All @@ -86,7 +89,7 @@ export class TrackedSet<T = unknown> implements Set<T> {

// **** KEY SETTERS ****
add(value: T): this {
this.dirtyStorageFor(value)
this.dirtyStorageFor(value);
setValue(this.collection, null);

this.vals.add(value);
Expand Down
2 changes: 1 addition & 1 deletion tests/dummy/app/app.js → tests/dummy/app/app.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ declare const config: {
podModulePrefix: string;
locationType: string;
rootURL: string;
APP: Record<string, unknown>;
};
4 changes: 2 additions & 2 deletions tests/test-helper.js → tests/test-helper.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
37 changes: 21 additions & 16 deletions tests/unit/map-test.js → tests/unit/map-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,30 @@ 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<TrackedMap<string, number>>().toMatchTypeOf<Map<string, number>>();
expectTypeOf<Map<string, number>>().not.toMatchTypeOf<TrackedMap<string, number>>();

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);
assert.ok(map instanceof Map);
});

test('works with all kinds of keys', assert => {
let map = new TrackedMap([
const map = new TrackedMap<unknown, unknown>([
['foo', 123],
[{}, {}],
[() => {}, 'bar'],
[() => {/* no op! */}, 'bar'],
[123, true],
[true, false],
[null, null],
Expand All @@ -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);
Expand All @@ -41,20 +46,20 @@ 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);
assert.equal(map.has('foo'), true);
});

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]);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -263,7 +268,7 @@ module('TrackedMap', function(hooks) {
map = new TrackedMap();

get value() {
this.map.forEach(() => {});
this.map.forEach(() => {/* no op! */});
return 'test';
}

Expand Down
Loading

0 comments on commit bed0d1a

Please sign in to comment.