diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1240b2010..0131de37c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,8 +69,8 @@ jobs: - name: ๐Ÿš€ Build run: pnpm build - # - name: ๐Ÿงช Test with coverage - # run: pnpm coverage + - name: ๐Ÿงช Test + run: pnpm test # - name: ๐Ÿ“ Upload coverage # if: always() diff --git a/package.json b/package.json index 8a953d540..b523f3d29 100644 --- a/package.json +++ b/package.json @@ -18,9 +18,7 @@ "story": "pnpm storybook dev -p 6006 --no-open", "lint": "eslint . --cache", "lint:fix": "eslint . --fix --cache", - "test": "vitest run", - "test:watch": "vitest --watch", - "test:nuxt": "vitest -c vitest.nuxt.config.ts --coverage", + "test": "pnpm --filter '@oku-ui/primitives' test", "coverage": "vitest run --coverage", "build:storybook": "pnpm storybook build", "typecheck": "tsc --noEmit", diff --git a/packages/core/.gitignore b/packages/core/.gitignore index 3b3246f37..8ad341737 100644 --- a/packages/core/.gitignore +++ b/packages/core/.gitignore @@ -32,3 +32,4 @@ storybook-static *.tsbuildinfo *storybook.log +.data \ No newline at end of file diff --git a/packages/core/package.json b/packages/core/package.json index 7cddf0d67..658fef781 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -92,6 +92,9 @@ "build-storybook": "storybook build", "eslint": "eslint .", "eslint:fix": "eslint . --fix", + "test": "vitest run", + "test:watch": "vitest --watch", + "test:nuxt": "vitest -c vitest.nuxt.config.ts --coverage", "release": "pnpm build && pnpm publish --no-git-checks --access public", "release:beta": "pnpm release --tag beta --access public", "release:alpha": "pnpm release --tag alpha --access public", diff --git a/packages/core/src/hooks/index.ts b/packages/core/src/hooks/index.ts index cb835d695..e071aaadb 100644 --- a/packages/core/src/hooks/index.ts +++ b/packages/core/src/hooks/index.ts @@ -1,7 +1,7 @@ export { createContext } from './createContext.ts' export { useBodyScrollLock } from './useBodyScrollLock.ts' export { useComposedElements } from './useComposedElements.ts' -export { useControllableState, useControllableStateV2, useControllableStateV3 } from './useControllableState.ts' +export { useControllableState, useControllableStateV2, useControllableStateV3, useControllableStateV4 } from './useControllableState.ts' export { useEscapeKeydown } from './useEscapeKeydown.ts' export { useForwardElement } from './useForwardElement.ts' export { useId } from './useId.ts' diff --git a/packages/core/src/hooks/tests/performance-results.ts b/packages/core/src/hooks/tests/performance-results.ts new file mode 100644 index 000000000..d24dfcbfd --- /dev/null +++ b/packages/core/src/hooks/tests/performance-results.ts @@ -0,0 +1,115 @@ +import { existsSync, mkdirSync, writeFileSync } from 'node:fs' +import { resolve } from 'node:path' + +interface PerformanceResult { + version: string + updateCount: number + executionTimeMs: number + onChangeCallCount: number + timestamp: string +} + +export class PerformanceLogger { + private static results: PerformanceResult[] = [] + private static readonly outputPath = resolve(__dirname, './.data/performance-results.json') + + static logResult(version: string, executionTimeMs: number, onChangeCallCount: number, updateCount = 5) { + const result = { + version, + updateCount, + executionTimeMs, + onChangeCallCount, + timestamp: new Date().toISOString(), + } + + this.results.push(result) + this.writeToFile() // Write to file after each new result + return result + } + + static getResults() { + return [...this.results] + } + + static compareVersions() { + const v3Results = this.results.filter(r => r.version === 'useControllableStateV3') + const v4Results = this.results.filter(r => r.version === 'useControllableStateV4') + const vueUseResults = this.results.filter(r => r.version === 'useVModel_VueUse') + + const v3Avg = this.calculateAverage(v3Results) + const v4Avg = this.calculateAverage(v4Results) + const vueUseAvg = this.calculateAverage(vueUseResults) + + const comparison = { + v3Average: v3Avg, + v4Average: v4Avg, + vueUseAverage: vueUseAvg, + performance: [] as string[], + } + + if (v3Avg && v4Avg && vueUseAvg) { + const implementations = [ + { name: 'V3', time: v3Avg.executionTimeMs }, + { name: 'V4', time: v4Avg.executionTimeMs }, + { name: 'VueUse', time: vueUseAvg.executionTimeMs }, + ].sort((a, b) => a.time - b.time) + + const fastest = implementations[0] + + implementations.slice(1).forEach((impl) => { + const diff = impl.time - fastest.time + const percentage = (diff / fastest.time) * 100 + comparison.performance.push( + `${fastest.name} is ${percentage.toFixed(2)}% faster than ${impl.name}`, + ) + }) + } + + return comparison + } + + static clearWarmupResults() { + this.results = this.results.slice(-5) // Keep only last 5 results + } + + private static calculateAverage(results: PerformanceResult[]) { + if (results.length === 0) + return null + + // Remove outliers + const sorted = [...results].sort((a, b) => a.executionTimeMs - b.executionTimeMs) + const q1Index = Math.floor(sorted.length * 0.25) + const q3Index = Math.floor(sorted.length * 0.75) + const validResults = sorted.slice(q1Index, q3Index + 1) + + return { + executionTimeMs: validResults.reduce((sum, r) => sum + r.executionTimeMs, 0) / validResults.length, + onChangeCallCount: validResults.reduce((sum, r) => sum + r.onChangeCallCount, 0) / validResults.length, + } + } + + private static writeToFile() { + const output = { + timestamp: new Date().toISOString(), + results: this.results, + comparison: this.compareVersions(), + summary: this.compareVersions().performance, + } + + // .data directory must exist before writing to it + if (!existsSync(resolve(__dirname, './.data'))) { + mkdirSync(resolve(__dirname, './.data')) + } + + try { + writeFileSync( + this.outputPath, + JSON.stringify(output, null, 2), + 'utf-8', + ) + } + catch (error) { + console.error('Failed to write performance results:', error) + } + } +} diff --git a/packages/core/src/hooks/tests/useControllableState.test.ts b/packages/core/src/hooks/tests/useControllableState.test.ts new file mode 100644 index 000000000..b9daee1f4 --- /dev/null +++ b/packages/core/src/hooks/tests/useControllableState.test.ts @@ -0,0 +1,239 @@ +import { useVModel } from '@vueuse/core' +import { afterAll, afterEach, describe, expect, it, vi } from 'vitest' +import { type ComponentInternalInstance, computed, nextTick, ref, type Ref, watch } from 'vue' +import { useControllableStateV3, useControllableStateV4 } from '../useControllableState' +import { PerformanceLogger } from './performance-results' + +// Mock Vue's lifecycle hooks +// eslint-disable-next-line unused-imports/no-unused-vars +let unmountHandler: (() => void) | undefined +vi.mock('vue', async () => { + const actual = await vi.importActual('vue') + return { + ...actual as any, + onBeforeUnmount: vi.fn((fn) => { + unmountHandler = fn + }), + } +}) + +// Update type definitions for controller tests +type Function = (...args: any[]) => any +type PropGetter = (() => T | undefined) & Function +type PropValue = T | undefined +type DefaultValueGetter = (() => T) & Function +type PropType = PropGetter | Exclude, Function> +type DefaultType = DefaultValueGetter | Exclude + +// Add type guard +function isPropGetter(prop: PropType): prop is PropGetter { + return typeof prop === 'function' +} + +// Update the controller function type +type ControllerFn = { + ( + prop: PropType, + onChange: ((value: T) => void) | undefined, + defaultValue: DefaultType, + options?: any + ): Ref +} + +// Fix the createVModelWrapper type signature +function createVModelWrapper( + prop: PropType, + onChange: ((value: T) => void) | undefined, + defaultValue: DefaultType, +): Ref { + const initialValue = typeof defaultValue === 'function' + ? (defaultValue as DefaultValueGetter)() + : defaultValue + const currentValue = ref(initialValue) + const emit = vi.fn((event: string, ...args: any[]) => { + if (event === 'update:modelValue' && onChange) + onChange(args[0]) + }) + + // Create mock instance with proper type handling + const instance = { + emit, + proxy: { + $props: computed(() => ({ + modelValue: isPropGetter(prop) ? prop() : prop, + })), + $emit: emit, + }, + refs: {}, + attrs: {}, + slots: {}, + setupState: {}, + ctx: { + emit, + }, + } as unknown as ComponentInternalInstance + + // Create model with sync between prop and internal value + const model = useVModel( + instance, + 'modelValue' as keyof ComponentInternalInstance, + emit, + { + defaultValue: initialValue as any, + passive: true, // Change to true for better prop sync + }, + ) + + // Sync with prop changes + if (isPropGetter(prop)) { + watch( + () => prop(), + (newVal) => { + if (newVal !== undefined) { + currentValue.value = newVal + model.value = newVal + } + }, + { immediate: true }, // Important for initial sync + ) + } + else if (prop !== undefined) { + currentValue.value = prop + model.value = prop + } + + // eslint-disable-next-line ts/ban-ts-comment + // @ts-ignore + return computed({ + get: () => model.value, + set: (value) => { + currentValue.value = value + model.value = value + }, + }) +} + +// Use explicit type for controllers +const controllers: Record = { + // eslint-disable-next-line ts/ban-ts-comment + // @ts-ignore + useControllableStateV3, + useControllableStateV4, + createVModelWrapper, +} + +// Update VueUse specific tests +describe('vueUse useVModel specific tests', () => { + it('should handle v-model correctly', async () => { + const onChange = vi.fn() + const value = createVModelWrapper(undefined, onChange, 0) + + value.value = 5 + await nextTick() + + expect(onChange).toHaveBeenCalledWith(5) + }) + + it('should handle external prop changes with nextTick', async () => { + const externalValue = ref(0) + const onChange = vi.fn() + const value = createVModelWrapper(() => externalValue.value, onChange, 0) + + await nextTick() + expect(value.value).toBe(0) // Initial value + + externalValue.value = 10 + await nextTick() + await nextTick() // Need two ticks for VueUse update to propagate + + expect(value.value).toBe(10) + }) + + // Fix the syntax error in the test + it('should respect controlled vs uncontrolled behavior with async updates', async () => { + const controlled = createVModelWrapper(() => 5, vi.fn(), 0) + const uncontrolled = createVModelWrapper(undefined, vi.fn(), 0) + + await nextTick() + await nextTick() + + expect(controlled.value).toBe(5) + expect(uncontrolled.value).toBe(0) // Fixed missing parenthesis + }) +}) + +// Add before performance tests +describe('performance Comparison', () => { + const WARMUP_ROUNDS = 1 // Reduced to 1 + const TEST_ROUNDS = 10 // Reduced to 2 + const UPDATES_PER_TEST = 25 // Reduced to 25 + const IMPLEMENTATIONS = [ + { name: 'useControllableStateV3', fn: controllers.useControllableStateV3 }, + { name: 'useControllableStateV4', fn: controllers.useControllableStateV4 }, + { name: 'useVModel_VueUse', fn: controllers.createVModelWrapper }, + ] + + // Optimized warmup function + const warmup = async () => { + const promises = [] + for (let i = 0; i < WARMUP_ROUNDS; i++) { + for (const impl of IMPLEMENTATIONS) { + // eslint-disable-next-line ts/no-use-before-define + promises.push(runPerformanceTest(impl.name, impl.fn)) + } + } + await Promise.all(promises) + PerformanceLogger.clearWarmupResults() + } + + // Optimized test function + const runPerformanceTest = async (name: string, controller: ControllerFn) => { + const onChange = vi.fn() + const value = controller(undefined, onChange, () => 0) + + for (let i = 0; i < UPDATES_PER_TEST; i++) { + value.value = i + } + await nextTick() + + return { name, calls: onChange.mock.calls.length } + } + + // Single performance test + it('should compare implementations performance', async () => { + await warmup() + + // Run tests in parallel + const testRuns = [] + for (let round = 0; round < TEST_ROUNDS; round++) { + const implementations = [...IMPLEMENTATIONS] + for (const impl of implementations) { + const start = performance.now() + await runPerformanceTest(impl.name, impl.fn) + const end = performance.now() + + PerformanceLogger.logResult( + impl.name, + end - start, + UPDATES_PER_TEST, + ) + } + testRuns.push(nextTick()) + } + + await Promise.all(testRuns) + + const comparison = PerformanceLogger.compareVersions() + expect(comparison).toBeDefined() + expect(comparison.v3Average?.executionTimeMs).toBeGreaterThan(0) + expect(comparison.v4Average?.executionTimeMs).toBeGreaterThan(0) + }, 15000) // Increased timeout + + afterEach(() => { + vi.clearAllTimers() + }) + + afterAll(() => { + PerformanceLogger.getResults() + }) +}) diff --git a/packages/core/src/hooks/useControllableState.ts b/packages/core/src/hooks/useControllableState.ts index 0f80e1bfc..4faf26c1b 100644 --- a/packages/core/src/hooks/useControllableState.ts +++ b/packages/core/src/hooks/useControllableState.ts @@ -142,3 +142,29 @@ export function useControllableStateV3( return proxy } + +export function useControllableStateV4( + prop: (() => T | undefined) | T | undefined, + onChange: ((value: T) => void) | undefined, + defaultValue: (() => T) | T, +): Ref { + const isFunc = typeof prop === 'function' + const defValue = typeof defaultValue === 'function' ? (defaultValue as () => T)() : defaultValue + const proxy = shallowRef(isFunc ? (prop as () => T)() ?? defValue : prop ?? defValue) + + if (!isFunc) + return computed({ get: () => proxy.value, set: v => onChange?.(v) }) + + let skip = false + watch(() => (prop as () => T)(), (v) => { + if (skip || v === undefined) + return + skip = true + proxy.value = v + nextTick(() => skip = false) + }) + + watch(proxy, v => !skip && onChange?.(v)) + + return proxy +} diff --git a/packages/core/src/slider/SliderRoot.ts b/packages/core/src/slider/SliderRoot.ts index 5849b4a05..e9341b5a0 100644 --- a/packages/core/src/slider/SliderRoot.ts +++ b/packages/core/src/slider/SliderRoot.ts @@ -3,7 +3,7 @@ import type { EmitsToHookProps, LooseRequired, PrimitiveDefaultProps, RadixPrimi import { computed, type HTMLAttributes, type MaybeRefOrGetter, type Ref, type UnwrapRef } from 'vue' import { createCollection } from '../collection/index.ts' import { type Direction, useDirection } from '../direction/index.ts' -import { createContext, type MutableRefObject, useControllableStateV3, useRef, type useSize } from '../hooks/index.ts' +import { createContext, type MutableRefObject, useControllableStateV4, useRef, type useSize } from '../hooks/index.ts' import { clamp, getDecimalCount, isNumber, mergePrimitiveAttrs, roundValue } from '../shared/index.ts' import { getClosestValueIndex, getNextSortedValues, hasMinStepsBetweenValues, linearScale } from './utils.ts' @@ -77,7 +77,7 @@ export const [provideSliderOrientationContext, useSliderOrientationContext] = cr export interface UseSliderRootProps extends EmitsToHookProps { el?: MutableRefObject - value?: () => number[] | undefined + value?: (() => number[] | undefined) | number[] defaultValue?: () => number[] name?: () => string | undefined disabled?: () => boolean | undefined @@ -113,13 +113,13 @@ export function useSliderRoot(props: UseSliderRootProps): RadixPrimitiveReturns const thumbs: SliderContext['thumbs'] = new Set() const valueIndexToChangeRef = useRef(0) - // TODO: is not reactive - const values = useControllableStateV3( + const values = useControllableStateV4( props.value, (v) => { const _thumbs = Array.from(thumbs) _thumbs[valueIndexToChangeRef.value]?.focus() props.onUpdateValue?.(v as number[]) + props.onUpdateValue?.(v) }, defaultValue, ) diff --git a/vitest.config.ts b/vitest.config.ts index 49a7b0a55..8c6449147 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -15,10 +15,6 @@ export default defineConfig({ exclude: [ '**/node_modules/**', '**/dist/**', - 'packages/nuxt-module/**', - 'packages/example-package/**', - 'packages/primitives/**', - 'packages/tsconfig/**', ], include: ['./**/*.test.ts'], setupFiles: ['./vitest-setup.ts'], @@ -29,7 +25,7 @@ export default defineConfig({ }, resolve: { alias: [ - ...primitivesPackagesAlias('./packages/vue/src', resolve), + ...primitivesPackagesAlias('./packages/core/src', resolve), ], }, })