Fixed bug in FlatMappedProperty that resulted in undo stack switching not working correctly.

src/core/observable/property/DependentProperty.ts

@@ -11,7 +11,8 @@ import { ChangeEvent } from "../Observable";
  */
 export abstract class DependentProperty<T> extends AbstractMinimalProperty<T> {
     private dependency_disposer = new Disposer();
-    private _val?: T;
+
+    protected _val?: T;
 
     get val(): T {
         return this.get_val();
@@ -19,7 +20,7 @@ export abstract class DependentProperty<T> extends AbstractMinimalProperty<T> {
 
     get_val(): T {
         if (this.should_recompute()) {
-            this._val = this.compute_value();
+            this._val = this.compute_value(this.observers.length > 0);
         }
 
         return this._val as T;
@@ -34,13 +35,13 @@ export abstract class DependentProperty<T> extends AbstractMinimalProperty<T> {
         options?: { call_now?: boolean },
     ): Disposable {
         if (this.dependency_disposer.length === 0) {
-            this._val = this.compute_value();
+            this._val = this.compute_value(true);
 
             this.dependency_disposer.add_all(
                 ...this.dependencies.map(dependency =>
                     dependency.observe(() => {
                         const old_value = this._val!;
-                        this._val = this.compute_value();
+                        this._val = this.compute_value(true);
 
                         if (this._val !== old_value) {
                             this.emit();
@@ -67,5 +68,5 @@ export abstract class DependentProperty<T> extends AbstractMinimalProperty<T> {
         return this.dependency_disposer.length === 0;
     }
 
-    protected abstract compute_value(): T;
+    protected abstract compute_value(has_observers: boolean): T;
 }

src/core/observable/property/FlatMappedProperty.test.ts

@@ -0,0 +1,26 @@
+import { FlatMappedProperty } from "./FlatMappedProperty";
+import { SimpleProperty } from "./SimpleProperty";
+import { with_disposable } from "../../../../test/src/core/observables/disposable_helpers";
+
+// This is a regression test, it's important that this exact sequence of statements stays the same.
+test(`It should emit a change when its direct property dependency changes.`, () => {
+    // p is the direct property dependency.
+    const p = new SimpleProperty(new SimpleProperty(7));
+    const fp = new FlatMappedProperty([p], () => p.val);
+    let v: number | undefined;
+
+    with_disposable(
+        fp.observe(({ value }) => (v = value)),
+        () => {
+            expect(v).toBeUndefined();
+
+            p.val.val = 99;
+
+            expect(v).toBe(99);
+
+            p.val = new SimpleProperty(7);
+
+            expect(v).toBe(7);
+        },
+    );
+});

src/core/observable/property/FlatMappedProperty.ts

@@ -9,10 +9,10 @@ export class FlatMappedProperty<T> extends DependentProperty<T> {
     private computed_disposable?: Disposable;
 
     get_val(): T {
-        if (this.should_recompute() || !this.computed_property) {
+        if (this.should_recompute()) {
             return super.get_val();
         } else {
-            return this.computed_property.val;
+            return this.computed_property!.val;
         }
     }
 
@@ -50,14 +50,17 @@ export class FlatMappedProperty<T> extends DependentProperty<T> {
         return new FlatMappedProperty([this], () => transform(this.val));
     }
 
-    protected compute_value(): T {
+    protected compute_value(has_observers: boolean): T {
-        this.computed_disposable?.dispose();
-
         this.computed_property = this.compute();
 
-        this.computed_disposable = this.computed_property.observe(() => {
+        this.computed_disposable?.dispose();
-            this.emit();
+
-        });
+        if (has_observers) {
+            this.computed_disposable = this.computed_property.observe(() => {
+                this._val = this.computed_property!.val;
+                this.emit();
+            });
+        }
 
         return this.computed_property.val;
     }

src/core/undo/SimpleUndo.ts

@@ -31,9 +31,9 @@ export class SimpleUndo implements Undo {
         undo_manager.current.val = this;
     }
 
-    readonly can_undo = property(false);
+    readonly can_undo: WritableProperty<boolean> = property(false);
 
-    readonly can_redo = property(false);
+    readonly can_redo: WritableProperty<boolean> = property(false);
 
     readonly first_undo: Property<Action | undefined>;
 

src/core/undo/UndoManager.ts

@@ -1,39 +1,46 @@
 import { property } from "../observable";
 import { Undo } from "./Undo";
+import { Property } from "../observable/property/Property";
+import { Action } from "./Action";
+import { WritableProperty } from "../observable/property/WritableProperty";
 
-const NOOP_UNDO: Undo = {
+class NoopUndo implements Undo {
-    can_redo: property(false),
+    readonly can_redo = property(false);
-    can_undo: property(false),
+    readonly can_undo = property(false);
-    first_redo: property(undefined),
+    readonly first_redo = property(undefined);
-    first_undo: property(undefined),
+    readonly first_undo = property(undefined);
 
-    make_current() {
+    constructor(private readonly manager: UndoManager) {}
-        undo_manager.current.val = this;
-    },
 
-    redo() {
+    make_current(): void {
+        this.manager.current.val = this;
+    }
+
+    redo(): boolean {
         return false;
-    },
+    }
 
-    reset() {
+    reset(): void {
         // Do nothing.
-    },
+    }
 
-    undo() {
+    undo(): boolean {
         return false;
-    },
+    }
-};
+}
 
 export class UndoManager {
-    readonly current = property<Undo>(NOOP_UNDO);
+    private readonly noop_undo = new NoopUndo(this);
 
-    can_undo = this.current.flat_map(c => c.can_undo);
+    readonly current: WritableProperty<Undo> = property<Undo>(this.noop_undo);
 
-    can_redo = this.current.flat_map(c => c.can_redo);
+    readonly can_undo: Property<boolean> = this.current.flat_map(c => c.can_undo);
 
-    first_undo = this.current.flat_map(c => c.first_undo);
+    readonly can_redo: Property<boolean> = this.current.flat_map(c => c.can_redo);
 
-    first_redo = this.current.flat_map(c => c.first_redo);
+    readonly first_undo: Property<Action | undefined> = this.current.flat_map(c => c.first_undo);
+
+    readonly first_redo: Property<Action | undefined> = this.current.flat_map(c => c.first_redo);
 
     undo(): boolean {
         return this.current.val.undo();
@@ -44,7 +51,7 @@ export class UndoManager {
     }
 
     make_noop_current(): void {
-        undo_manager.current.val = NOOP_UNDO;
+        this.current.val = this.noop_undo;
     }
 }