fix(core): remove createSignal and use signal directly. queue all signal updates in renderer in microtask

Chau Tran · Chau Tran · commit 9315c3074983 · 2023-05-27T16:44:58.000-05:00
diff --git a/libs/angular-three/src/lib/di/ref.ts b/libs/angular-three/src/lib/di/ref.ts
@@ -1,5 +1,4 @@
 import {
-    ApplicationRef,
     ChangeDetectorRef,
     DestroyRef,
     ElementRef,
@@ -8,14 +7,14 @@ import {
     computed,
     inject,
     runInInjectionContext,
+    signal,
     untracked,
 } from '@angular/core';
 import { NgtInstanceNode } from '../types';
 import { assertInjectionContext } from '../utils/assert-in-injection-context';
 import { getLocalState } from '../utils/instance';
 import { is } from '../utils/is';
 import { safeDetectChanges } from '../utils/safe-detect-changes';
-import { createSignal } from '../utils/signal';
 
 export type NgtInjectedRef<TElement> = ElementRef<TElement> & {
     /* consumers should use this for listenting to children changes on this ref */
@@ -29,13 +28,12 @@ export function injectNgtRef<TElement>(
     injector?: Injector
 ): NgtInjectedRef<TElement> {
     injector = assertInjectionContext(injectNgtRef, injector);
+    const ref = is.ref(initial) ? initial : new ElementRef<TElement>(initial as TElement);
+    const signalRef = signal(ref.nativeElement);
+    const readonlySignal = signalRef.asReadonly();
+    const cached = new Map();
     return runInInjectionContext(injector, () => {
         const cdr = inject(ChangeDetectorRef);
-        const appRef = inject(ApplicationRef);
-        const ref = is.ref(initial) ? initial : new ElementRef<TElement>(initial as TElement);
-        const signalRef = createSignal(ref.nativeElement);
-        const readonlySignal = signalRef.asReadonly();
-        const cached = new Map();
 
         inject(DestroyRef).onDestroy(() => void cached.clear());
 
@@ -60,9 +58,15 @@ export function injectNgtRef<TElement>(
         Object.defineProperty(ref, 'nativeElement', {
             set: (newElement) => {
                 if (newElement !== untracked(signalRef)) {
-                    signalRef.set(newElement);
-                    // trigger CDR
+                    try {
+                        signalRef.set(newElement);
+                    } catch {
+                        requestAnimationFrame(() => {
+                            signalRef.set(newElement);
+                        });
+                    }
                     requestAnimationFrame(() => void safeDetectChanges(cdr));
+                    // trigger CDR
                 }
             },
             get: () => readonlySignal(),
diff --git a/libs/angular-three/src/lib/renderer/renderer.ts b/libs/angular-three/src/lib/renderer/renderer.ts
@@ -8,6 +8,7 @@ import {
     effect,
     getDebugNode,
     inject,
+    signal,
     untracked,
     type Renderer2,
     type RendererType2,
@@ -18,7 +19,6 @@ import { NgtStore } from '../stores/store';
 import type { NgtAnyRecord } from '../types';
 import { getLocalState, prepare } from '../utils/instance';
 import { is } from '../utils/is';
-import { createSignal } from '../utils/signal';
 import { NGT_COMPOUND_PREFIXES } from './di';
 import { NgtRendererClassId } from './enums';
 import { NgtRendererStore, type NgtRendererNode, type NgtRendererState } from './store';
@@ -117,7 +117,7 @@ export class NgtRenderer implements Renderer2 {
                     { __ngt_renderer__: { rawValue: undefined } },
                     // NOTE: we assign this manually to a raw value node
                     // because we say it is a 'three' node but we're not using prepare()
-                    { __ngt__: { isRaw: true, parent: createSignal(null) } }
+                    { __ngt__: { isRaw: true, parent: signal(null) } }
                 )
             );
         }
diff --git a/libs/angular-three/src/lib/renderer/store.ts b/libs/angular-three/src/lib/renderer/store.ts
@@ -4,7 +4,7 @@ import { NgtArgs } from '../directives/args';
 import type { NgtCommonDirective } from '../directives/common';
 import { NgtParent } from '../directives/parent';
 import { NgtStore } from '../stores/store';
-import type { NgtAnyRecord } from '../types';
+import type { NgtAnyRecord, NgtInstanceNode } from '../types';
 import { applyProps } from '../utils/apply-props';
 import { getLocalState } from '../utils/instance';
 import { is } from '../utils/is';
@@ -207,6 +207,7 @@ export class NgtRendererStore {
         }
 
         applyProps(node, { [name]: value });
+        this.#updateNativeProps(node, name, value);
     }
 
     applyProperty(node: NgtRendererNode, name: string, value: any) {
@@ -245,6 +246,7 @@ export class NgtRendererStore {
             value = compound[NgtCompoundClassId.props][name];
         }
         applyProps(node, { [name]: value });
+        this.#updateNativeProps(node, name, value);
     }
 
     isCompound(name: string) {
@@ -407,7 +409,15 @@ export class NgtRendererStore {
         }
     }
 
-     #firstNonInjectedDirective<T extends NgtCommonDirective>(dir: Type<T>) {
+    #updateNativeProps(node: NgtInstanceNode, name: string, value: any) {
+        const localState = getLocalState(node);
+        if (!localState || !localState.nativeProps) return;
+        queueMicrotask(() => {
+            localState.nativeProps.set({ [name]: value });
+        });
+    }
+
+    #firstNonInjectedDirective<T extends NgtCommonDirective>(dir: Type<T>) {
         let directive: T | undefined;
 
         let i = this.#comments.length - 1;
@@ -434,7 +444,7 @@ export class NgtRendererStore {
         return directive;
     }
 
-     #tryGetPortalStore() {
+    #tryGetPortalStore() {
         let store: NgtStore | undefined;
         // we only care about the portal states because NgtStore only differs per Portal
         let i = this.portals.length - 1;
diff --git a/libs/angular-three/src/lib/renderer/utils.ts b/libs/angular-three/src/lib/renderer/utils.ts
@@ -75,7 +75,9 @@ export function attachThreeChild(parent: NgtInstanceNode, child: NgtInstanceNode
             // attach
             if (cLS.isRaw) {
                 if (cLS.parent) {
-                    cLS.parent.set(parent);
+                    queueMicrotask(() => {
+                        cLS.parent.set(parent);
+                    });
                 }
                 // at this point we don't have rawValue yet, so we bail and wait until the Renderer recalls attach
                 if (child.__ngt_renderer__[NgtRendererClassId.rawValue] === undefined) return;
@@ -94,7 +96,9 @@ export function attachThreeChild(parent: NgtInstanceNode, child: NgtInstanceNode
     pLS.add(child, added ? 'objects' : 'nonObjects');
 
     if (cLS.parent) {
-        cLS.parent.set(parent);
+        queueMicrotask(() => {
+            cLS.parent.set(parent);
+        });
     }
 
     if (cLS.afterAttach) cLS.afterAttach.emit({ parent, node: child });
@@ -108,7 +112,9 @@ export function removeThreeChild(parent: NgtInstanceNode, child: NgtInstanceNode
     const cLS = getLocalState(child);
 
     // clear parent ref
-    cLS.parent?.set(null);
+    queueMicrotask(() => {
+        cLS.parent?.set(null);
+    });
 
     // remove child from parent
     if (untracked(pLS.objects)) pLS.remove(child, 'objects');
diff --git a/libs/angular-three/src/lib/stores/signal.store.ts b/libs/angular-three/src/lib/stores/signal.store.ts
@@ -3,13 +3,13 @@ import {
     Injectable,
     Optional,
     computed,
+    signal,
     untracked,
     type CreateComputedOptions,
     type Signal,
     type WritableSignal,
 } from '@angular/core';
 import type { NgtAnyRecord } from '../types';
-import { createSignal } from '../utils/signal';
 
 const STORE_COMPUTED_KEY = '__ngt_store_computed__' as const;
 
@@ -25,7 +25,7 @@ export class NgtSignalStore<TState extends object> {
         initialState: Partial<TState> = {} as unknown as Partial<TState>
     ) {
         initialState ??= {};
-        this.#state = createSignal(Object.assign(initialState, { __ngt_dummy_state__: Date.now() }) as TState);
+        this.#state = signal(Object.assign(initialState, { __ngt_dummy_state__: Date.now() }) as TState);
         this.state = this.#state.asReadonly();
     }
 
diff --git a/libs/angular-three/src/lib/utils/apply-props.ts b/libs/angular-three/src/lib/utils/apply-props.ts
@@ -88,8 +88,6 @@ export function applyProps(instance: NgtInstanceNode, props: NgtAnyRecord): NgtI
                 // auto-convert srgb
                 if (!THREE.ColorManagement && !rootState?.linear && isColor) targetProp.convertSRGBToLinear();
             }
-
-            localState?.nativeProps?.set({ [key]: targetProp });
         }
         // else just overwrite the value
         else {
@@ -107,7 +105,6 @@ export function applyProps(instance: NgtInstanceNode, props: NgtAnyRecord): NgtI
                     else texture.encoding = rootState.gl.outputEncoding;
                 }
             }
-            localState?.nativeProps?.set({ [key]: value });
         }
 
         checkUpdate(targetProp);
diff --git a/libs/angular-three/src/lib/utils/instance.ts b/libs/angular-three/src/lib/utils/instance.ts
@@ -1,7 +1,6 @@
-import { untracked } from '@angular/core';
+import { signal, untracked } from '@angular/core';
 import { NgtSignalStore } from '../stores/signal.store';
 import type { NgtAnyRecord, NgtInstanceLocalState, NgtInstanceNode } from '../types';
-import { createSignal } from './signal';
 import { checkUpdate } from './update';
 
 export function getLocalState<TInstance extends object = NgtAnyRecord>(
@@ -25,37 +24,41 @@ export function prepare<TInstance extends object = NgtAnyRecord>(
 
     if (localState?.primitive || !instance.__ngt__) {
         const {
-            objects = createSignal<NgtInstanceNode[]>([]),
-            nonObjects = createSignal<NgtInstanceNode[]>([]),
+            objects = signal<NgtInstanceNode[]>([]),
+            nonObjects = signal<NgtInstanceNode[]>([]),
             ...rest
         } = localState || {};
 
         instance.__ngt__ = {
             previousAttach: null,
             store: null,
-            parent: createSignal(null),
+            parent: signal(null),
             memoized: {},
             eventCount: 0,
             handlers: {},
             objects,
             nonObjects,
             nativeProps: new NgtSignalStore({}),
             add: (object, type) => {
-                const current = untracked(instance.__ngt__[type]);
-                const foundIndex = current.indexOf((obj: NgtInstanceNode) => obj === object);
-                if (foundIndex > -1) {
-                    // if we add an object with the same reference, then we switch it out
-                    // and update the BehaviorSubject
-                    current.splice(foundIndex, 1, object);
-                    instance.__ngt__[type].set(current);
-                } else {
-                    instance.__ngt__[type].update((prev) => [...prev, object]);
-                }
-                notifyAncestors(untracked(instance.__ngt__.parent));
+                queueMicrotask(() => {
+                    const current = untracked(instance.__ngt__[type]);
+                    const foundIndex = current.indexOf((obj: NgtInstanceNode) => obj === object);
+                    if (foundIndex > -1) {
+                        // if we add an object with the same reference, then we switch it out
+                        // and update the BehaviorSubject
+                        current.splice(foundIndex, 1, object);
+                        instance.__ngt__[type].set(current);
+                    } else {
+                        instance.__ngt__[type].update((prev) => [...prev, object]);
+                    }
+                    notifyAncestors(untracked(instance.__ngt__.parent));
+                });
             },
             remove: (object, type) => {
-                instance.__ngt__[type].update((prev) => prev.filter((o) => o !== object));
-                notifyAncestors(untracked(instance.__ngt__.parent));
+                queueMicrotask(() => {
+                    instance.__ngt__[type].update((prev) => prev.filter((o) => o !== object));
+                    notifyAncestors(untracked(instance.__ngt__.parent));
+                });
             },
             ...rest,
         } as NgtInstanceLocalState;
diff --git a/libs/angular-three/src/lib/utils/signal.ts b/libs/angular-three/src/lib/utils/signal.ts
diff --git a/libs/soba/loaders/src/progress/progress.ts b/libs/soba/loaders/src/progress/progress.ts
@@ -1,27 +1,20 @@
-import { ChangeDetectorRef, Injector, inject, runInInjectionContext } from '@angular/core';
-import { assertInjectionContext, createSignal, safeDetectChanges } from 'angular-three';
+import { ChangeDetectorRef, Injector, inject, runInInjectionContext, signal } from '@angular/core';
+import { assertInjectionContext, safeDetectChanges } from 'angular-three';
 import * as THREE from 'three';
 
 export function injectNgtsProgress(injector?: Injector) {
     injector = assertInjectionContext(injectNgtsProgress, injector);
     return runInInjectionContext(injector, () => {
         const cdr = inject(ChangeDetectorRef);
 
-        const progress = createSignal<{
+        const progress = signal<{
             errors: string[];
             active: boolean;
             progress: number;
             item: string;
             loaded: number;
             total: number;
-        }>({
-            errors: [],
-            active: false,
-            progress: 0,
-            item: '',
-            loaded: 0,
-            total: 0,
-        });
+        }>({ errors: [], active: false, progress: 0, item: '', loaded: 0, total: 0 });
 
         let saveLastTotalLoaded = 0;
 
diff --git a/libs/soba/staging/src/stage/stage.ts b/libs/soba/staging/src/stage/stage.ts
@@ -10,8 +10,9 @@ import {
     Input,
     OnChanges,
     Output,
+    signal,
 } from '@angular/core';
-import { createSignal, extend, NgtArgs, NgtSignalStore, safeDetectChanges } from 'angular-three';
+import { extend, NgtArgs, NgtSignalStore, safeDetectChanges } from 'angular-three';
 import { AmbientLight, Group, PointLight, SpotLight, Vector2 } from 'three';
 import { NgtsAccumulativeShadows } from '../accumulative-shadows/accumulative-shadows';
 import { NgtsRandomizedLights } from '../accumulative-shadows/randomized-lights';
@@ -235,7 +236,7 @@ export class NgtsStage extends NgtSignalStore<NgtsStageProps> {
 
     @Output() centered = new EventEmitter() as NgtsCenter['centered'];
 
-    readonly boundingState = createSignal({ radius: 0, width: 0, height: 0, depth: 0 });
+    readonly boundingState = signal({ radius: 0, width: 0, height: 0, depth: 0 });
 
     readonly #preset = this.select('preset');
     readonly #environment = this.select('environment');

Original file line number	Diff line number	Diff line change
`@@ -88,8 +88,6 @@ export function applyProps(instance: NgtInstanceNode, props: NgtAnyRecord): NgtI`
`88`	`88`	`// auto-convert srgb`
`89`	`89`	`if (!THREE.ColorManagement && !rootState?.linear && isColor) targetProp.convertSRGBToLinear();`
`90`	`90`	`}`
`91`		`-`
`92`		`- localState?.nativeProps?.set({ [key]: targetProp });`
`93`	`91`	`}`
`94`	`92`	`// else just overwrite the value`
`95`	`93`	`else {`
`@@ -107,7 +105,6 @@ export function applyProps(instance: NgtInstanceNode, props: NgtAnyRecord): NgtI`
`107`	`105`	`else texture.encoding = rootState.gl.outputEncoding;`
`108`	`106`	`}`
`109`	`107`	`}`
`110`		`- localState?.nativeProps?.set({ [key]: value });`
`111`	`108`	`}`
`112`	`109`
`113`	`110`	`checkUpdate(targetProp);`