-
-
Notifications
You must be signed in to change notification settings - Fork 427
fix(angular-virtual): RuntimeError: NG0600: Writing to signals is not allowed in a computed or an effect by default #1109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@tanstack/angular-virtual': patch | ||
| --- | ||
|
|
||
| Fixes writing to signal error in angular effect |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,6 @@ import { | |||||||||||||||||||||||||||
| effect, | ||||||||||||||||||||||||||||
| inject, | ||||||||||||||||||||||||||||
| signal, | ||||||||||||||||||||||||||||
| untracked, | ||||||||||||||||||||||||||||
| } from '@angular/core' | ||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||
| Virtualizer, | ||||||||||||||||||||||||||||
|
|
@@ -16,7 +15,6 @@ import { | |||||||||||||||||||||||||||
| observeWindowRect, | ||||||||||||||||||||||||||||
| windowScroll, | ||||||||||||||||||||||||||||
| } from '@tanstack/virtual-core' | ||||||||||||||||||||||||||||
| import { proxyVirtualizer } from './proxy' | ||||||||||||||||||||||||||||
| import type { ElementRef, Signal } from '@angular/core' | ||||||||||||||||||||||||||||
| import type { PartialKeys, VirtualizerOptions } from '@tanstack/virtual-core' | ||||||||||||||||||||||||||||
| import type { AngularVirtualizer } from './types' | ||||||||||||||||||||||||||||
|
|
@@ -30,52 +28,86 @@ function createVirtualizerBase< | |||||||||||||||||||||||||||
| >( | ||||||||||||||||||||||||||||
| options: Signal<VirtualizerOptions<TScrollElement, TItemElement>>, | ||||||||||||||||||||||||||||
| ): AngularVirtualizer<TScrollElement, TItemElement> { | ||||||||||||||||||||||||||||
| let virtualizer: Virtualizer<TScrollElement, TItemElement> | ||||||||||||||||||||||||||||
| function lazyInit() { | ||||||||||||||||||||||||||||
| virtualizer ??= new Virtualizer(options()) | ||||||||||||||||||||||||||||
| return virtualizer | ||||||||||||||||||||||||||||
| const instance = new Virtualizer<TScrollElement, TItemElement>(options()) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const virtualItems = signal(instance.getVirtualItems(), { | ||||||||||||||||||||||||||||
| equal: () => false, | ||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||
| const totalSize = signal(instance.getTotalSize()) | ||||||||||||||||||||||||||||
| const isScrolling = signal(instance.isScrolling) | ||||||||||||||||||||||||||||
| const range = signal(instance.range, { equal: () => false }) | ||||||||||||||||||||||||||||
| const scrollDirection = signal(instance.scrollDirection) | ||||||||||||||||||||||||||||
| const scrollElement = signal(instance.scrollElement) | ||||||||||||||||||||||||||||
| const scrollOffset = signal(instance.scrollOffset) | ||||||||||||||||||||||||||||
| const scrollRect = signal(instance.scrollRect) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const handler = { | ||||||||||||||||||||||||||||
| get( | ||||||||||||||||||||||||||||
| target: Virtualizer<TScrollElement, TItemElement>, | ||||||||||||||||||||||||||||
| prop: keyof Virtualizer<TScrollElement, TItemElement>, | ||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||
| switch (prop) { | ||||||||||||||||||||||||||||
| case 'getVirtualItems': | ||||||||||||||||||||||||||||
| return virtualItems | ||||||||||||||||||||||||||||
| case 'getTotalSize': | ||||||||||||||||||||||||||||
| return totalSize | ||||||||||||||||||||||||||||
| case 'isScrolling': | ||||||||||||||||||||||||||||
| return isScrolling | ||||||||||||||||||||||||||||
| case 'options': | ||||||||||||||||||||||||||||
| return options | ||||||||||||||||||||||||||||
| case 'range': | ||||||||||||||||||||||||||||
| return range | ||||||||||||||||||||||||||||
| case 'scrollDirection': | ||||||||||||||||||||||||||||
| return scrollDirection | ||||||||||||||||||||||||||||
| case 'scrollElement': | ||||||||||||||||||||||||||||
| return scrollElement | ||||||||||||||||||||||||||||
| case 'scrollOffset': | ||||||||||||||||||||||||||||
| return scrollOffset | ||||||||||||||||||||||||||||
| case 'scrollRect': | ||||||||||||||||||||||||||||
| return scrollRect | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| return scrollRect | |
| return scrollRect | |
| case 'getOffsetForIndex': | |
| case 'getVirtualItemForOffset': | |
| case 'indexFromElement': | |
| // Wrap argument-taking query methods so they read a signal and stay reactive | |
| return (...args: unknown[]) => { | |
| // Access a signal that updates on virtualizer changes to register dependency | |
| virtualItems() | |
| // Delegate to the underlying Virtualizer method | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| return (target as any)[prop](...args) | |
| } |
Copilot
AI
Mar 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The adapter signals (virtualItems, totalSize, range, etc.) are only updated inside the onChange callback. However, Virtualizer.setOptions() does not itself trigger notify(), and _willUpdate() only notifies when the scroll element changes (or via subsequent scroll/resize observers). This means option-only changes (e.g. count changes from a signal, which is a documented use case) can leave these signals stale until an unrelated scroll/resize happens. To keep the Angular signals consistent, update them once after applying options (or otherwise force a notify) within this effect run.
| }) | |
| }) | |
| // Ensure adapter signals stay in sync even when only options change | |
| virtualItems.set(instance.getVirtualItems()) | |
| totalSize.set(instance.getTotalSize()) | |
| isScrolling.set(instance.isScrolling) | |
| range.set(instance.range) | |
| scrollDirection.set(instance.scrollDirection) | |
| scrollElement.set(instance.scrollElement) | |
| scrollOffset.set(instance.scrollOffset) | |
| scrollRect.set(instance.scrollRect) |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optionsproperty on the returnedAngularVirtualizerproxy is currently mapped to the inputoptionssignal rather thaninstance.options(the fully-normalized options object thatVirtualizeractually uses). This changes runtime semantics and also diverges fromAngularVirtualizer['options'](which is typed asSignal<Virtualizer['options']>). Consider exposing a dedicatedoptionssignal that is updated frominstance.options(e.g. inonChangeand/or aftersetOptions) instead of returning the adapter’soptionsinput signal here.