diff --git a/.changeset/quiet-tables-watch.md b/.changeset/quiet-tables-watch.md new file mode 100644 index 000000000..678a911ff --- /dev/null +++ b/.changeset/quiet-tables-watch.md @@ -0,0 +1,5 @@ +--- +'@tanstack/vue-virtual': patch +--- + +Detach scroll observers when the scroll element becomes null. diff --git a/packages/vue-virtual/package.json b/packages/vue-virtual/package.json index 706a6fa13..4fdb3410a 100644 --- a/packages/vue-virtual/package.json +++ b/packages/vue-virtual/package.json @@ -28,9 +28,11 @@ ], "scripts": { "clean": "premove ./dist ./coverage", - "test:eslint": "eslint ./src", + "test:eslint": "eslint ./src ./tests", "test:types": "tsc", "test:build": "publint --strict", + "test:lib": "vitest", + "test:lib:dev": "pnpm run test:lib --watch", "build": "vite build" }, "type": "module", diff --git a/packages/vue-virtual/src/index.ts b/packages/vue-virtual/src/index.ts index 57f73311c..8fbd418cc 100644 --- a/packages/vue-virtual/src/index.ts +++ b/packages/vue-virtual/src/index.ts @@ -35,10 +35,8 @@ function useVirtualizerBase< watch( () => unref(options).getScrollElement(), - (el) => { - if (el) { - virtualizer._willUpdate() - } + () => { + virtualizer._willUpdate() }, { immediate: true, diff --git a/packages/vue-virtual/tests/index.test.ts b/packages/vue-virtual/tests/index.test.ts new file mode 100644 index 000000000..3fb223527 --- /dev/null +++ b/packages/vue-virtual/tests/index.test.ts @@ -0,0 +1,117 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' +import { effectScope, nextTick, shallowRef } from 'vue' +import { useVirtualizer, useWindowVirtualizer } from '../src' +import type { EffectScope } from 'vue' + +describe('scroll element lifecycle', () => { + let scope: EffectScope + + beforeEach(() => { + scope = effectScope() + }) + + afterEach(() => { + scope.stop() + }) + + function setup(initialElement: T | null) { + const scrollElement = shallowRef(initialElement) + const cleanupRect = vi.fn() + const cleanupOffset = vi.fn() + const options = { + count: 10, + estimateSize: () => 50, + getScrollElement: () => scrollElement.value, + observeElementRect: vi.fn(() => cleanupRect), + observeElementOffset: vi.fn(() => cleanupOffset), + scrollToFn: vi.fn(), + } + + return { scrollElement, cleanupRect, cleanupOffset, options } + } + + test('attaches observers when an initially null element becomes available', async () => { + const { scrollElement, options } = setup(null) + const virtualizer = scope.run(() => useVirtualizer(options))! + + expect(virtualizer.value.scrollElement).toBeNull() + expect(options.observeElementRect).not.toHaveBeenCalled() + expect(options.observeElementOffset).not.toHaveBeenCalled() + + scrollElement.value = document.createElement('div') + await nextTick() + + expect(virtualizer.value.scrollElement).toBe(scrollElement.value) + expect(options.observeElementRect).toHaveBeenCalledTimes(1) + expect(options.observeElementOffset).toHaveBeenCalledTimes(1) + }) + + test('cleans up observers when the element becomes null and reattaches later', async () => { + const element = document.createElement('div') + const { scrollElement, cleanupRect, cleanupOffset, options } = + setup(element) + const virtualizer = scope.run(() => useVirtualizer(options))! + + expect(virtualizer.value.scrollElement).toBe(element) + + scrollElement.value = null + await nextTick() + + expect(cleanupRect).toHaveBeenCalledTimes(1) + expect(cleanupOffset).toHaveBeenCalledTimes(1) + expect(virtualizer.value.scrollElement).toBeNull() + expect(virtualizer.value.targetWindow).toBeNull() + + scrollElement.value = element + await nextTick() + + expect(virtualizer.value.scrollElement).toBe(element) + expect(options.observeElementRect).toHaveBeenCalledTimes(2) + expect(options.observeElementOffset).toHaveBeenCalledTimes(2) + + scope.stop() + expect(cleanupRect).toHaveBeenCalledTimes(2) + expect(cleanupOffset).toHaveBeenCalledTimes(2) + expect(virtualizer.value.scrollElement).toBeNull() + }) + + test('cleans up observers on element replacement and scope disposal', async () => { + const { scrollElement, cleanupRect, cleanupOffset, options } = setup( + document.createElement('div'), + ) + const virtualizer = scope.run(() => useVirtualizer(options))! + + scrollElement.value = document.createElement('div') + await nextTick() + + expect(cleanupRect).toHaveBeenCalledTimes(1) + expect(cleanupOffset).toHaveBeenCalledTimes(1) + expect(virtualizer.value.scrollElement).toBe(scrollElement.value) + expect(options.observeElementRect).toHaveBeenCalledTimes(2) + expect(options.observeElementOffset).toHaveBeenCalledTimes(2) + + scope.stop() + expect(cleanupRect).toHaveBeenCalledTimes(2) + expect(cleanupOffset).toHaveBeenCalledTimes(2) + expect(virtualizer.value.scrollElement).toBeNull() + }) + + test('cleans up window observers when the window getter becomes null', async () => { + const { scrollElement, cleanupRect, cleanupOffset, options } = + setup(window) + const virtualizer = scope.run(() => useWindowVirtualizer(options))! + + expect(virtualizer.value.scrollElement).toBe(window) + + scrollElement.value = null + await nextTick() + + expect(cleanupRect).toHaveBeenCalledTimes(1) + expect(cleanupOffset).toHaveBeenCalledTimes(1) + expect(virtualizer.value.scrollElement).toBeNull() + + scope.stop() + expect(cleanupRect).toHaveBeenCalledTimes(1) + expect(cleanupOffset).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/vue-virtual/tsconfig.json b/packages/vue-virtual/tsconfig.json index b36d1bf92..050561671 100644 --- a/packages/vue-virtual/tsconfig.json +++ b/packages/vue-virtual/tsconfig.json @@ -1,4 +1,4 @@ { "extends": "../../tsconfig.json", - "include": ["src", "vite.config.ts"] + "include": ["src", "tests", "vite.config.ts"] } diff --git a/packages/vue-virtual/vite.config.ts b/packages/vue-virtual/vite.config.ts index a8d4b401f..6edd13337 100644 --- a/packages/vue-virtual/vite.config.ts +++ b/packages/vue-virtual/vite.config.ts @@ -1,9 +1,16 @@ import { defineConfig, mergeConfig } from 'vitest/config' import { tanstackViteConfig } from '@tanstack/vite-config' import vue from '@vitejs/plugin-vue' +import packageJson from './package.json' const config = defineConfig({ plugins: [vue()], + test: { + name: packageJson.name, + dir: './tests', + watch: false, + environment: 'jsdom', + }, }) export default mergeConfig(