refactor: drop signal writes effect (#455)

Harpush · web-flow · commit f2219e8442a8 · 2025-01-19T19:55:19.000+01:00
diff --git a/projects/angular-split/src/lib/split-area/split-area.component.ts b/projects/angular-split/src/lib/split-area/split-area.component.ts
@@ -47,9 +47,9 @@ export class SplitAreaComponent {
         return 0
       }
 
-      const size = this.size()
-      // auto acts the same as * in all calculations
-      return size === 'auto' ? '*' : size
+      const visibleIndex = this.split._visibleAreas().findIndex((area) => area === this)
+
+      return this.split._alignedVisibleAreasSizes()[visibleIndex]
     }),
   )
   /**
diff --git a/projects/angular-split/src/lib/split/split.component.ts b/projects/angular-split/src/lib/split/split.component.ts
@@ -13,9 +13,9 @@ import {
   effect,
   inject,
   input,
+  isDevMode,
   output,
   signal,
-  untracked,
 } from '@angular/core'
 import { takeUntilDestroyed } from '@angular/core/rxjs-interop'
 import type { SplitAreaComponent } from '../split-area/split-area.component'
@@ -31,6 +31,7 @@ import {
   numberAttributeWithFallback,
   sum,
   toRecord,
+  assertUnreachable,
 } from '../utils'
 import { DOCUMENT, NgStyle, NgTemplateOutlet } from '@angular/common'
 import { SplitGutterInteractionEvent, SplitAreaSize } from '../models'
@@ -115,56 +116,11 @@ export class SplitComponent {
 
   readonly dragProgress$ = this.dragProgressSubject.asObservable()
 
-  private readonly visibleAreas = computed(() => this._areas().filter((area) => area.visible()))
-  private readonly gridTemplateColumnsStyle = computed(() => {
-    const columns: string[] = []
-    const sumNonWildcardSizes = sum(this.visibleAreas(), (area) => {
-      const size = area._internalSize()
-      return size === '*' ? 0 : size
-    })
-    const visibleAreasCount = this.visibleAreas().length
-
-    let visitedVisibleAreas = 0
-
-    this._areas().forEach((area, index, areas) => {
-      const unit = this.unit()
-      const areaSize = area._internalSize()
-
-      // Add area size column
-      if (!area.visible()) {
-        columns.push(unit === 'percent' || areaSize === '*' ? '0fr' : '0px')
-      } else {
-        if (unit === 'pixel') {
-          const columnValue = areaSize === '*' ? '1fr' : `${areaSize}px`
-          columns.push(columnValue)
-        } else {
-          const percentSize = areaSize === '*' ? 100 - sumNonWildcardSizes : areaSize
-          const columnValue = `${percentSize}fr`
-          columns.push(columnValue)
-        }
-
-        visitedVisibleAreas++
-      }
-
-      const isLastArea = index === areas.length - 1
-
-      if (isLastArea) {
-        return
-      }
-
-      const remainingVisibleAreas = visibleAreasCount - visitedVisibleAreas
-
-      // Only add gutter with size if this area is visible and there are more visible areas after this one
-      // to avoid ghost gutters
-      if (area.visible() && remainingVisibleAreas > 0) {
-        columns.push(`${this.gutterSize()}px`)
-      } else {
-        columns.push('0px')
-      }
-    })
-
-    return this.direction() === 'horizontal' ? `1fr / ${columns.join(' ')}` : `${columns.join(' ')} / 1fr`
-  })
+  /**
+   * @internal
+   */
+  readonly _visibleAreas = computed(() => this._areas().filter((area) => area.visible()))
+  private readonly gridTemplateColumnsStyle = computed(() => this.createGridTemplateColumnsStyle())
   private readonly hostClasses = computed(() =>
     createClassesString({
       [`as-${this.direction()}`]: true,
@@ -179,6 +135,11 @@ export class SplitComponent {
    * @internal
    */
   readonly _isDragging = computed(() => this.draggedGutterIndex() !== undefined)
+  /**
+   * @internal
+   * Should only be used by {@link SplitAreaComponent._internalSize}
+   */
+  readonly _alignedVisibleAreasSizes = computed(() => this.createAlignedVisibleAreasSize())
 
   @HostBinding('class') protected get hostClassesBinding() {
     return this.hostClasses()
@@ -189,45 +150,17 @@ export class SplitComponent {
   }
 
   constructor() {
-    effect(
-      () => {
-        const visibleAreas = this.visibleAreas()
-        const unit = this.unit()
-        const isInAutoMode = visibleAreas.every((area) => area.size() === 'auto')
-
-        untracked(() => {
-          // Special mode when no size input was declared which is a valid mode
-          if (unit === 'percent' && visibleAreas.length > 1 && isInAutoMode) {
-            visibleAreas.forEach((area) => area._internalSize.set(100 / visibleAreas.length))
-            return
-          }
-
-          visibleAreas.forEach((area) => area._internalSize.reset())
-
-          const isValid = areAreasValid(visibleAreas, unit)
-
-          if (isValid) {
-            return
-          }
+    if (isDevMode()) {
+      // Logs warnings to console when the provided areas sizes are invalid
+      effect(() => {
+        // Special mode when no size input was declared which is a valid mode
+        if (this.unit() === 'percent' && this._visibleAreas().every((area) => area.size() === 'auto')) {
+          return
+        }
 
-          if (unit === 'percent') {
-            // Distribute sizes equally
-            const defaultSize = 100 / visibleAreas.length
-            visibleAreas.forEach((area) => area._internalSize.set(defaultSize))
-          } else if (unit === 'pixel') {
-            const wildcardAreas = visibleAreas.filter((area) => area._internalSize() === '*')
-
-            // Make sure only one wildcard area
-            if (wildcardAreas.length === 0) {
-              visibleAreas[0]._internalSize.set('*')
-            } else if (wildcardAreas.length > 1) {
-              wildcardAreas.filter((_, i) => i !== 0).forEach((area) => area._internalSize.set(100))
-            }
-          }
-        })
-      },
-      { allowSignalWrites: true },
-    )
+        areAreasValid(this._visibleAreas(), this.unit(), true)
+      })
+    }
 
     // Responsible for updating grid template style. Must be this way and not based on HostBinding
     // as change detection for host binding is bound to the parent component and this style
@@ -450,7 +383,7 @@ export class SplitComponent {
   }
 
   private createAreaSizes() {
-    return this.visibleAreas().map((area) => area._internalSize())
+    return this._visibleAreas().map((area) => area._internalSize())
   }
 
   private createDragStartContext(
@@ -460,7 +393,7 @@ export class SplitComponent {
   ): DragStartContext {
     const splitBoundingRect = this.elementRef.nativeElement.getBoundingClientRect()
     const splitSize = this.direction() === 'horizontal' ? splitBoundingRect.width : splitBoundingRect.height
-    const totalAreasPixelSize = splitSize - (this.visibleAreas().length - 1) * this.gutterSize()
+    const totalAreasPixelSize = splitSize - (this._visibleAreas().length - 1) * this.gutterSize()
     // Use the internal size and split size to calculate the pixel size from wildcard and percent areas
     const areaPixelSizesWithWildcard = this._areas().map((area) => {
       if (this.unit() === 'pixel') {
@@ -598,4 +531,92 @@ export class SplitComponent {
 
     this.dragProgressSubject.next(this.createDragInteractionEvent(this.draggedGutterIndex()))
   }
+
+  private createGridTemplateColumnsStyle(): string {
+    const columns: string[] = []
+    const sumNonWildcardSizes = sum(this._visibleAreas(), (area) => {
+      const size = area._internalSize()
+      return size === '*' ? 0 : size
+    })
+    const visibleAreasCount = this._visibleAreas().length
+
+    let visitedVisibleAreas = 0
+
+    this._areas().forEach((area, index, areas) => {
+      const unit = this.unit()
+      const areaSize = area._internalSize()
+
+      // Add area size column
+      if (!area.visible()) {
+        columns.push(unit === 'percent' || areaSize === '*' ? '0fr' : '0px')
+      } else {
+        if (unit === 'pixel') {
+          const columnValue = areaSize === '*' ? '1fr' : `${areaSize}px`
+          columns.push(columnValue)
+        } else {
+          const percentSize = areaSize === '*' ? 100 - sumNonWildcardSizes : areaSize
+          const columnValue = `${percentSize}fr`
+          columns.push(columnValue)
+        }
+
+        visitedVisibleAreas++
+      }
+
+      const isLastArea = index === areas.length - 1
+
+      if (isLastArea) {
+        return
+      }
+
+      const remainingVisibleAreas = visibleAreasCount - visitedVisibleAreas
+
+      // Only add gutter with size if this area is visible and there are more visible areas after this one
+      // to avoid ghost gutters
+      if (area.visible() && remainingVisibleAreas > 0) {
+        columns.push(`${this.gutterSize()}px`)
+      } else {
+        columns.push('0px')
+      }
+    })
+
+    return this.direction() === 'horizontal' ? `1fr / ${columns.join(' ')}` : `${columns.join(' ')} / 1fr`
+  }
+
+  private createAlignedVisibleAreasSize(): SplitAreaSize[] {
+    const visibleAreasSizes = this._visibleAreas().map((area): SplitAreaSize => {
+      const size = area.size()
+      return size === 'auto' ? '*' : size
+    })
+    const isValid = areAreasValid(this._visibleAreas(), this.unit(), false)
+
+    if (isValid) {
+      return visibleAreasSizes
+    }
+
+    const unit = this.unit()
+
+    if (unit === 'percent') {
+      // Distribute sizes equally
+      const defaultPercentSize = 100 / visibleAreasSizes.length
+      return visibleAreasSizes.map(() => defaultPercentSize)
+    }
+
+    if (unit === 'pixel') {
+      // Make sure only one wildcard area
+      const wildcardAreas = visibleAreasSizes.filter((areaSize) => areaSize === '*')
+
+      if (wildcardAreas.length === 0) {
+        return ['*', ...visibleAreasSizes.slice(1)]
+      } else {
+        const firstWildcardIndex = visibleAreasSizes.findIndex((areaSize) => areaSize === '*')
+        const defaultPxSize = 100
+
+        return visibleAreasSizes.map((areaSize, index) =>
+          index === firstWildcardIndex || areaSize !== '*' ? areaSize : defaultPxSize,
+        )
+      }
+    }
+
+    return assertUnreachable(unit, 'SplitUnit')
+  }
 }
diff --git a/projects/angular-split/src/lib/utils.ts b/projects/angular-split/src/lib/utils.ts
@@ -131,3 +131,7 @@ export function leaveNgZone<T>() {
 }
 
 export const numberAttributeWithFallback = (fallback: number) => (value: unknown) => numberAttribute(value, fallback)
+
+export const assertUnreachable = (value: never, name: string) => {
+  throw new Error(`as-split: unknown value "${value}" for "${name}"`)
+}
diff --git a/projects/angular-split/src/lib/validations.ts b/projects/angular-split/src/lib/validations.ts
@@ -1,17 +1,21 @@
-import { isDevMode } from '@angular/core'
-import { SplitUnit } from './models'
+import { SplitAreaSize, SplitUnit } from './models'
 import { SplitAreaComponent } from './split-area/split-area.component'
 import { sum } from './utils'
 
-export function areAreasValid(areas: readonly SplitAreaComponent[], unit: SplitUnit): boolean {
+export function areAreasValid(areas: readonly SplitAreaComponent[], unit: SplitUnit, logWarnings: boolean): boolean {
   if (areas.length === 0) {
     return true
   }
 
-  const wildcardAreas = areas.filter((area) => area._internalSize() === '*')
+  const areaSizes = areas.map((area): SplitAreaSize => {
+    const size = area.size()
+    return size === 'auto' ? '*' : size
+  })
+
+  const wildcardAreas = areaSizes.filter((areaSize) => areaSize === '*')
 
   if (wildcardAreas.length > 1) {
-    if (isDevMode()) {
+    if (logWarnings) {
       console.warn('as-split: Maximum one * area is allowed')
     }
 
@@ -23,33 +27,30 @@ export function areAreasValid(areas: readonly SplitAreaComponent[], unit: SplitU
       return true
     }
 
-    if (isDevMode()) {
+    if (logWarnings) {
       console.warn('as-split: Pixel mode must have exactly one * area')
     }
 
     return false
   }
 
-  const sumPercent = sum(areas, (area) => {
-    const size = area._internalSize()
-    return size === '*' ? 0 : size
-  })
+  const sumPercent = sum(areaSizes, (areaSize) => (areaSize === '*' ? 0 : areaSize))
 
   // As percent calculation isn't perfect we allow for a small margin of error
   if (wildcardAreas.length === 1) {
     if (sumPercent <= 100.1) {
       return true
     }
 
-    if (isDevMode()) {
+    if (logWarnings) {
       console.warn(`as-split: Percent areas must total 100%`)
     }
 
     return false
   }
 
   if (sumPercent < 99.9 || sumPercent > 100.1) {
-    if (isDevMode()) {
+    if (logWarnings) {
       console.warn('as-split: Percent areas must total 100%')
     }
 

Original file line number	Diff line number	Diff line change
`@@ -47,9 +47,9 @@ export class SplitAreaComponent {`
`47`	`47`	`return 0`
`48`	`48`	`}`
`49`	`49`
`50`		`- const size = this.size()`
`51`		`- // auto acts the same as * in all calculations`
`52`		`- return size === 'auto' ? '*' : size`
	`50`	`+ const visibleIndex = this.split._visibleAreas().findIndex((area) => area === this)`
	`51`	`+`
	`52`	`+ return this.split._alignedVisibleAreasSizes()[visibleIndex]`
`53`	`53`	`}),`
`54`	`54`	`)`
`55`	`55`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -131,3 +131,7 @@ export function leaveNgZone<T>() {`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`export const numberAttributeWithFallback = (fallback: number) => (value: unknown) => numberAttribute(value, fallback)`
	`134`	`+`
	`135`	`+export const assertUnreachable = (value: never, name: string) => {`
	`136`	+ throw new Error(`as-split: unknown value "${value}" for "${name}"`)
	`137`	`+}`