refactor: remove allowSignalWrites

qupaya · Oct 14, 2024 · 9afd028 · 9afd028
1 parent 2682369
commit 9afd028
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 87 deletions.
diff --git a/apps/demo-app/src/app/pages/select-sample/select-with-style/select-with-style.component.ts b/apps/demo-app/src/app/pages/select-sample/select-with-style/select-with-style.component.ts
@@ -1,4 +1,4 @@
-import { Component, effect, signal } from '@angular/core';
+import { Component, computed, signal } from '@angular/core';
 import { CommonModule } from '@angular/common';
 import {
   faArrowAltCircleDown,
@@ -55,7 +55,9 @@ import { FormsModule } from '@angular/forms';
 })
 export class SelectWithStyleComponent {
   readonly showAnimation = signal(false);
-  readonly animateOptions = signal<'hidden' | 'visible'>('hidden');
+  readonly animateOptions = computed<'hidden' | 'visible'>(() =>
+    this.showAnimation() ? 'visible' : 'hidden'
+  );
   readonly multiple = signal(false);
   readonly selectedValues = signal<SelectDemoOption[]>([]);
   readonly options: SelectDemoOption[] = [
@@ -94,10 +96,5 @@ export class SelectWithStyleComponent {
     this.value = value;
   }
 
-  protected readonly runAnimation = effect(
-    () => this.animateOptions.set(this.showAnimation() ? 'visible' : 'hidden'),
-    { allowSignalWrites: true }
-  );
-
   protected readonly Array = Array;
 }
diff --git a/libs/sketch/src/lib/components/dialog/dialog.component.ts b/libs/sketch/src/lib/components/dialog/dialog.component.ts
@@ -52,21 +52,18 @@ export class DialogComponent {
   private readonly dialogElement =
     viewChild.required<ElementRef<HTMLDialogElement>>('dialogElement');
 
-  protected readonly openEvents = effect(
-    () => {
-      const dialog = untracked(this.dialogElement);
-      const containerRef = untracked(this.dialogOverlayContainerRef);
+  protected readonly openEvents = effect(() => {
+    const dialog = untracked(this.dialogElement);
+    const containerRef = untracked(this.dialogOverlayContainerRef);
 
-      if (this.open()) {
-        dialog.nativeElement.showModal();
-        this.overlayContainer.addContainer(containerRef.nativeElement);
-      } else {
-        dialog.nativeElement.close();
-        this.overlayContainer.removeContainer();
-      }
-    },
-    { allowSignalWrites: true }
-  );
+    if (this.open()) {
+      dialog.nativeElement.showModal();
+      this.overlayContainer.addContainer(containerRef.nativeElement);
+    } else {
+      dialog.nativeElement.close();
+      this.overlayContainer.removeContainer();
+    }
+  });
 
   handleEscape(event: Event): void {
     this.close.emit();

diff --git a/libs/sketch/src/lib/components/list/components/list-item/list-item.component.ts b/libs/sketch/src/lib/components/list/components/list-item/list-item.component.ts
@@ -1,15 +1,17 @@
 import {
   Component,
   computed,
-  effect,
   inject,
   signal,
-  untracked,
   ViewEncapsulation,
+  OnInit,
+  DestroyRef,
 } from '@angular/core';
 import { ListItemActiveDirective } from '../../directives/list-item-active.directive';
 import { ListService } from '../../services/list.service';
 import { ListComponent } from '../../list.component';
+import { takeUntilDestroyed, toObservable } from '@angular/core/rxjs-interop';
+import { filter, NEVER, withLatestFrom } from 'rxjs';
 
 @Component({
   selector: 'sk-list-item',
@@ -18,7 +20,8 @@ import { ListComponent } from '../../list.component';
   styleUrl: './list-item.component.css',
   encapsulation: ViewEncapsulation.ShadowDom,
 })
-export class ListItemComponent {
+export class ListItemComponent implements OnInit {
+  private readonly destroyRef = inject(DestroyRef);
   private readonly listService = inject(ListService);
   private readonly activeItem = inject(ListItemActiveDirective, {
     optional: true,
@@ -41,28 +44,41 @@ export class ListItemComponent {
     return id ? this.listService.isParentActive(id) : true;
   });
 
-  readonly registerListItem = effect(
-    () => {
-      const path = this.activeItem?.itemId();
-      if (path) {
-        this.listService.registerItem(path, this.parent?.activeItem?.itemId());
-      }
-    },
-    { allowSignalWrites: true }
-  );
+  readonly listItems$ = toObservable(this.listService.items);
+  readonly activeItem$ = this.activeItem
+    ? toObservable(this.activeItem.itemId)
+    : NEVER;
 
-  readonly updateActiveState = effect(
-    () => {
-      if (this.activeItem) {
-        const items = this.listService.items();
-        const currentLink = untracked(this.activeItem.itemId);
-        const item = items.find(({ id }) => id === currentLink);
-        if (this.parentList && item?.active) {
-          this.parentList.activateItem(item?.id);
-        }
-        this.active.set(item?.active ?? false);
-      }
-    },
-    { allowSignalWrites: true }
-  );
+  ngOnInit(): void {
+    this.registerListItem();
+    this.updateActiveState();
+  }
+
+  private registerListItem(): void {
+    this.activeItem$
+      .pipe(
+        filter((path) => !!path),
+        takeUntilDestroyed(this.destroyRef)
+      )
+      .subscribe((path) =>
+        this.listService.registerItem(path, this.parent?.activeItem?.itemId())
+      );
+  }
+
+  private updateActiveState(): void {
+    if (this.activeItem) {
+      this.listItems$
+        .pipe(
+          withLatestFrom(this.activeItem$),
+          takeUntilDestroyed(this.destroyRef)
+        )
+        .subscribe(([items, currentLink]) => {
+          const item = items.find(({ id }) => id === currentLink);
+          if (this.parentList && item?.active) {
+            this.parentList.activateItem(item?.id);
+          }
+          this.active.set(item?.active ?? false);
+        });
+    }
+  }
 }
diff --git a/libs/sketch/src/lib/components/overlay/overlay.directive.ts b/libs/sketch/src/lib/components/overlay/overlay.directive.ts
@@ -60,18 +60,15 @@ export class CdkOverlayDirective {
   private _relatedElement?: HTMLElement =
     this.relativeTo() || this.elementRef.nativeElement;
 
-  protected readonly detectVisibleChange = effect(
-    () => {
-      if (this._relatedElement) {
-        if (this.showOverlay()) {
-          this.createOverlay();
-        } else {
-          this.hide();
-        }
+  protected readonly detectVisibleChange = effect(() => {
+    if (this._relatedElement) {
+      if (this.showOverlay()) {
+        this.createOverlay();
+      } else {
+        this.hide();
       }
-    },
-    { allowSignalWrites: true }
-  );
+    }
+  });
 
   protected readonly updateOverlayPortal = effect(() => {
     if (this.windowResize && this.windowResize()) {

diff --git a/libs/sketch/src/lib/components/select/select.component.ts b/libs/sketch/src/lib/components/select/select.component.ts
@@ -3,10 +3,10 @@ import {
   ChangeDetectorRef,
   Component,
   computed,
-  effect,
   forwardRef,
   HostListener,
   inject,
+  Input,
   input,
   output,
   signal,
@@ -36,11 +36,10 @@ import { CdkOverlayDirective } from '../overlay/overlay.directive';
 export class SelectComponent<T> implements ControlValueAccessor {
   private readonly changeDetectorRef = inject(ChangeDetectorRef);
 
-  animationDelay = input(0);
-  closeOnSelect = input(false);
-  panelOffsetX = input(0);
-  panelOffsetY = input(0);
-  multiple = input(false, { transform: booleanAttribute });
+  readonly animationDelay = input(0);
+  readonly closeOnSelect = input(false);
+  readonly panelOffsetX = input(0);
+  readonly panelOffsetY = input(0);
 
   readonly autoFocus = signal(true);
   readonly selectedValue = signal<T | T[] | undefined>(undefined);
@@ -53,6 +52,30 @@ export class SelectComponent<T> implements ControlValueAccessor {
     );
   });
 
+  private readonly isMultiple = signal(false);
+
+  @Input({ transform: booleanAttribute })
+  set multiple(value: boolean) {
+    this.isMultiple.set(value);
+
+    const selectedValue = untracked(this.selectedValue);
+    if (!value) {
+      this.selectedValue.set(
+        Array.isArray(selectedValue) ? selectedValue[0] : selectedValue
+      );
+    } else {
+      this.selectedValue.set(
+        Array.isArray(selectedValue)
+          ? selectedValue
+          : selectedValue
+          ? [selectedValue]
+          : undefined
+      );
+    }
+    this.onChange?.(this.selectedValue());
+    this.onTouched?.();
+  }
+
   readonly open = output<boolean>();
 
   @HostListener('document:keydown.escape')
@@ -62,35 +85,13 @@ export class SelectComponent<T> implements ControlValueAccessor {
     }
   }
 
-  protected readonly updateSelectionMode = effect(
-    () => {
-      const selectedValue = untracked(this.selectedValue);
-      if (!this.multiple()) {
-        this.selectedValue.set(
-          Array.isArray(selectedValue) ? selectedValue[0] : selectedValue
-        );
-      } else {
-        this.selectedValue.set(
-          Array.isArray(selectedValue)
-            ? selectedValue
-            : selectedValue
-            ? [selectedValue]
-            : undefined
-        );
-      }
-      this.onChange?.(this.selectedValue());
-      this.onTouched?.();
-    },
-    { allowSignalWrites: true }
-  );
-
   togglePanel(visible: boolean): void {
     this.panelIsVisible.set(visible);
     this.open.emit(visible);
   }
 
   selectionChanged(value: T): void {
-    if (this.multiple()) {
+    if (this.isMultiple()) {
       this.selectedValue.update((selected) => {
         if (Array.isArray(selected)) {
           return selected.includes(value)