diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1977365..9504275 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,12 +64,18 @@ jobs: run_install: | - args: [--frozen-lockfile, --strict-peer-dependencies] - name: Unit tests - run: pnpm run test + env: + TEST: FULL + run: pnpm run test:full - name: Build run: pnpm run build - name: Post-build repo check run: git diff --exit-code - - name: Archive Build artifacts + - name: Coveralls + uses: coverallsapp/github-action@master + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Archive Build uses: actions/upload-artifact@v2 with: name: dist diff --git a/.gitignore b/.gitignore index 8114a1e..3e7f633 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ dist/ lib/ node_modules/ +coverage/ \ No newline at end of file diff --git a/jest.config.js b/jest.config.js index 678b0f3..f4e2272 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,4 +1,4 @@ -export default { +const baseConfig = { preset: 'ts-jest', testEnvironment: 'node', transform: { @@ -7,3 +7,35 @@ export default { testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.ts?$', moduleFileExtensions: ['js', 'ts'], }; + +const fullConfig = { + collectCoverage: true, + coverageDirectory: './coverage', + coverageThreshold: { + global: { + branches: 100, + functions: 100, + lines: 100, + statements: 100, + }, + }, + projects: [ + { + displayName: 'prod', + setupFiles: ['./tests/variants/prod.js'], + ...baseConfig, + }, + { + displayName: 'dev', + setupFiles: ['./tests/variants/dev.js'], + ...baseConfig, + }, + { + displayName: 'check', + setupFiles: ['./tests/variants/check.js'], + ...baseConfig, + }, + ], +}; + +export default process.env.TEST === 'FULL' ? fullConfig : baseConfig; diff --git a/package.json b/package.json index 4927d52..7665315 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,10 @@ "module": "dist/esm/probe-core.js", "unpkg": "dist/umd/probe-core.es6.js", "scripts": { - "test": "jest --verbose", + "test:full": "set TEST=FULL&& jest", + "test:quick": "jest", + "test:watch": "jest --watch", + "test": "pnpm run test:quick", "ci": "pnpm run lint", "tsc": "tsc", "lint": "eslint --ext .ts src/", diff --git a/src/Node.ts b/src/Node.ts index 77f953d..4d14dd3 100644 --- a/src/Node.ts +++ b/src/Node.ts @@ -53,6 +53,7 @@ export abstract class BaseNode implements IPNode { _onDispose?: DisposeOp[]; _buildData?: NodeBuildData; + _uniqueNodeId?: number; } export class NodeImpl extends BaseNode { diff --git a/src/Prober.ts b/src/Prober.ts index 33c060d..57568f2 100644 --- a/src/Prober.ts +++ b/src/Prober.ts @@ -6,16 +6,29 @@ const isIntrinsic = (cb: keyof I | ((...args: any[]) => any)): cb is keyof I return typeof cb === 'string'; }; +const addToDisposeQueue = (node: BaseNode, ops: DisposeOp[]) => { + if (!node._onDispose) { + node._onDispose = ops; + } else { + node._onDispose = node._onDispose.concat(ops); + } +}; + +let _NextUniqueNodeId = 0; + class Prober> implements IProber { private _intrinsics: I; private _queueHead?: BaseNode; private _insert?: BaseNode; - private _end?: BaseNode; - private _insertStack: (BaseNode | undefined)[] = []; - private _endStack: (BaseNode | undefined)[] = []; + private _end?: BaseNode; private _pendingOnDispose: DisposeOp[] = []; + private _finalizeStack: { + _end: BaseNode | undefined; + _pendingOnDispose: DisposeOp[]; + }[] = []; + _onDispose(op: DisposeOp): void { this._pendingOnDispose.push(op); } @@ -38,6 +51,10 @@ class Prober> implements IProber { } const newNode = new NodeImpl>(); + if (process.env.NODE_ENV !== 'production') { + newNode._uniqueNodeId = _NextUniqueNodeId++; + } + const _cb = this._getCb(what); let _next: IPNode | undefined; @@ -62,15 +79,14 @@ class Prober> implements IProber { _finalize(target: IPNode): void { // This can be called recursively, pushEnv(this); - this._endStack.push(this._end); - - // We need to loop until target and any node probed while compiling target have been finalized. - // this._end will be updated accordingly inside of announce() + this._finalizeStack.push({ _end: this._end, _pendingOnDispose: this._pendingOnDispose }); + this._pendingOnDispose = []; this._end = target; let currentNode: BaseNode; do { currentNode = this._queueHead!; + this._queueHead = currentNode._buildData!._next; // If a component returns a Node (as opposed to a value), then we short-circuit to the parent. let destinationNode = currentNode; @@ -86,7 +102,12 @@ class Prober> implements IProber { if (isPNode(cbResult)) { if (cbResult.finalized) { + // Post-ex-facto proxying. destinationNode._result = cbResult._result; + if (cbResult._onDispose) { + addToDisposeQueue(destinationNode, cbResult._onDispose); + cbResult._onDispose = []; + } } else { cbResult._buildData!._resolveAs = destinationNode; } @@ -95,21 +116,17 @@ class Prober> implements IProber { } if (this._pendingOnDispose.length > 0) { - if (!destinationNode._onDispose) { - destinationNode._onDispose = this._pendingOnDispose; - } else { - destinationNode._onDispose = destinationNode._onDispose.concat(this._pendingOnDispose); - } - + addToDisposeQueue(destinationNode, this._pendingOnDispose); this._pendingOnDispose = []; } - this._queueHead = currentNode._buildData!._next; currentNode._buildData = undefined; this._insert = this._insertStack.pop(); - } while (currentNode !== this._end); + } while (currentNode !== this._end && this._queueHead); - this._end = this._endStack.pop(); + const finalizePop = this._finalizeStack.pop()!; + this._end = finalizePop._end; + this._pendingOnDispose = finalizePop._pendingOnDispose; popEnv(); } diff --git a/src/dynamic/List.ts b/src/dynamic/List.ts index 10074db..2a91ed2 100644 --- a/src/dynamic/List.ts +++ b/src/dynamic/List.ts @@ -15,14 +15,7 @@ */ import { DynamicBaseImpl } from './Base'; -import { DynamicValueImpl } from './Value'; -import { ListValueType, DynamicValue, DynamicList } from '../ApiTypes'; - -interface MapCacheEntry { - value: U; - index: number; - indexProp?: DynamicValue; -} +import { ListValueType, DynamicList } from '../ApiTypes'; export class DynamicListImpl> extends DynamicBaseImpl implements DynamicList { push(v: ListValueType): void { @@ -31,36 +24,13 @@ export class DynamicListImpl> extends DynamicBaseImpl(cb: (value: ListValueType, index: number, array: T) => U): DynamicList { - let cache = new Map, MapCacheEntry>(); - const indexSensitive = cb.length >= 2; - const regenerate = (newArray: T): U[] => { - const newCache = new Map, MapCacheEntry>(); - // For loop instead of map, because this is confusing TypeScript. const result: U[] = []; const len = newArray.length; for (let i = 0; i < len; ++i) { - const v = newArray[i]; - const cacheEntry = cache.get(v); - if (cacheEntry) { - if (indexSensitive && i !== cacheEntry.index) { - cacheEntry.indexProp!.current = i; - } - newCache.set(v, cacheEntry); - result.push(cacheEntry.value); - } else { - const value = cb(v, i, newArray); - let indexProp: DynamicValue | undefined = undefined; - if (indexSensitive) { - indexProp = new DynamicValueImpl(i); - } - newCache.set(v, { value, index: i, indexProp }); - result.push(value); - } + result.push(cb(newArray[i], i, newArray)); } - - cache = newCache; return result; }; diff --git a/tests/dynamicList.test.ts b/tests/dynamicList.test.ts index cf4df76..4e07f47 100644 --- a/tests/dynamicList.test.ts +++ b/tests/dynamicList.test.ts @@ -60,4 +60,5 @@ describe('Dynamic Array', () => { x.current = [2, 2, 2]; expect(y.current).toEqual([4, 4, 4]); }); + }); diff --git a/tests/dynamicOperations.test.ts b/tests/dynamicOperations.test.ts new file mode 100644 index 0000000..5772184 --- /dev/null +++ b/tests/dynamicOperations.test.ts @@ -0,0 +1,69 @@ +/** + * Copyright 2021 Francois Chabot + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { DisposeOp, popEnv, pushEnv } from '../src/Environment'; + +import { dynamic, listen, valType } from '../src'; + +let disposeQueue: DisposeOp[] = []; +const cleanup = () => { + disposeQueue.forEach((v) => v()); + disposeQueue = []; +}; + +beforeEach(() => { + pushEnv({ + _onDispose: (op: DisposeOp) => disposeQueue.push(op), + }); +}); + +afterEach(() => { + cleanup(); + popEnv(); +}); + +describe('listen', () => { + it('Works on regular values', () => { + let y = 0; + const cb = (v: number) => (y += v); + const v = 12; + listen(v, cb); + expect(y).toBe(12); + }); + + it('Works on dynamic values', () => { + let y = 0; + const cb = (v: number) => (y += v); + const v = dynamic(12); + listen(v, cb); + expect(y).toBe(12); + + v.current = 13; + expect(y).toBe(25); + }); +}); + +describe('valType', () => { + it('Works on regular values', () => { + const v = 0; + expect(valType(v)).toBe('number'); + }); + + it('Works on dynamic values', () => { + const v = dynamic(0); + expect(valType(v)).toBe('number'); + }); +}); diff --git a/tests/dynamicVal.test.ts b/tests/dynamicVal.test.ts index e6139e5..087ca3c 100644 --- a/tests/dynamicVal.test.ts +++ b/tests/dynamicVal.test.ts @@ -73,9 +73,10 @@ describe('Dynamic Value', () => { const x = dynamic(12); let y = 0; - x.addListener(() => { + const listener = () => { y += 1; - }); + }; + x.addListener(listener); x.current = 23; expect(y).toBe(1); @@ -85,6 +86,17 @@ describe('Dynamic Value', () => { x.current = 23; expect(y).toBe(1); expect(x.valueOf()).toBe(23); + + // Setting a different value triggers notification again. + x.current = 24; + expect(y).toBe(2); + expect(x.valueOf()).toBe(24); + + // Removing the listener cancels notifications. + x.removeListener(listener); + x.current = 12; + expect(y).toBe(2); + expect(x.valueOf()).toBe(12); }); it('Cleans up correctly', () => { diff --git a/tests/environment.test.ts b/tests/environment.test.ts new file mode 100644 index 0000000..e6b57ef --- /dev/null +++ b/tests/environment.test.ts @@ -0,0 +1,66 @@ +import { popEnv, pushEnv, useOnDispose, Environment } from '../src/Environment'; +import { expectThrowInNotProd } from './utils'; + +const noop = () => { + // do nothing. +}; + +const pingDispose = () => { + useOnDispose(() => { + noop; + }); +}; + +describe('useOnDispose ', () => { + it('Fails if used out of context', () => { + expect(pingDispose).toThrow(); + }); +}); + +class TestEnv implements Environment { + count = 0; + _onDispose(): void { + this.count += 1; + } +} +describe('Environment', () => { + it('Catches underflows', () => { + expectThrowInNotProd(popEnv); + }); + + it('Registers', () => { + const env = new TestEnv(); + pushEnv(env); + pingDispose(); + + expect(env.count).toBe(1); + popEnv(); + + expectThrowInNotProd(pingDispose); + expectThrowInNotProd(popEnv); + }); + + it('Maintains the stack', () => { + const envA = new TestEnv(); + const envB = new TestEnv(); + pushEnv(envA); + pingDispose(); + pingDispose(); + + pushEnv(envB); + pingDispose(); + + expect(envA.count).toBe(2); + expect(envB.count).toBe(1); + popEnv(); + + pingDispose(); + pingDispose(); + expect(envA.count).toBe(4); + expect(envB.count).toBe(1); + + popEnv(); + expectThrowInNotProd(pingDispose); + expectThrowInNotProd(popEnv); + }); +}); diff --git a/tests/probe.test.ts b/tests/probe.test.ts index cda3fa6..c2d8272 100644 --- a/tests/probe.test.ts +++ b/tests/probe.test.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { probe, createProber, PNode, useOnDispose } from '../src'; +import { probe, createProber, PNode, useOnDispose, Component } from '../src'; +import { expectThrowInNotProd } from './utils'; describe('Basic prober', () => { it('Works with function without arguments', () => { @@ -36,47 +37,31 @@ describe('Basic prober', () => { }); it('Fails when using invalid CB', () => { - expect(() => { - //@ts-expect-error - probe(null); - }).toThrow(); - - expect(() => { - //@ts-ignore - probe(undefined); - }).toThrow(); - - expect(() => { - //@ts-expect-error - probe(12); - }).toThrow(); - - expect(() => { - //@ts-expect-error - probe(true); - }).toThrow(); - - expect(() => { - //@ts-expect-error - probe([]); - }).toThrow(); - - expect(() => { - //@ts-expect-error - probe({}); - }).toThrow(); + //@ts-expect-error + expectThrowInNotProd(() => probe(null)); + + //@ts-expect-error + expectThrowInNotProd(() => probe(undefined)); + + //@ts-expect-error + expectThrowInNotProd(() => probe(12)); + + //@ts-expect-error + expectThrowInNotProd(() => probe(true)); + + //@ts-expect-error + expectThrowInNotProd(() => probe([])); + + //@ts-expect-error + expectThrowInNotProd(() => probe({})); }); it('Fails when using an intrinsic', () => { - expect(() => { - //@ts-expect-error - probe('yo', {}); - }).toThrow(); - - expect(() => { - //@ts-expect-error - probe('', {}); - }).toThrow(); + //@ts-expect-error + expectThrowInNotProd(() => probe('yo', {})); + + //@ts-expect-error + expectThrowInNotProd(() => probe('', {})); }); }); @@ -95,15 +80,17 @@ describe('Prober with intrinsics', () => { }); it('Fails when using wrong intrinsic', () => { - expect(() => { - //@ts-expect-error - sutProbe('aab', {}); - }).toThrow(); - - expect(() => { - //@ts-expect-error - sutProbe('', {}); - }).toThrow(); + //@ts-expect-error + expectThrowInNotProd(() => sutProbe('xyz', {})); + + //@ts-expect-error + expectThrowInNotProd(() => sutProbe('aa', {})); + + //@ts-expect-error + expectThrowInNotProd(() => sutProbe('aaaa', {})); + + //@ts-expect-error + expectThrowInNotProd(() => sutProbe('', {})); }); }); @@ -131,6 +118,17 @@ describe('Component With dispose', () => { }); }); +describe('Component with no dispose', () => { + const component = (x: number) => x * x; + + it('Disposing is harmless', () => { + const node = probe(component, 2); + expect(node.result).toBe(4); + + node.dispose(); + }); +}); + describe('Hierarchical components', () => { const Leaf = () => { return 3; @@ -150,3 +148,69 @@ describe('Hierarchical components', () => { expect(probe(Root).result).toBe(9); }); }); + +describe('Proxy components', () => { + let disposed = 0; + + const Sub = (v: number) => { + useOnDispose(() => (disposed += 2)); + return v * v; + }; + + const Parent = (v: number) => { + useOnDispose(() => (disposed += 3)); + return probe(Sub, v); + }; + + const node = probe(Parent, 4); + const result = node.result; + + node.dispose(); + + it('Produced the correct value', () => { + expect(result).toBe(16); + }); + + it('Disposed correctly', () => { + expect(disposed).toBe(5); + }); +}); + +describe('Pre-finalized components', () => { + let disposed = 0; + + const Sub = (v: number) => { + useOnDispose(() => (disposed += 2)); + return v * v; + }; + + const HarmlessSub = (v: number) => { + return v * v; + }; + + const Parent = (v: number, c: Component<[number], T>) => { + useOnDispose(() => (disposed += 3)); + const subNode = probe(c, v); + subNode.finalize(); + + return subNode; + }; + + const node = probe(Parent, 4, Sub); + const harmlessNode = probe(Parent, 5, HarmlessSub); + + const result = node.result; + const harmlessResult = harmlessNode.result; + + node.dispose(); + harmlessNode.dispose(); + + it('Produced the correct value', () => { + expect(result).toBe(16); + expect(harmlessResult).toBe(25); + }); + + it('Disposed correctly', () => { + expect(disposed).toBe(8); + }); +}); diff --git a/tests/utils.ts b/tests/utils.ts new file mode 100644 index 0000000..273b933 --- /dev/null +++ b/tests/utils.ts @@ -0,0 +1,7 @@ +export const expectThrowInNotProd = (cb: () => void) => { + if (process.env.NODE_ENV === 'production') { + //expect(cb).not.toThrow(); + } else { + expect(cb).toThrow(); + } +}; diff --git a/tests/variants/check.js b/tests/variants/check.js new file mode 100644 index 0000000..0494743 --- /dev/null +++ b/tests/variants/check.js @@ -0,0 +1 @@ +process.env.NODE_ENV = 'check'; diff --git a/tests/variants/dev.js b/tests/variants/dev.js new file mode 100644 index 0000000..010a278 --- /dev/null +++ b/tests/variants/dev.js @@ -0,0 +1 @@ +process.env.NODE_ENV = 'development'; diff --git a/tests/variants/prod.js b/tests/variants/prod.js new file mode 100644 index 0000000..f16904e --- /dev/null +++ b/tests/variants/prod.js @@ -0,0 +1 @@ +process.env.NODE_ENV = 'production';