From 3c2f19020c889351be0cd28c220b855ddd850801 Mon Sep 17 00:00:00 2001 From: ersimont <8042088+ersimont@users.noreply.github.com> Date: Tue, 14 Sep 2021 13:54:22 -0400 Subject: [PATCH] feat(ng-dev): `AsyncMethodController` no longer needs the `ctx` argument BREAKING CHANGE: The behavior for `AsyncMethodController` to automatically trigger promise handlers and change detection is now opt-out instead of opt-in. The `ctx` option for its constructor has been removed. If you are using an `AngularContext` and do _not_ want automatic calls to `.tick()` after each `.flush()` and `.error()`, pass a new option the constructor: `autoTick: false`. --- .../lib/spies/async-method-controller.spec.ts | 4 +- .../src/lib/spies/async-method-controller.ts | 10 ++--- .../ng-dev/src/lib/spies/test-call.spec.ts | 37 +++++++++++++++++-- projects/ng-dev/src/lib/spies/test-call.ts | 13 +++++-- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/projects/ng-dev/src/lib/spies/async-method-controller.spec.ts b/projects/ng-dev/src/lib/spies/async-method-controller.spec.ts index af58e1f1..28c6142e 100644 --- a/projects/ng-dev/src/lib/spies/async-method-controller.spec.ts +++ b/projects/ng-dev/src/lib/spies/async-method-controller.spec.ts @@ -384,9 +384,7 @@ describe('AsyncMethodController', () => { const ctx = new AngularContext(); // mock the browser API for pasting - const controller = new AsyncMethodController(clipboard, 'readText', { - ctx, - }); + const controller = new AsyncMethodController(clipboard, 'readText'); ctx.run(() => { // BEGIN production code that copies to the clipboard let pastedText: string; diff --git a/projects/ng-dev/src/lib/spies/async-method-controller.ts b/projects/ng-dev/src/lib/spies/async-method-controller.ts index ea3fd10a..7dcce58f 100644 --- a/projects/ng-dev/src/lib/spies/async-method-controller.ts +++ b/projects/ng-dev/src/lib/spies/async-method-controller.ts @@ -24,9 +24,7 @@ type AsyncMethodKeys = { * const ctx = new AngularContext(); * * // mock the browser API for pasting - * const controller = new AsyncMethodController(clipboard, 'readText', { - * ctx, - * }); + * const controller = new AsyncMethodController(clipboard, 'readText'); * ctx.run(() => { * // BEGIN production code that copies to the clipboard * let pastedText: string; @@ -53,18 +51,18 @@ export class AsyncMethodController< #testCalls: TestCall[] = []; /** - * Optionally provide `ctx` to automatically trigger promise handlers and changed detection after calling `flush()` or `error()`. This is the normal production behavior of asynchronous browser APIs. However, if the function you are stubbing is not patched by zone.js, change detection would not run automatically, in which case you many not want to pass this parameter. See the list of functions that zone.js patches [here](https://github.com/angular/angular/blob/master/packages/zone.js/STANDARD-APIS.md). + * If you are using an `AngularContext`, the default behavior is to automatically call `.tick()` after each `.flush()` and `.error()` to trigger promise handlers and changed detection. This is the normal production behavior of asynchronous browser APIs. However, if zone.js does not patch the function you are stubbing, change detection would not run automatically. In that case you many want to turn off this behavior by passing the option `autoTick: false`. See the list of functions that zone.js patches [here](https://github.com/angular/angular/blob/master/packages/zone.js/STANDARD-APIS.md). */ constructor( obj: WrappingObject, methodName: FunctionName, - { ctx = undefined as { tick(): void } | undefined } = {}, + { autoTick = true } = {}, ) { // Note: it wasn't immediately clear how avoid `any` in this constructor, and this will be invisible to users. So I gave up. (For now?) this.#spy = spyOn(obj, methodName as any) as any; this.#spy.and.callFake((() => { const deferred = new Deferred(); - this.#testCalls.push(new TestCall(deferred, ctx)); + this.#testCalls.push(new TestCall(deferred, autoTick)); return deferred.promise; }) as any); } diff --git a/projects/ng-dev/src/lib/spies/test-call.spec.ts b/projects/ng-dev/src/lib/spies/test-call.spec.ts index b170340e..cb7e2706 100644 --- a/projects/ng-dev/src/lib/spies/test-call.spec.ts +++ b/projects/ng-dev/src/lib/spies/test-call.spec.ts @@ -43,12 +43,11 @@ describe('TestCall', () => { expectSingleCallAndReset(spy, 'the clipboard text'); })); - it('triggers change detection if the AsyncMethodController was passed a context', () => { + it('triggers a tick if appropriate', () => { const ctx = new ComponentContext(TestComponent); const controller = new AsyncMethodController( navigator.clipboard, 'readText', - { ctx }, ); ctx.run(() => { @@ -78,12 +77,11 @@ describe('TestCall', () => { expectSingleCallAndReset(spy, 'some problem'); })); - it('triggers change detection if the AsyncMethodController was passed a context', () => { + it('triggers a tick if appropriate', () => { const ctx = new ComponentContext(TestComponent); const controller = new AsyncMethodController( navigator.clipboard, 'readText', - { ctx }, ); ctx.run(() => { @@ -96,4 +94,35 @@ describe('TestCall', () => { }); }); }); + + describe('.maybeTick()', () => { + it('does not call .tick() when autoTick is false', () => { + const ctx = new ComponentContext(TestComponent); + const controller = new AsyncMethodController( + navigator.clipboard, + 'readText', + { autoTick: false }, + ); + + ctx.run(() => { + navigator.clipboard.readText(); + ctx.getComponentInstance().name = 'Spy'; + controller.expectOne([]).flush('this is the clipboard content'); + + expect(ctx.fixture.nativeElement.textContent).not.toContain('Spy'); + }); + }); + + it('gracefully handles being run outside an AngularContext', () => { + const controller = new AsyncMethodController( + navigator.clipboard, + 'readText', + ); + + navigator.clipboard.readText(); + expect(() => { + controller.expectOne([]).flush('this is the clipboard content'); + }).not.toThrowError(); + }); + }); }); diff --git a/projects/ng-dev/src/lib/spies/test-call.ts b/projects/ng-dev/src/lib/spies/test-call.ts index c43f7393..1cd602eb 100644 --- a/projects/ng-dev/src/lib/spies/test-call.ts +++ b/projects/ng-dev/src/lib/spies/test-call.ts @@ -1,5 +1,6 @@ import { Deferred } from '@s-libs/js-core'; import { PromiseType } from 'utility-types'; +import { AngularContext } from '../angular-context'; /** * A mock method call that was made and is ready to be answered. This interface allows access to the underlying jasmine.CallInfo, and allows resolving or rejecting the asynchronous call's result. @@ -10,7 +11,7 @@ export class TestCall { constructor( private deferred: Deferred>>, - private ctx?: { tick(): void }, + private autoTick: boolean, ) {} /** @@ -18,7 +19,7 @@ export class TestCall { */ flush(value: PromiseType>): void { this.deferred.resolve(value); - this.ctx?.tick(); + this.maybeTick(); } /** @@ -26,6 +27,12 @@ export class TestCall { */ error(reason: any): void { this.deferred.reject(reason); - this.ctx?.tick(); + this.maybeTick(); + } + + private maybeTick(): void { + if (this.autoTick) { + AngularContext.getCurrent()?.tick(); + } } }