fix(core): observer remove unregisterOnNextCall (#2334)

packages/core/src/observer/__tests__/observable.spec.ts

@@ -0,0 +1,94 @@
+/**
+ * Copyright 2023-present DreamNum Inc.
+ *
+ * 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 { describe, expect, it } from 'vitest';
+import { EventState, Observable, Observer } from '../observable';
+
+describe('EventState', () => {
+    it('should initialize with skipNextObservers set to false', () => {
+        const eventState = new EventState();
+        expect(eventState.skipNextObservers).toBe(false);
+    });
+
+    it('should allow stopPropagation to be set', () => {
+        const eventState = new EventState();
+        eventState.stopPropagation();
+        expect(eventState.isStopPropagation).toBe(true);
+    });
+});
+
+describe('Observer', () => {
+    it('should create an observer with a callback', () => {
+        const callback = (eventData: any, eventState: EventState) => {};
+        const observable = new Observable();
+        const observer = new Observer(callback, observable);
+        expect(observer.callback).toBe(callback);
+        expect(observer.observable).toBe(observable);
+    });
+
+    it('should dispose an observer', () => {
+        const callback = (eventData: any, eventState: EventState) => {};
+        const observable = new Observable();
+        // const observer = new Observer(callback, observable);
+        const observer = observable.add(callback);
+        observer?.dispose();
+        expect(observable.observers.length).toBe(0);
+    });
+});
+
+describe('Observable', () => {
+    it('should add observers', () => {
+        const observable = new Observable();
+        const callback = (eventData: any, eventState: EventState) => {};
+        observable.add(callback);
+        expect(observable.observers.length).toBe(1);
+    });
+
+    it('should notify observers', () => {
+        const observable = new Observable();
+        const callback = (eventData: any, eventState: EventState) => {
+            eventState.lastReturnValue = eventData;
+            return eventData; // Ensure the callback returns the eventData
+        };
+        observable.add(callback);
+        const result = observable.notifyObservers('testData');
+        expect(result?.lastReturnValue).toBe('testData');
+    });
+
+    it('should skip observers when skipNextObservers is set', () => {
+        const observable = new Observable();
+        const callback1 = (eventData: any, eventState: EventState) => {
+            eventState.skipNextObservers = true;
+        };
+        const callback2 = (eventData: any, eventState: EventState) => {
+            eventState.lastReturnValue = 'should not be called';
+        };
+        observable.add(callback1);
+        observable.add(callback2);
+        const result = observable.notifyObservers('testData');
+        expect(result?.lastReturnValue).not.toBe('should not be called');
+    });
+
+    it('should notify observers with promise', async () => {
+        const observable = new Observable();
+        const callback = async (eventData: any, eventState: EventState) => {
+            eventState.lastReturnValue = eventData;
+        };
+        observable.add(callback);
+        const result = await observable.notifyObserversWithPromise('testData');
+        expect(result).toBe('testData');
+    });
+});

packages/core/src/observer/observable.ts

@@ -76,14 +76,6 @@ export function isObserver(value: any) {
  * @deprecated use rxjs instead
  */
 export class Observer<T = void> {
-    /** @hidden */
-    _willBeUnregistered = false;
-
-    /**
-     * Gets or sets a property defining that the observer as to be unregistered after the next notification
-     */
-    unregisterOnNextCall = false;
-
     dispose() {
         this.observable.remove(this);
     }
@@ -150,15 +142,13 @@ export class Observable<T> {
      */
     add(
         callback: (eventData: T, eventState: EventState) => void,
-        insertFirst = false,
-        unregisterOnFirstCall = false
+        insertFirst = false
     ): Nullable<Observer<T>> {
         if (!callback) {
             return null;
         }
 
         const observer = new Observer(callback, this);
-        observer.unregisterOnNextCall = unregisterOnFirstCall;
 
         if (insertFirst) {
             this._observers.unshift(observer);
@@ -173,15 +163,6 @@ export class Observable<T> {
         return observer;
     }
 
-    /**
-     * Create a new WorkBookObserver with the specified callback and unregisters after the next notification
-     * @param callback the callback that will be executed for that WorkBookObserver
-     * @returns the new observer created for the callback
-     */
-    addOnce(callback: (eventData: T, eventState: EventState) => void): Nullable<Observer<T>> {
-        return this.add(callback, undefined, true);
-    }
-
     /**
      * Remove an WorkBookObserver from the Observable object
      * @param observer the instance of the WorkBookObserver to remove
@@ -195,33 +176,13 @@ export class Observable<T> {
         const index = this._observers.indexOf(observer);
 
         if (index !== -1) {
-            this._deferUnregister(observer);
+            this._remove(observer);
             return true;
         }
 
         return false;
     }
 
-    /**
-     * Remove a callback from the Observable object
-     * @param callback the callback to remove
-     * @returns false if it doesn't belong to this Observable
-     */
-    removeCallback(callback: (eventData: T, eventState: EventState) => void): boolean {
-        for (let index = 0; index < this._observers.length; index++) {
-            const observer = this._observers[index];
-            if (observer._willBeUnregistered) {
-                continue;
-            }
-            if (observer.callback === callback) {
-                this._deferUnregister(observer);
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     /**
      * Moves the observable to the top of the observer list making it get called first when notified
      * @param observer the observer to move
@@ -259,16 +220,9 @@ export class Observable<T> {
 
         for (let index = 0; index < this._observers.length; index++) {
             const obs = this._observers[index];
-            if (obs._willBeUnregistered) {
-                continue;
-            }
 
             state.lastReturnValue = obs.callback(eventData, state);
 
-            if (obs.unregisterOnNextCall) {
-                this._deferUnregister(obs);
-            }
-
             if (state.isStopPropagation) {
                 _isStopPropagation = true;
             }
@@ -314,15 +268,8 @@ export class Observable<T> {
             if (state.skipNextObservers) {
                 continue;
             }
-            if (obs._willBeUnregistered) {
-                continue;
-            }
 
             p = p.then(() => obs.callback(eventData, state));
-
-            if (obs.unregisterOnNextCall) {
-                this._deferUnregister(obs);
-            }
         }
 
         // return the eventData
@@ -335,19 +282,11 @@ export class Observable<T> {
      * @param eventData defines the data to be sent to each callback
      */
     notifyObserver(observer: Observer<T>, eventData: T): Nullable<INotifyObserversReturn> {
-        if (observer._willBeUnregistered) {
-            return;
-        }
-
         const state = this._eventState;
         state.skipNextObservers = false;
 
         observer.callback(eventData, state);
 
-        if (observer.unregisterOnNextCall) {
-            this._deferUnregister(observer);
-        }
-
         return {
             lastReturnValue: state.lastReturnValue,
             stopPropagation: state.isStopPropagation,
@@ -382,14 +321,6 @@ export class Observable<T> {
         return result;
     }
 
-    private _deferUnregister(observer: Observer<T>): void {
-        observer.unregisterOnNextCall = false;
-        observer._willBeUnregistered = true;
-        setTimeout(() => {
-            this._remove(observer);
-        }, 0);
-    }
-
     // This should only be called when not iterating over _observers to avoid callback skipping.
     // Removes an observer from the _observer Array.
     private _remove(observer: Nullable<Observer<T>>): boolean {

packages/slides/src/views/render/canvas-view.ts

@@ -155,8 +155,10 @@ export class CanvasView extends RxDisposable {
 
         const { scene, engine } = currentRender;
 
-        engine.onTransformChangeObservable.addOnce(() => {
+        const observer = engine.onTransformChangeObservable.add(() => {
             this._scrollToCenter();
+            // add once
+            observer?.dispose();
         });
 
         engine.onTransformChangeObservable.add(() => {