From 51d22e592f55286ebf230b1c55ad865a1bbfead7 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Fri, 7 Jul 2017 09:43:38 +0100 Subject: [PATCH 1/2] Changes to align with shim refactors --- src/IdentityRegistry.ts | 32 ++++----- src/MultiMap.ts | 3 +- src/aspect.ts | 6 +- src/lang.ts | 21 +----- src/request/providers/node.ts | 14 ++-- src/request/providers/xhr.ts | 14 ++-- tests/support/util.ts | 7 +- tests/unit/lang.ts | 119 +--------------------------------- tsconfig.json | 1 + 9 files changed, 41 insertions(+), 176 deletions(-) diff --git a/src/IdentityRegistry.ts b/src/IdentityRegistry.ts index 771b1e91..a89b1226 100644 --- a/src/IdentityRegistry.ts +++ b/src/IdentityRegistry.ts @@ -12,15 +12,15 @@ interface Entry { readonly value: V; } -interface State { +interface State { readonly entryMap: Map>; readonly idMap: WeakMap; } const privateStateMap = new WeakMap, State>(); -function getState(instance: IdentityRegistry): State { - return privateStateMap.get(instance); +function getState(instance: IdentityRegistry): State { + return privateStateMap.get(instance)!; } /** @@ -31,7 +31,7 @@ export type Identity = string | symbol; /** * A registry of values, mapped by identities. */ -export default class IdentityRegistry implements Iterable<[Identity, V]> { +export default class IdentityRegistry implements Iterable<[Identity, V]> { constructor() { privateStateMap.set(this, { entryMap: new Map>(), @@ -48,7 +48,7 @@ export default class IdentityRegistry implements Iterable<[Ide * @return The value */ get(id: Identity): V { - const entry = getState(this).entryMap.get(id); + const entry = getState(this).entryMap.get(id); if (!entry) { throw new Error(`Could not find a value for identity '${id.toString()}'`); } @@ -62,7 +62,7 @@ export default class IdentityRegistry implements Iterable<[Ide * @return `true` if the value has been registered, `false` otherwise */ contains(value: V): boolean { - return getState(this).idMap.has(value); + return getState(this).idMap.has(value); } /** @@ -71,7 +71,7 @@ export default class IdentityRegistry implements Iterable<[Ide * @return `true` if the value was removed, `false` otherwise */ delete(id: Identity): boolean { - const entry = getState(this).entryMap.get(id); + const entry = getState(this).entryMap.get(id); if (!entry) { return false; } @@ -86,7 +86,7 @@ export default class IdentityRegistry implements Iterable<[Ide * @return `true` if a value has been registered, `false` otherwise */ has(id: Identity): boolean { - return getState(this).entryMap.has(id); + return getState(this).entryMap.has(id); } /** @@ -97,12 +97,12 @@ export default class IdentityRegistry implements Iterable<[Ide * @param value The value * @return The identifier otherwise */ - identify(value: V): Identity { + identify(value: V): Identity | undefined { if (!this.contains(value)) { throw new Error('Could not identify non-registered value'); } - return getState(this).idMap.get(value); + return getState(this).idMap.get(value); } /** @@ -126,7 +126,7 @@ export default class IdentityRegistry implements Iterable<[Ide const existingId = this.contains(value) ? this.identify(value) : null; if (existingId && existingId !== id) { - const str = ( existingId).toString(); + const str = existingId.toString(); throw new Error(`The value has already been registered with a different identity (${str})`); } @@ -138,12 +138,12 @@ export default class IdentityRegistry implements Iterable<[Ide const handle = { destroy: () => { handle.destroy = noop; - getState(this).entryMap.delete(id); + getState(this).entryMap.delete(id); } }; entryMap.set(id, { handle, value }); - getState(this).idMap.set(value, id); + getState(this).idMap.set(value, id); return handle; } @@ -151,7 +151,7 @@ export default class IdentityRegistry implements Iterable<[Ide entries(): IterableIterator<[Identity, V]> { const values = new List<[Identity, V]>(); - getState(this).entryMap.forEach((value: Entry, key: Identity) => { + getState(this).entryMap.forEach((value: Entry, key: Identity) => { values.add([key, value.value]); }); @@ -161,7 +161,7 @@ export default class IdentityRegistry implements Iterable<[Ide values(): IterableIterator { const values = new List(); - getState(this).entryMap.forEach((value: Entry, key: Identity) => { + getState(this).entryMap.forEach((value: Entry, key: Identity) => { values.add(value.value); }); @@ -169,7 +169,7 @@ export default class IdentityRegistry implements Iterable<[Ide } ids(): IterableIterator { - return getState(this).entryMap.keys(); + return getState(this).entryMap.keys(); } [Symbol.iterator](): IterableIterator<[Identity, V]> { diff --git a/src/MultiMap.ts b/src/MultiMap.ts index fb58a5c7..42a50002 100644 --- a/src/MultiMap.ts +++ b/src/MultiMap.ts @@ -1,6 +1,5 @@ import '@dojo/shim/Symbol'; import Map from '@dojo/shim/Map'; -import { ArrayLike } from '@dojo/shim/interfaces'; import { from as arrayFrom } from '@dojo/shim/array'; import { forOf, Iterable, IterableIterator, ShimIterator } from '@dojo/shim/iterator'; @@ -36,7 +35,7 @@ export default class MultiMap implements Map { * * @return the multi map instance */ - set(keys: any[], value: T): MultiMap { + set(keys: any[], value: T): this { let map = this._map; let childMap; diff --git a/src/aspect.ts b/src/aspect.ts index cfcce12a..9d197404 100644 --- a/src/aspect.ts +++ b/src/aspect.ts @@ -191,7 +191,7 @@ function adviseJoinPoint, T>(this: any, joinPoint: } else { dispatcher = getJoinPointDispatcher(joinPoint); - const adviceMap = dispatchAdviceMap.get(dispatcher); + const adviceMap = dispatchAdviceMap.get(dispatcher)!; if (type === 'before') { (adviceMap.before || (adviceMap.before = [])).unshift( advice); } @@ -281,7 +281,7 @@ function getDispatcherObject(target: Targetable, methodName: string): Dispatcher function getJoinPointDispatcher, T>(joinPoint: F): F { function dispatcher(this: Function, ...args: any[]): T { - const { before, after, joinPoint } = dispatchAdviceMap.get(dispatcher); + const { before, after, joinPoint } = dispatchAdviceMap.get(dispatcher)!; if (before) { args = before.reduce((previousArgs, advice) => { const currentArgs = advice.apply(this, previousArgs); @@ -300,7 +300,7 @@ function getJoinPointDispatcher, T>(joinPoint: F): /* We want to "clone" the advice that has been applied already, if this * joinPoint is already advised */ if (dispatchAdviceMap.has(joinPoint)) { - const adviceMap = dispatchAdviceMap.get(joinPoint); + const adviceMap = dispatchAdviceMap.get(joinPoint)!; let { before, after } = adviceMap; if (before) { before = before.slice(0); diff --git a/src/lang.ts b/src/lang.ts index 45982434..dd556e5c 100644 --- a/src/lang.ts +++ b/src/lang.ts @@ -1,5 +1,6 @@ -import has from './has'; import { Handle } from '@dojo/interfaces/core'; +import { assign } from '@dojo/shim/object'; +export { assign } from '@dojo/shim/object'; const slice = Array.prototype.slice; const hasOwnProperty = Object.prototype.hasOwnProperty; @@ -93,24 +94,6 @@ interface ObjectAssignConstructor extends ObjectConstructor { assign(target: any, ...sources: any[]): any; } -/** - * Copies the values of all enumerable own properties of one or more source objects to the target object. - * - * @param target The target object to receive values from source objects - * @param sources Any number of objects whose enumerable own properties will be copied to the target object - * @return The modified target object - */ -export const assign = has('object-assign') ? - ( Object).assign : - function (target: any, ...sources: any[]): any { - return _mixin({ - deep: false, - inherited: false, - sources: sources, - target: target - }); - }; - /** * Creates a new object from the given prototype, and copies all enumerable own properties of one or more * source objects to the newly created target object. diff --git a/src/request/providers/node.ts b/src/request/providers/node.ts index 35252283..17364672 100644 --- a/src/request/providers/node.ts +++ b/src/request/providers/node.ts @@ -111,7 +111,7 @@ const discardedDuplicates = new Set([ ]); function getDataTask(response: NodeResponse): Task { - const data = dataMap.get(response); + const data = dataMap.get(response)!; if (data.used) { return Task.reject(new TypeError('Body already read')); @@ -134,27 +134,27 @@ export class NodeResponse extends Response { downloadBody = true; get bodyUsed(): boolean { - return dataMap.get(this).used; + return dataMap.get(this)!.used; } get nativeResponse(): http.IncomingMessage { - return dataMap.get(this).nativeResponse; + return dataMap.get(this)!.nativeResponse; } get requestOptions(): NodeRequestOptions { - return dataMap.get(this).requestOptions; + return dataMap.get(this)!.requestOptions; } get url(): string { - return dataMap.get(this).url; + return dataMap.get(this)!.url; } get download(): Observable { - return dataMap.get(this).downloadObservable; + return dataMap.get(this)!.downloadObservable; } get data(): Observable { - return dataMap.get(this).dataObservable; + return dataMap.get(this)!.dataObservable; } constructor(response: http.IncomingMessage) { diff --git a/src/request/providers/xhr.ts b/src/request/providers/xhr.ts index 3d17b3ae..4d6245aa 100644 --- a/src/request/providers/xhr.ts +++ b/src/request/providers/xhr.ts @@ -33,7 +33,7 @@ interface RequestData { const dataMap = new WeakMap(); function getDataTask(response: XhrResponse): Task { - const data = dataMap.get(response); + const data = dataMap.get(response)!; if (data.used) { return Task.reject(new TypeError('Body already read')); @@ -54,27 +54,27 @@ export class XhrResponse extends Response { readonly statusText: string; get bodyUsed(): boolean { - return dataMap.get(this).used; + return dataMap.get(this)!.used; } get nativeResponse(): XMLHttpRequest { - return dataMap.get(this).nativeResponse; + return dataMap.get(this)!.nativeResponse; } get requestOptions(): XhrRequestOptions { - return dataMap.get(this).requestOptions; + return dataMap.get(this)!.requestOptions; } get url(): string { - return dataMap.get(this).url; + return dataMap.get(this)!.url; } get download(): Observable { - return dataMap.get(this).downloadObservable; + return dataMap.get(this)!.downloadObservable; } get data(): Observable { - return dataMap.get(this).dataObservable; + return dataMap.get(this)!.dataObservable; } constructor(request: XMLHttpRequest) { diff --git a/tests/support/util.ts b/tests/support/util.ts index 1b679c9f..47326166 100644 --- a/tests/support/util.ts +++ b/tests/support/util.ts @@ -1,8 +1,5 @@ -import { Thenable } from '@dojo/shim/interfaces'; -export { Thenable } from '@dojo/shim/interfaces'; - -export function isEventuallyRejected(promise: Thenable): Thenable { - return promise.then(function () { +export function isEventuallyRejected(promise: PromiseLike): PromiseLike { + return promise.then(function () { throw new Error('unexpected code path'); }, function () { return true; // expect rejection diff --git a/tests/unit/lang.ts b/tests/unit/lang.ts index bb13205e..f34fd8f1 100644 --- a/tests/unit/lang.ts +++ b/tests/unit/lang.ts @@ -6,123 +6,8 @@ registerSuite({ name: 'lang functions', '.assign()'() { - const source: { - a: number - b: { - enumerable: boolean, - configurable: boolean, - writable: boolean, - value: number - } - } = Object.create({ a: 1 }, { - b: { - enumerable: false, - configurable: true, - writable: true, - value: 2 - } - }); - ( source).c = 3; - ( source).nested = { a: 5 }; - - const object: { - c: number, - nested: { - a: number - } - } = Object.create(null); - const assignedObject: typeof object & typeof source = lang.assign(object, source); - - assert.strictEqual(object, assignedObject, 'assign should return the modified target object'); - assert.isUndefined(assignedObject.a, 'assign should not copy inherited properties'); - assert.isUndefined(assignedObject.b, 'assign should not copy non-enumerable properties'); - assert.strictEqual(assignedObject.c, 3); - assert.strictEqual(assignedObject.nested, ( source).nested, 'assign should perform a shallow copy'); - assert.strictEqual(assignedObject.nested.a, 5); - }, - - '.assign() with multiple sources'() { - let source1 = { - property3: 'value3', - property4: 'value4' - }; - - let source3 = { - property7: 'value7', - property8: 'value8' - }; - - const object = { - property1: 'value1', - property2: 'value2' - }; - - lang.assign(object, source1, null, source3); - - assert.deepEqual(object, { - property1: 'value1', - property2: 'value2', - property3: 'value3', - property4: 'value4', - property7: 'value7', - property8: 'value8' - }); - }, - - '.assign() with inferred type from multiple sources'() { - let source1: { a: number, b: number } | { c: number, d: number } = { - a: 1, - b: 2, - c: 3, - d: 4 - }; - - let source2 = { - a: 3, - b: 2 - }; - - let source3 = { - c: 3, - d: 4 - }; - - const object = {}; - - const assignedObject = lang.assign(object, source1, source2, source3); - - assert(assignedObject); - - // Verify that the inferred type is what we expect - const alsoAssigned: {} & ({ a: number, b: number } | { c: number, d: number }) = lang.assign(object, source1, source2, source3); - - assert(alsoAssigned); - }, - - '.assign() with different types of sources'() { - const baseObject = { - foo: 'foo' - }; - - const assignedObject = lang.assign({}, baseObject, { bar: 'bar' }); - assert.strictEqual(assignedObject.bar, 'bar'); - assert.strictEqual(assignedObject.foo, 'foo'); - }, - '.assign() with a source whose type is a subset of another'() { - type FooBar = { - foo: string; - bar: string; - }; - const foobar = { - foo: 'foo', - bar: 'bar' - }; - const bar = { - bar: 'baz' - }; - const assignedObject = lang.assign({}, foobar, bar); - assert.strictEqual(assignedObject.foo, 'foo'); - assert.strictEqual(assignedObject.bar, 'baz'); + // this is a re-export from `@dojo/shim/object::assign` + assert.isFunction(lang.assign); }, '.deepAssign()'() { diff --git a/tsconfig.json b/tsconfig.json index eb380cd8..fd3d9812 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,6 +6,7 @@ "dom", "es5", "es2015.iterable", + "es2015.promise", "es2015.symbol", "es2015.symbol.wellknown" ], From 1310a43d8c2c79f2c3525f744f4a620fa32ae966 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Wed, 26 Jul 2017 18:59:38 +0100 Subject: [PATCH 2/2] Code comments about use of not null assertions. [ci skip] --- src/aspect.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/aspect.ts b/src/aspect.ts index 9d197404..57cc3887 100644 --- a/src/aspect.ts +++ b/src/aspect.ts @@ -191,6 +191,7 @@ function adviseJoinPoint, T>(this: any, joinPoint: } else { dispatcher = getJoinPointDispatcher(joinPoint); + // cannot have undefined in map due to code logic, using ! const adviceMap = dispatchAdviceMap.get(dispatcher)!; if (type === 'before') { (adviceMap.before || (adviceMap.before = [])).unshift( advice); @@ -281,6 +282,7 @@ function getDispatcherObject(target: Targetable, methodName: string): Dispatcher function getJoinPointDispatcher, T>(joinPoint: F): F { function dispatcher(this: Function, ...args: any[]): T { + // cannot have undefined in map due to code logic, using ! const { before, after, joinPoint } = dispatchAdviceMap.get(dispatcher)!; if (before) { args = before.reduce((previousArgs, advice) => { @@ -300,6 +302,7 @@ function getJoinPointDispatcher, T>(joinPoint: F): /* We want to "clone" the advice that has been applied already, if this * joinPoint is already advised */ if (dispatchAdviceMap.has(joinPoint)) { + // cannot have undefined in map due to code logic, using ! const adviceMap = dispatchAdviceMap.get(joinPoint)!; let { before, after } = adviceMap; if (before) {