Core architecture fixes -- Review in steps

packages/ts/src/components/annotations/index.ts

@@ -31,7 +31,7 @@ export class Annotations extends ComponentCore<unknown[], AnnotationsConfigInter
 
   constructor (config?: AnnotationsConfigInterface) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   _render (customDuration?: number): void {

packages/ts/src/components/area/index.ts

@@ -42,7 +42,7 @@ export class Area<Datum> extends XYComponentCore<Datum, AreaConfigInterface<Datu
 
   constructor (config?: AreaConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     // Determine if the provided chart should be stacked
     this.stacked = Array.isArray(this.config.y)

packages/ts/src/components/axis/index.ts

@@ -51,7 +51,7 @@ export class Axis<Datum> extends XYComponentCore<Datum, AxisConfigInterface<Datu
 
   constructor (config?: AxisConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.g.attr('axis-type', this.config.type)
 

packages/ts/src/components/brush/index.ts

@@ -38,7 +38,7 @@ export class Brush<Datum> extends XYComponentCore<Datum, BrushConfigInterface<Da
 
   constructor (config?: BrushConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     const directions: BrushHandleType[] = [{ type: BrushDirection.West }, { type: BrushDirection.East }]
     this.unselectedRange = this.g

packages/ts/src/components/bullet-legend/index.ts

@@ -37,7 +37,7 @@ export class BulletLegend {
 
     this.element = this.div.node()
 
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   setConfig (config: BulletLegendConfigInterface): void {

packages/ts/src/components/chord-diagram/index.ts

@@ -76,7 +76,7 @@ export class ChordDiagram<
 
   constructor (config?: ChordDiagramConfigInterface<N, L>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.background = this.g.append('rect').attr('class', s.background)
     this.linkGroup = this.g.append('g').attr('class', s.links)

packages/ts/src/components/crosshair/index.ts

@@ -89,7 +89,7 @@ export class Crosshair<Datum> extends XYComponentCore<Datum, CrosshairConfigInte
 
   constructor (config?: CrosshairConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.g.style('opacity', 0)
     this.line = this.g.append('line')

packages/ts/src/components/donut/index.ts

@@ -46,7 +46,7 @@ export class Donut<Datum> extends ComponentCore<Datum[], DonutConfigInterface<Da
 
   constructor (config?: DonutConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     this.arcBackground = this.g.append('path')
     this.arcGroup = this.g.append('g')
     this.centralLabel = this.g.append('text')

packages/ts/src/components/flow-legend/index.ts

@@ -35,7 +35,7 @@ export class FlowLegend {
 
     this.labels = this.div.append('div')
 
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   setConfig (config: FlowLegendConfigInterface): void {

packages/ts/src/components/free-brush/index.ts

@@ -25,7 +25,7 @@ export class FreeBrush<Datum> extends XYComponentCore<Datum, FreeBrushConfigInte
 
   constructor (config: FreeBrushConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.brush = this.g
       .append('g')

packages/ts/src/components/graph/index.ts

@@ -157,7 +157,7 @@ export class Graph<
 
   constructor (config?: GraphConfigInterface<N, L>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this._backgroundRect = this.g.append('rect').attr('class', generalSelectors.background)
     this._graphGroup = this.g.append('g').attr('class', generalSelectors.graphGroup)

packages/ts/src/components/grouped-bar/index.ts

@@ -39,7 +39,7 @@ export class GroupedBar<Datum> extends XYComponentCore<Datum, GroupedBarConfigIn
 
   constructor (config?: GroupedBarConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   get bleed (): Spacing {

packages/ts/src/components/leaflet-map/index.ts

@@ -124,7 +124,7 @@ export class LeafletMap<Datum extends GenericDataRecord> extends ComponentCore<D
       .attr('class', s.root)
       .attr('aria-hidden', true)
 
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     if (!this._container.clientWidth) {
       console.warn('Unovis | Leaflet Map: The width of the container is not set. Setting to 100%.')
@@ -184,9 +184,9 @@ export class LeafletMap<Datum extends GenericDataRecord> extends ComponentCore<D
 
         this._map.leaflet.setView(initialMapCenter, initialMapZoom)
 
-        if (isDarkThemeEnabled() && config.styleDarkTheme) {
+        if (isDarkThemeEnabled() && this.config.styleDarkTheme) {
           this._isDarkThemeActive = true
-          this.setTheme(config.styleDarkTheme)
+          this.setTheme(this.config.styleDarkTheme)
         }
 
         this.config.onMapInitialized?.()
@@ -226,10 +226,10 @@ export class LeafletMap<Datum extends GenericDataRecord> extends ComponentCore<D
   setConfig (config: LeafletMapConfigInterface<Datum>): void {
     super.setConfig(config)
 
-    if (config.width) this._containerSelection.style('width', isString(config.width) ? config.width : `${config.width}px`)
-    if (config.height) this._containerSelection.style('height', isString(config.height) ? config.height : `${config.height}px`)
+    if (config?.width) this._containerSelection.style('width', isString(config.width) ? config.width : `${config.width}px`)
+    if (config?.height) this._containerSelection.style('height', isString(config.height) ? config.height : `${config.height}px`)
 
-    if (this._map && config.renderer === LeafletMapRenderer.MapLibre) {
+    if (this._map && config?.renderer === LeafletMapRenderer.MapLibre) {
       const layer = this._map.layer as any // Using any because the typings are not full
       const maplibreMap = layer.getMaplibreMap()
       if (maplibreMap.isStyleLoaded()) updateTopoJson(maplibreMap, this.config)
@@ -242,7 +242,7 @@ export class LeafletMap<Datum extends GenericDataRecord> extends ComponentCore<D
     }
 
     // Apply the `aria-label` attribute
-    this._containerSelection.attr('aria-label', config.ariaLabel)
+    this._containerSelection.attr('aria-label', this.config.ariaLabel)
   }
 
   setData (data: Datum[]): void {

packages/ts/src/components/line/index.ts

@@ -42,7 +42,7 @@ export class Line<Datum> extends XYComponentCore<Datum, LineConfigInterface<Datu
 
   constructor (config?: LineConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   get bleed (): Spacing {

packages/ts/src/components/nested-donut/index.ts

@@ -54,7 +54,7 @@ NestedDonutConfigInterface<Datum>
 
   constructor (config?: NestedDonutConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     this.arcBackground = this.g.append('g')
     this.arcGroup = this.g.append('g')
       .attr('class', s.segmentsGroup)

packages/ts/src/components/plotband/index.ts

@@ -25,7 +25,7 @@ export class Plotband<Datum> extends XYComponentCore<Datum, PlotbandConfigInterf
 
   constructor (config: PlotbandConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.plotband = this.g.append('rect')
       .attr('class', s.plotband)

packages/ts/src/components/plotline/index.ts

@@ -24,7 +24,7 @@ export class Plotline<Datum> extends XYComponentCore<Datum, PlotlineConfigInterf
 
   constructor (config: PlotlineConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     this.plotline = this.g
       .append('line')

packages/ts/src/components/radial-bar/index.ts

@@ -50,7 +50,7 @@ export class RadialBar<Datum> extends ComponentCore<Datum[], RadialBarConfigInte
 
   constructor (config?: RadialBarConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     this.trackGroup = this.g.append('g')
     this.barGroup = this.g.append('g')
     this.centralLabel = this.g.append('text')

packages/ts/src/components/rolling-pin-legend/index.ts

@@ -27,7 +27,7 @@ export class RollingPinLegend {
 
     this.element = this.div.node()
 
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   setConfig (config: RollingPinLegendConfigInterface): void {

packages/ts/src/components/sankey/index.ts

@@ -91,7 +91,7 @@ export class Sankey<
 
   constructor (config?: SankeyConfigInterface<N, L>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     // eslint-disable-next-line @typescript-eslint/naming-convention
     this._gNode = this.g.node() as (SVGGElement & { __zoom: ZoomTransform })
     this._backgroundRect = this.g.append('rect').attr('class', s.background).style('pointer-events', 'all')

packages/ts/src/components/scatter/index.ts

@@ -51,7 +51,7 @@ export class Scatter<Datum> extends XYComponentCore<Datum, ScatterConfigInterfac
 
   constructor (config?: ScatterConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   setConfig (config: ScatterConfigInterface<Datum>): void {

packages/ts/src/components/stacked-bar/index.ts

@@ -38,7 +38,7 @@ export class StackedBar<Datum> extends XYComponentCore<Datum, StackedBarConfigIn
 
   constructor (config?: StackedBarConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   get bleed (): Spacing {

packages/ts/src/components/timeline/index.ts

@@ -82,7 +82,7 @@ export class Timeline<Datum> extends XYComponentCore<Datum, TimelineConfigInterf
 
   constructor (config?: TimelineConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
 
     // Invisible background rect to track events
     this._background = this.g.append('rect').attr('class', s.background)

packages/ts/src/components/topojson-map/index.ts

@@ -142,7 +142,7 @@ export class TopoJSONMap<
       .on('zoom', this._onZoom.bind(this))
       .on('end', this._onZoomEnd.bind(this))
 
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     if (data) this.setData(data)
 
     this.g.append('defs')

packages/ts/src/components/treemap/index.ts

@@ -40,7 +40,7 @@ export class Treemap<Datum> extends ComponentCore<Datum[], TreemapConfigInterfac
 
   constructor (config?: TreemapConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
     this.tiles = this.g.append('g').attr('class', s.tiles)
   }
 

packages/ts/src/components/xy-labels/index.ts

@@ -29,7 +29,7 @@ export class XYLabels<Datum> extends XYComponentCore<Datum, XYLabelsConfigInterf
 
   constructor (config?: XYLabelsConfigInterface<Datum>) {
     super()
-    if (config) this.setConfig(config)
+    this.setConfig(config)
   }
 
   _render (customDuration?: number): void {

packages/ts/src/containers/xy-container/index.ts

@@ -20,7 +20,7 @@ import { ScaleDimension } from 'types/scale'
 import { Direction } from 'types/direction'
 
 // Utils
-import { clamp, clean, flatten, isEqual } from 'utils/data'
+import { clamp, clean, flatten } from 'utils/data'
 import { guid } from 'utils/misc'
 
 // Config
@@ -117,6 +117,7 @@ export class XYContainer<Datum> extends ContainerCore {
     const { components, config } = this
     if (!data) return
 
+    const hasDataUpdated = this.datamodel.data !== data
     this.datamodel.data = data
 
     components.forEach((c) => {
@@ -130,7 +131,6 @@ export class XYContainer<Datum> extends ContainerCore {
     // Hide tooltip and crosshair if the data has changed
     // Important: We still want to do `setData` for the components above even if the data hasn't changed
     // because calling `updateContainer` may add new components and we need to pass them the data
-    const hasDataUpdated = !isEqual(this.datamodel.data, data)
     if (hasDataUpdated) {
       config.tooltip?.hide()
       config.crosshair?.hide()

packages/ts/src/core/xy-component/index.ts

@@ -45,8 +45,10 @@ export class XYComponentCore<
 
   setConfig (config: ConfigInterface): void {
     // We don't allow changing scales after the component has been initialized
-    if (this.config?.xScale) config.xScale = this.config.xScale
-    if (this.config?.yScale) config.yScale = this.config.yScale
+    if (config) {
+      if (this.config?.xScale) config.xScale = this.config.xScale
+      if (this.config?.yScale) config.yScale = this.config.yScale
+    }
 
     super.setConfig(config)
   }

packages/ts/src/utils/data.ts

@@ -29,6 +29,8 @@ export const isEqual = (
   skipKeys: string[] = [],
   visited: Set<any> = new Set()
 ): boolean => {
+  if (a === b) return true
+
   if (Array.isArray(a)) {
     if (!Array.isArray(b) || a.length !== b.length) return false
 
@@ -114,7 +116,9 @@ export const cloneDeep = <T>(obj: T, stack: Map<any, any> = new Map()): T => {
 export const merge = <T, K>(obj1: T, obj2: K, visited: Map<any, any> = new Map()): T & K => {
   type Rec = Record<string | number, unknown>
 
-  if (!obj1 || !obj2) return obj1 as T & K
+  // Clone `obj1` even when there's nothing to merge, so that callers never get a reference
+  // to the original object (e.g. a shared default config) that they could accidentally mutate
+  if (!obj1 || !obj2) return (obj1 ? cloneDeep(obj1) : obj1) as T & K
   if ((obj1 as unknown) === (obj2 as unknown)) return obj1 as T & K
 
   const newObj = (isAClassInstance(obj1 as Rec) ? obj1 : cloneDeep(obj1)) as T & K
@@ -129,9 +133,8 @@ export const merge = <T, K>(obj1: T, obj2: K, visited: Map<any, any> = new Map()
 
     if (isPlainObject((obj1 as Rec)[key]) && isPlainObject((obj2 as Rec)[key])) {
       (newObj as Rec)[key] = merge((obj1 as Rec)[key], (obj2 as Rec)[key], visited)
-    } else if (isAClassInstance(obj2 as Rec)) {
-      (newObj as Rec)[key] = obj2
     } else {
+      // `cloneDeep` copies class instances and primitives by reference, and clones plain objects / arrays
       (newObj as Rec)[key] = cloneDeep((obj2 as Rec)[key])
     }
   })