Redesign initialization of HasProps instances

bokehjs/src/lib/core/has_props.ts

@@ -337,8 +337,6 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
       assert(keys(attrs).length == 1, "'id' cannot be used together with property initializers")
     } else {
       this.initialize_props(attrs)
-      this.finalize()
-      this.connect_signals()
     }
   }
 
@@ -359,21 +357,35 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
     }
   }
 
+  private _initialized: boolean = false
+  initialize(): void {
+    assert(!this._initialized)
+    this._initialized = true
+  }
+
+  /*
   finalize(): void {
     this.initialize()
   }
+  */
+  //instance.finalize()
+  //instance.assert_initialized()
 
-  initialize(): void {}
-
+  /*
   assert_initialized(): void {
     for (const prop of this) {
       if (prop.syncable && !prop.readonly) {
         prop.get_value()
       }
     }
   }
+  */
 
+  private _signals_connected: boolean = false
   connect_signals(): void {
+    assert(!this._signals_connected)
+    this._signals_connected = true
+
     for (const prop of this) {
       if (!(prop instanceof p.VectorSpec || prop instanceof p.ScalarSpec)) {
         continue
@@ -394,6 +406,7 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
 
   disconnect_signals(): void {
     Signal.disconnect_receiver(this)
+    this._signals_connected = false
   }
 
   destroy(): void {
@@ -559,6 +572,9 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
               HasProps._value_record_references(value, refs, {recursive})
             }
           }
+          for (const ref of value._references()) {
+            HasProps._value_record_references(ref, refs, {recursive})
+          }
         }
       }
     } else if (isIterable(value)) {
@@ -572,6 +588,8 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
     }
   }
 
+  protected *_references(): Iterable<HasProps> {}
+
   static references(value: unknown, options: {recursive: boolean}): Set<HasProps> {
     const refs = new Set<HasProps>()
     HasProps._value_record_references(value, refs, options)
@@ -591,16 +609,24 @@ export abstract class HasProps extends Signalable() implements Equatable, Printa
       if (this.document == doc) {
         return
       } else {
-        throw new Error("models must be owned by only a single document")
+        throw new Error(`${this} must be owned by only a single document`)
       }
     }
 
     this.document = doc
     this._doc_attached()
+
+    if (!this._initialized) {
+      this.initialize()
+    }
+    if (!this._signals_connected) {
+      this.connect_signals()
+    }
   }
 
   detach_document(): void {
     // This should only be called by the Document implementation to unset the document field
+    this.disconnect_signals()
     this._doc_detached()
     this.document = null
   }

bokehjs/src/lib/core/serialization/deserializer.ts

@@ -70,9 +70,7 @@ export class Deserializer {
     })()
 
     for (const instance of finalizable) {
-      this.finalize?.(instance)
-      instance.finalize()
-      instance.assert_initialized()
+      instance.initialize()
     }
 
     // `connect_signals` has to be executed last because it may rely on properties
@@ -83,6 +81,10 @@ export class Deserializer {
       instance.connect_signals()
     }
 
+    for (const instance of finalizable) {
+      this.finalize?.(instance)
+    }
+
     return decoded
   }
 

bokehjs/src/lib/model.ts

@@ -35,7 +35,7 @@ export interface Model extends Model.Attrs {}
 export class Model extends HasProps {
   declare properties: Model.Props
 
-  private /*readonly*/ _js_callbacks: Map<string, (() => void)[]>
+  private readonly _js_callbacks: Map<string, (() => void)[]> = new Map()
 
   override get is_syncable(): boolean {
     return this.syncable
@@ -60,11 +60,6 @@ export class Model extends HasProps {
     }))
   }
 
-  override initialize(): void {
-    super.initialize()
-    this._js_callbacks = new Map()
-  }
-
   override connect_signals(): void {
     super.connect_signals()
 

bokehjs/src/lib/models/annotations/legend.ts

@@ -496,17 +496,12 @@ export class Legend extends Annotation {
   declare properties: Legend.Props
   declare __view_type__: LegendView
 
-  item_change: Signal0<this>
+  readonly item_change = new Signal0(this, "item_change")
 
   constructor(attrs?: Partial<Legend.Attrs>) {
     super(attrs)
   }
 
-  override initialize(): void {
-    super.initialize()
-    this.item_change = new Signal0(this, "item_change")
-  }
-
   static {
     this.prototype.default_view = LegendView
 

bokehjs/src/lib/models/annotations/legend_item.ts

@@ -23,7 +23,7 @@ export interface LegendItem extends LegendItem.Attrs {}
 export class LegendItem extends Model {
   declare properties: LegendItem.Props
 
-  legend: Legend | null
+  legend: Legend | null = null
 
   constructor(attrs?: Partial<LegendItem.Attrs>) {
     super(attrs)
@@ -70,8 +70,6 @@ export class LegendItem extends Model {
 
   override initialize(): void {
     super.initialize()
-    this.legend = null
-    this.connect(this.change, () => this.legend?.item_change.emit())
 
     // Validate data_sources match
     const data_source_validation = this._check_data_sources_on_renderers()
@@ -86,6 +84,11 @@ export class LegendItem extends Model {
     }
   }
 
+  override connect_signals(): void {
+    super.connect_signals()
+    this.connect(this.change, () => this.legend?.item_change.emit())
+  }
+
   get_field_from_label_prop(): string | null {
     const {label} = this
     return isField(label) ? label.field : null

bokehjs/src/lib/models/expressions/expression.ts

@@ -18,12 +18,7 @@ export abstract class Expression<T = Arrayable> extends Model {
     super(attrs)
   }
 
-  protected _result: Map<ColumnarDataSource, T>
-
-  override initialize(): void {
-    super.initialize()
-    this._result = new Map()
-  }
+  protected readonly _result: Map<ColumnarDataSource, T> = new Map()
 
   protected abstract _v_compute(source: ColumnarDataSource): T
 
@@ -52,12 +47,7 @@ export abstract class ScalarExpression<T> extends Model {
     super(attrs)
   }
 
-  protected _result: Map<ColumnarDataSource, T>
-
-  override initialize(): void {
-    super.initialize()
-    this._result = new Map()
-  }
+  protected readonly _result: Map<ColumnarDataSource, T> = new Map()
 
   protected abstract _compute(source: ColumnarDataSource): T
 

bokehjs/src/lib/models/formatters/log_tick_formatter.ts

@@ -32,12 +32,7 @@ export class LogTickFormatter extends TickFormatter {
     }))
   }
 
-  protected basic_formatter: BasicTickFormatter
-
-  override initialize(): void {
-    super.initialize()
-    this.basic_formatter = new BasicTickFormatter()
-  }
+  protected readonly basic_formatter = new BasicTickFormatter()
 
   override format_graphics(ticks: number[], opts: {loc: number}): GraphicsBox[] {
     if (ticks.length == 0) {

bokehjs/src/lib/models/mappers/color_mapper.ts

@@ -41,17 +41,12 @@ export interface ColorMapper extends ColorMapper.Attrs {}
 export abstract class ColorMapper extends Mapper<Color> {
   declare properties: ColorMapper.Props
 
-  metrics_change: Signal0<this>
+  readonly metrics_change = new Signal0(this, "metrics_change")
 
   constructor(attrs?: Partial<ColorMapper.Attrs>) {
     super(attrs)
   }
 
-  override initialize(): void {
-    super.initialize()
-    this.metrics_change = new Signal0(this, "metrics_change")
-  }
-
   static {
     this.define<ColorMapper.Props>(({Color, List}) => ({
       palette:   [ List(Color) ],

bokehjs/src/lib/models/sources/columnar_data_source.ts

@@ -49,8 +49,8 @@ export abstract class ColumnarDataSource extends DataSource {
     return column as T[]
   }
 
-  _select: Signal0<this>
-  inspect: Signal<[GlyphRenderer, {geometry: Geometry}], this>
+  readonly _select: Signal0<this> = new Signal0(this, "select")
+  readonly inspect: Signal<[GlyphRenderer, {geometry: Geometry}], this> = new Signal(this, "inspect")
 
   readonly selection_manager = new SelectionManager(this)
 
@@ -69,13 +69,6 @@ export abstract class ColumnarDataSource extends DataSource {
     }))
   }
 
-  override initialize(): void {
-    super.initialize()
-
-    this._select = new Signal0(this, "select")
-    this.inspect = new Signal(this, "inspect")
-  }
-
   get inferred_defaults(): Map<string, unknown> {
     const defaults: Map<string, unknown> = new Map()
     for (const [name, array] of entries(this.data)) {

bokehjs/src/lib/models/sources/geojson_data_source.ts

@@ -61,7 +61,8 @@ export class GeoJSONDataSource extends ColumnarDataSource {
 
   override connect_signals(): void {
     super.connect_signals()
-    this.connect(this.properties.geojson.change, () => this._update_data())
+    const {geojson} = this.properties
+    this.on_change(geojson, () => this._update_data())
   }
 
   protected _update_data(): void {

bokehjs/src/lib/models/tiles/tile_source.ts

@@ -48,11 +48,10 @@ export abstract class TileSource extends Model {
     }))
   }
 
-  tiles: Map<string, Tile>
+  readonly tiles: Map<string, Tile> = new Map()
 
   override initialize(): void {
     super.initialize()
-    this.tiles = new Map()
     this._normalize_case()
   }
 
@@ -86,7 +85,7 @@ export abstract class TileSource extends Model {
   }
 
   protected _clear_cache(): void {
-    this.tiles = new Map()
+    this.tiles.clear()
   }
 
   tile_xyz_to_key(x: number, y: number, z: number): string {

bokehjs/src/lib/models/tools/tool_proxy.ts

@@ -39,10 +39,9 @@ export class ToolProxy<T extends Tool> extends Model {
     }))
   }
 
-  do: Signal0<this>
+  readonly do = new Signal0(this, "do")
 
   // Operates all the tools given only one button
-
   get underlying(): Tool {
     return this.tools[0]
   }
@@ -91,11 +90,6 @@ export class ToolProxy<T extends Tool> extends Model {
     return tool.visible
   }
 
-  override initialize(): void {
-    super.initialize()
-    this.do = new Signal0(this, "do")
-  }
-
   override connect_signals(): void {
     super.connect_signals()
     this.connect(this.do, () => this.doit())