fix: clean up change detection runs

nartc · nartc · commit 4ebddbb0f722 · 2023-03-06T15:57:36.000-06:00
diff --git a/libs/angular-three/src/lib/canvas.ts b/libs/angular-three/src/lib/canvas.ts
@@ -26,6 +26,7 @@ import { NgtRxStore } from './stores/rx-store';
 import { NgtStore, rootStateMap } from './stores/store';
 import type { NgtAnyRecord, NgtCanvasInputs, NgtDomEvent, NgtDpr, NgtState } from './types';
 import { is } from './utils/is';
+import { safeDetectChanges } from './utils/safe-detect-changes';
 import { createPointerEvents } from './web/events';
 
 @Component({
@@ -162,7 +163,7 @@ export class NgtCanvas extends NgtRxStore<NgtCanvasInputs> implements OnInit, On
             this.store.set({
                 onPointerMissed: (event: MouseEvent) => {
                     this.pointerMissed.emit(event);
-                    this.cdr.detectChanges();
+                    safeDetectChanges(this.cdr);
                 },
             });
         }
@@ -210,24 +211,20 @@ export class NgtCanvas extends NgtRxStore<NgtCanvasInputs> implements OnInit, On
         // emit created event if observed
         if (this.created.observed) {
             this.created.emit(this.store.get());
-            this.cdr.detectChanges();
         }
 
         // render
         if (this.glRef) this.glRef.destroy();
 
         requestAnimationFrame(() => {
+            const { store, cdr: changeDetectorRef, compoundPrefixes } = this;
             this.glEnvInjector = createEnvironmentInjector(
-                [
-                    provideNgtRenderer({
-                        store: this.store,
-                        changeDetectorRef: this.cdr,
-                        compoundPrefixes: this.compoundPrefixes,
-                    }),
-                ],
+                [provideNgtRenderer({ store, changeDetectorRef, compoundPrefixes })],
                 this.envInjector
             );
             this.glRef = this.glAnchor.createComponent(this.sceneGraph, { environmentInjector: this.glEnvInjector });
+            // detach the Scene graph from Change Detection mechanism
+            // everything from this point forward will trigger CD manually with detectChanges
             this.glRef.changeDetectorRef.detach();
             this.setSceneGraphInputs();
 
@@ -249,14 +246,14 @@ export class NgtCanvas extends NgtRxStore<NgtCanvasInputs> implements OnInit, On
         const originalDetectChanges = this.cdr.detectChanges.bind(this.cdr);
         this.cdr.detectChanges = () => {
             originalDetectChanges();
-            this.glRef?.changeDetectorRef.detectChanges();
+safeDetectChanges(this.glRef?.changeDetectorRef);
         };
     }
 
     private setSceneGraphInputs() {
         for (const key of Object.keys(this.sceneGraphInputs)) {
-            this.glRef?.setInput(key, this.sceneGraphInputs[key]);
+            this.glRef!.setInput(key, this.sceneGraphInputs[key]);
         }
-        this.cdr.detectChanges();
+        safeDetectChanges(this.cdr);
     }
 }
diff --git a/libs/angular-three/src/lib/di/ref.ts b/libs/angular-three/src/lib/di/ref.ts
@@ -53,6 +53,8 @@ export function injectNgtRef<T>(initialValue: NgtInjectedRef<T> | (T | null) = n
         });
     };
 
+    const useCDR = (cdr: ChangeDetectorRef) => void cdRefs.push(cdr);
+
     const $ = obs$.pipe(
         filter((value, index) => index > 0 || value != null),
         takeUntil(destroy$)
@@ -83,6 +85,7 @@ export function injectNgtRef<T>(initialValue: NgtInjectedRef<T> | (T | null) = n
             takeUntil(destroy$)
         );
 
+    // here, we override nativeElement to  add more functionalities to nativeElement
     Object.defineProperty(ref, 'nativeElement', {
         set: (newVal: T) => {
             if (ref.nativeElement !== newVal) {
@@ -110,5 +113,5 @@ export function injectNgtRef<T>(initialValue: NgtInjectedRef<T> | (T | null) = n
         get: () => ref$.value,
     });
 
-    return Object.assign(ref, { subscribe, $, children$, useCDR: (cdr: ChangeDetectorRef) => void cdRefs.push(cdr) });
+    return Object.assign(ref, { subscribe, $, children$, useCDR });
 }
diff --git a/libs/angular-three/src/lib/di/run-in-context.ts b/libs/angular-three/src/lib/di/run-in-context.ts
@@ -1,5 +1,10 @@
 import { EnvironmentInjector, inject, Injector } from '@angular/core';
 
+/**
+ * Please use this sparringly and know what you're doing.
+ *
+ * Create a runInContext function that has access to the NodeInjector as well
+ */
 export function createRunInContext() {
     const nodeInjector = inject(Injector);
     const envInjector = inject(EnvironmentInjector);
diff --git a/libs/angular-three/src/lib/directives/common.ts b/libs/angular-three/src/lib/directives/common.ts
@@ -1,4 +1,5 @@
 import { Directive, EmbeddedViewRef, inject, TemplateRef, ViewContainerRef } from '@angular/core';
+import { safeDetectChanges } from '../utils/safe-detect-changes';
 
 @Directive()
 export abstract class NgtCommonDirective {
@@ -25,7 +26,7 @@ export abstract class NgtCommonDirective {
                 this.view.destroy();
             }
             this.view = this.vcr.createEmbeddedView(this.template);
-            this.view.detectChanges();
+            safeDetectChanges(this.view);
         }
     }
 }
diff --git a/libs/angular-three/src/lib/loader.ts b/libs/angular-three/src/lib/loader.ts
@@ -17,6 +17,7 @@ import {
 import type { GLTF } from 'three/examples/jsm/loaders/GLTFLoader';
 import type { NgtBranchingReturn, NgtLoaderExtensions, NgtLoaderResult, NgtObjectMap } from './types';
 import { makeObjectGraph } from './utils/make';
+import { safeDetectChanges } from './utils/safe-detect-changes';
 
 interface NgtLoader {
     <TReturnType, TUrl extends string | string[] | Record<string, string>>(
@@ -46,15 +47,13 @@ export type NgtLoaderResults<
 
 const cached = new Map<string, Observable<any>>();
 
-function injectLoader<TReturnType, TUrl extends string | string[] | Record<string, string>>(
+function load<TReturnType, TUrl extends string | string[] | Record<string, string>>(
     loaderConstructorFactory: (inputs: TUrl) => new (...args: any[]) => NgtLoaderResult<TReturnType>,
     input: TUrl | Observable<TUrl>,
     extensions?: NgtLoaderExtensions,
     onProgress?: (event: ProgressEvent) => void
-): Observable<NgtLoaderResults<TUrl, NgtBranchingReturn<TReturnType, GLTF, GLTF & NgtObjectMap>>> {
+) {
     const urls$ = isObservable(input) ? input : of(input);
-    const cdr = inject(ChangeDetectorRef);
-
     return urls$.pipe(
         map((inputs) => {
             const loaderConstructor = loaderConstructorFactory(inputs);
@@ -83,7 +82,19 @@ function injectLoader<TReturnType, TUrl extends string | string[] | Record<strin
                 }),
                 inputs,
             ] as [Array<Observable<any>>, TUrl | TUrl[]];
-        }),
+        })
+    );
+}
+
+function injectLoader<TReturnType, TUrl extends string | string[] | Record<string, string>>(
+    loaderConstructorFactory: (inputs: TUrl) => new (...args: any[]) => NgtLoaderResult<TReturnType>,
+    input: TUrl | Observable<TUrl>,
+    extensions?: NgtLoaderExtensions,
+    onProgress?: (event: ProgressEvent) => void
+): Observable<NgtLoaderResults<TUrl, NgtBranchingReturn<TReturnType, GLTF, GLTF & NgtObjectMap>>> {
+    const cdr = inject(ChangeDetectorRef);
+
+    return load(loaderConstructorFactory, input, extensions, onProgress).pipe(
         switchMap(([observables$, inputs]) => {
             return forkJoin(observables$).pipe(
                 map((results) => {
@@ -96,7 +107,7 @@ function injectLoader<TReturnType, TUrl extends string | string[] | Record<strin
                     }, {} as { [key in keyof TUrl]: NgtBranchingReturn<TReturnType, GLTF, GLTF & NgtObjectMap> });
                 }),
                 tap(() => {
-                    requestAnimationFrame(() => cdr.detectChanges());
+                    requestAnimationFrame(() => void safeDetectChanges(cdr));
                 })
             );
         })
@@ -108,7 +119,7 @@ function injectLoader<TReturnType, TUrl extends string | string[] | Record<strin
 };
 
 (injectLoader as NgtLoader).preLoad = (loaderConstructorFactory, inputs, extensions) => {
-    injectLoader(loaderConstructorFactory, inputs, extensions).pipe(take(1)).subscribe();
+    load(loaderConstructorFactory, inputs, extensions).pipe(take(1)).subscribe();
 };
 
 export const injectNgtLoader = injectLoader as NgtLoader;
diff --git a/libs/angular-three/src/lib/pipes/push.ts b/libs/angular-three/src/lib/pipes/push.ts
@@ -1,5 +1,6 @@
-import { ChangeDetectorRef, inject, OnDestroy, Pipe, PipeTransform } from '@angular/core';
+import { ChangeDetectorRef, EnvironmentInjector, inject, OnDestroy, Pipe, PipeTransform } from '@angular/core';
 import { isObservable, ObservableInput, Subscription } from 'rxjs';
+import { safeDetectChanges } from '../utils/safe-detect-changes';
 
 function isPromise(value: unknown): value is Promise<unknown> {
     return (
@@ -12,45 +13,36 @@ function isPromise(value: unknown): value is Promise<unknown> {
 export class NgtPush<T> implements PipeTransform, OnDestroy {
     private readonly cdr = inject(ChangeDetectorRef);
     private readonly parentCdr = inject(ChangeDetectorRef, { skipSelf: true, optional: true });
+    private readonly envCdr = inject(EnvironmentInjector).get(ChangeDetectorRef, null);
 
     private sub?: Subscription;
     private obj?: ObservableInput<T>;
     private latestValue?: T;
 
     transform(value: ObservableInput<T>, defaultValue: T = null!): T {
-        if (this.obj === value) {
-            return this.latestValue as T;
-        }
+        if (this.obj === value) return this.latestValue as T;
+
         this.obj = value;
         this.latestValue = defaultValue;
 
-        if (this.sub) {
-            this.sub.unsubscribe();
-        }
+        if (this.sub) this.sub.unsubscribe();
 
-        if (isObservable(this.obj)) {
-            this.sub = this.obj.subscribe(this.updateValue.bind(this));
-        } else if (isPromise(this.obj)) {
-            (this.obj as Promise<T>).then(this.updateValue.bind(this));
-        } else {
-            throw new Error(`[NGT] Invalid value passed to ngtPush pipe`);
-        }
+        if (isObservable(this.obj)) this.sub = this.obj.subscribe(this.updateValue.bind(this));
+        else if (isPromise(this.obj)) (this.obj as Promise<T>).then(this.updateValue.bind(this));
+        else throw new Error(`[NGT] Invalid value passed to ngtPush pipe`);
 
         return this.latestValue as T;
     }
 
     private updateValue(val: T) {
         this.latestValue = val;
-        this.cdr.detectChanges();
-        if (this.parentCdr) {
-            this.parentCdr.detectChanges();
-        }
+        safeDetectChanges(this.cdr);
+        safeDetectChanges(this.parentCdr);
+        safeDetectChanges(this.envCdr);
     }
 
     ngOnDestroy() {
-        if (this.sub) {
-            this.sub.unsubscribe();
-        }
+        if (this.sub) this.sub.unsubscribe();
         this.latestValue = undefined;
         this.obj = undefined;
     }
diff --git a/libs/angular-three/src/lib/portal.ts b/libs/angular-three/src/lib/portal.ts
@@ -197,11 +197,11 @@ export class NgtPortal extends NgtRxStore<NgtPortalInputs> implements OnInit, On
         });
 
         this.hold(this.parentStore.select(), (previous) =>
-            this.portalStore.set((state) => this.#inject(previous, state))
+            this.portalStore.set((state) => this.inject(previous, state))
         );
 
         requestAnimationFrame(() => {
-            this.portalStore.set((injectState) => this.#inject(this.parentStore.get(), injectState));
+            this.portalStore.set((injectState) => this.inject(this.parentStore.get(), injectState));
         });
         this.portalContentView = this.portalContentAnchor.createEmbeddedView(this.portalContentTemplate);
         this.portalContentView.detectChanges();
@@ -222,7 +222,7 @@ export class NgtPortal extends NgtRxStore<NgtPortalInputs> implements OnInit, On
         super.ngOnDestroy();
     }
 
-    #inject(rootState: NgtState, injectState: NgtState) {
+    private inject(rootState: NgtState, injectState: NgtState) {
         const intersect: Partial<NgtState> = { ...rootState };
 
         Object.keys(intersect).forEach((key) => {
diff --git a/libs/angular-three/src/lib/renderer/renderer.ts b/libs/angular-three/src/lib/renderer/renderer.ts
@@ -14,6 +14,7 @@ import { NgtStore } from '../stores/store';
 import { NgtAnyRecord } from '../types';
 import { getLocalState, prepare } from '../utils/instance';
 import { is } from '../utils/is';
+import { safeDetectChanges } from '../utils/safe-detect-changes';
 import { NGT_COMPOUND_PREFIXES } from './di';
 import { NgtRendererClassId } from './enums';
 import { NgtRendererNode, NgtRendererState, NgtRendererStore } from './state';
@@ -342,8 +343,8 @@ export class NgtRenderer implements Renderer2 {
         // if target is DOM node, then we pass that to delegate Renderer
         const callbackWithCdr = (event: any) => {
             const value = callback(event);
-            if (targetCdr) targetCdr.detectChanges();
-            this.store.rootCdr.detectChanges();
+            safeDetectChanges(targetCdr);
+            safeDetectChanges(this.store.rootCdr);
             return value;
         };
         if (this.store.isDOM(target)) {
diff --git a/libs/angular-three/src/lib/renderer/utils.ts b/libs/angular-three/src/lib/renderer/utils.ts
@@ -4,6 +4,7 @@ import type { NgtEventHandlers, NgtInstanceNode } from '../types';
 import { attach, detach } from '../utils/attach';
 import { getLocalState, invalidateInstance } from '../utils/instance';
 import { is } from '../utils/is';
+import { safeDetectChanges } from '../utils/safe-detect-changes';
 import { supportedEvents } from '../web/events';
 import { NgtRendererClassId } from './enums';
 
@@ -197,8 +198,8 @@ export function processThreeEvent(
 export function eventToHandler(callback: (event: any) => void, cdr: ChangeDetectorRef, targetCdr?: ChangeDetectorRef) {
     return (event: Parameters<Exclude<NgtEventHandlers[(typeof supportedEvents)[number]], undefined>>[0]) => {
         callback(event);
-        if (targetCdr) targetCdr.detectChanges();
-        cdr.detectChanges();
+        safeDetectChanges(targetCdr);
+        safeDetectChanges(cdr);
     };
 }
 
diff --git a/libs/angular-three/src/lib/routed-scene.ts b/libs/angular-three/src/lib/routed-scene.ts
@@ -2,6 +2,7 @@ import { Component } from '@angular/core';
 import { ActivationEnd, Router, RouterOutlet } from '@angular/router';
 import { filter, takeUntil } from 'rxjs';
 import { injectNgtDestroy } from './di/destroy';
+import { safeDetectChanges } from './utils/safe-detect-changes';
 
 @Component({
     standalone: true,
@@ -19,6 +20,6 @@ export class NgtRoutedScene {
                 filter((event) => event instanceof ActivationEnd),
                 takeUntil(destroy$)
             )
-            .subscribe(cdr.detectChanges.bind(cdr));
+            .subscribe(() => safeDetectChanges(cdr));
     }
 }
diff --git a/libs/angular-three/src/lib/stores/rx-store.ts b/libs/angular-three/src/lib/stores/rx-store.ts
diff --git a/libs/angular-three/src/lib/stores/store.ts b/libs/angular-three/src/lib/stores/store.ts
diff --git a/libs/angular-three/src/lib/utils/instance.ts b/libs/angular-three/src/lib/utils/instance.ts
diff --git a/libs/angular-three/src/lib/utils/safe-detect-changes.ts b/libs/angular-three/src/lib/utils/safe-detect-changes.ts

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`import { Directive, EmbeddedViewRef, inject, TemplateRef, ViewContainerRef } from '@angular/core';`
	`2`	`+import { safeDetectChanges } from '../utils/safe-detect-changes';`
`2`	`3`
`3`	`4`	`@Directive()`
`4`	`5`	`export abstract class NgtCommonDirective {`
`@@ -25,7 +26,7 @@ export abstract class NgtCommonDirective {`
`25`	`26`	`this.view.destroy();`
`26`	`27`	`}`
`27`	`28`	`this.view = this.vcr.createEmbeddedView(this.template);`
`28`		`- this.view.detectChanges();`
	`29`	`+ safeDetectChanges(this.view);`
`29`	`30`	`}`
`30`	`31`	`}`
`31`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ import { Component } from '@angular/core';`
`2`	`2`	`import { ActivationEnd, Router, RouterOutlet } from '@angular/router';`
`3`	`3`	`import { filter, takeUntil } from 'rxjs';`
`4`	`4`	`import { injectNgtDestroy } from './di/destroy';`
	`5`	`+import { safeDetectChanges } from './utils/safe-detect-changes';`
`5`	`6`
`6`	`7`	`@Component({`
`7`	`8`	`standalone: true,`
`@@ -19,6 +20,6 @@ export class NgtRoutedScene {`
`19`	`20`	`filter((event) => event instanceof ActivationEnd),`
`20`	`21`	`takeUntil(destroy$)`
`21`	`22`	`)`
`22`		`- .subscribe(cdr.detectChanges.bind(cdr));`
	`23`	`+ .subscribe(() => safeDetectChanges(cdr));`
`23`	`24`	`}`
`24`	`25`	`}`