From 8f8b3551ff28587f17e94e9c58922c3fbe262370 Mon Sep 17 00:00:00 2001 From: Evobaso-J <57828270+Evobaso-J@users.noreply.github.com> Date: Tue, 19 Mar 2024 06:47:19 +0100 Subject: [PATCH 1/4] test: add regression test --- tests/mount.spec.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/mount.spec.ts b/tests/mount.spec.ts index ad0b03fc2..3f457ee38 100644 --- a/tests/mount.spec.ts +++ b/tests/mount.spec.ts @@ -4,6 +4,7 @@ import { mount } from '../src' import DefinePropsAndDefineEmits from './components/DefinePropsAndDefineEmits.vue' import WithDeepRef from './components/WithDeepRef.vue' import HelloFromVitestPlayground from './components/HelloFromVitestPlayground.vue' +import Hello from './components/Hello.vue' describe('mount: general tests', () => { it('correctly handles component, throwing on mount', () => { @@ -28,7 +29,7 @@ describe('mount: general tests', () => { }) it('should not warn on readonly hasOwnProperty when mounting a component', () => { - const spy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const spy = vi.spyOn(console, 'warn').mockImplementation(() => { }) mount(HelloFromVitestPlayground, { props: { count: 2 } }) @@ -70,4 +71,17 @@ describe('mount: general tests', () => { expect(wrapper.get('#oneLayerCountArrayObjectValue').text()).toBe('7') expect(wrapper.get('#oneLayerCountObjectMatrixValue').text()).toBe('8') }) + it('circular reference in prop does not cause a stack overflow', () => { + type ParentItem = { children: any[] } + type Item = { id: number; parent: ParentItem | null } + + const item: Item = { id: 1, parent: null } + const parentItem: ParentItem = { children: [item] } + item.parent = parentItem + + const wrapper = mount(Hello, { + props: { msg: 'Hello world', item: item } + }) + expect(wrapper.text()).toContain('Hello world') + }) }) From 83a47ca684c6879d07539ef1d0579199abaac4d6 Mon Sep 17 00:00:00 2001 From: Evobaso-J <57828270+Evobaso-J@users.noreply.github.com> Date: Tue, 19 Mar 2024 08:44:03 +0100 Subject: [PATCH 2/4] fix: check infinite loop in isDeepRef --- src/createInstance.ts | 2 +- src/utils.ts | 17 ++--------------- src/utils/isDeepRef.ts | 36 ++++++++++++++++++++++++++++++++++++ tests/mount.spec.ts | 21 ++++++++++++++------- 4 files changed, 53 insertions(+), 23 deletions(-) create mode 100644 src/utils/isDeepRef.ts diff --git a/src/createInstance.ts b/src/createInstance.ts index 34f307783..1afe8e95e 100644 --- a/src/createInstance.ts +++ b/src/createInstance.ts @@ -16,7 +16,6 @@ import { MountingOptions, Slot } from './types' import { getComponentsFromStubs, getDirectivesFromStubs, - isDeepRef, isFunctionalComponent, isObject, isObjectComponent, @@ -36,6 +35,7 @@ import { CreateStubComponentsTransformerConfig } from './vnodeTransformers/stubComponentsTransformer' import { createStubDirectivesTransformer } from './vnodeTransformers/stubDirectivesTransformer' +import { isDeepRef } from './utils/isDeepRef' const MOUNT_OPTIONS: ReadonlyArray> = [ 'attachTo', diff --git a/src/utils.ts b/src/utils.ts index 97b417e84..f1119b4b8 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,12 +1,11 @@ -import { DeepRef, GlobalMountOptions, RefSelector, Stub, Stubs } from './types' +import { GlobalMountOptions, RefSelector, Stub, Stubs } from './types' import { Component, ComponentOptions, ComponentPublicInstance, ConcreteComponent, Directive, - FunctionalComponent, - isRef + FunctionalComponent } from 'vue' import { config } from './config' @@ -253,15 +252,3 @@ export const getGlobalThis = (): any => { : {}) ) } - -/** - * Checks if the given value is a DeepRef. - * - * For both arrays and objects, it will recursively check - * if any of their values is a Ref. - * - * @param {DeepRef | unknown} r - The value to check. - * @returns {boolean} Returns true if the value is a DeepRef, false otherwise. - */ -export const isDeepRef = (r: DeepRef | unknown): r is DeepRef => - isRef(r) || (isObject(r) && Object.values(r).some(isDeepRef)) diff --git a/src/utils/isDeepRef.ts b/src/utils/isDeepRef.ts new file mode 100644 index 000000000..00002c2bd --- /dev/null +++ b/src/utils/isDeepRef.ts @@ -0,0 +1,36 @@ +import { isObject } from '../utils' +import type { DeepRef } from '../types' +import { isRef } from 'vue' + +/** + * Implementation details of isDeepRef to avoid circular dependencies. + * It keeps track of visited objects to avoid infinite recursion. + * + * @param r The value to check for a Ref. + * @param visitedObjects a weak map to keep track of visited objects and avoid infinite recursion + * @returns returns true if the value is a Ref, false otherwise + */ +const deeplyCheckForRef = ( + r: DeepRef | unknown, + visitedObjects: WeakMap +): r is DeepRef => { + if (isRef(r)) return true + if (!isObject(r)) return false + if (visitedObjects.has(r)) return false + visitedObjects.set(r, true) + return Object.values(r).some((val) => deeplyCheckForRef(val, visitedObjects)) +} + +/** + * Checks if the given value is a DeepRef. + * + * For both arrays and objects, it will recursively check + * if any of their values is a Ref. + * + * @param {DeepRef | unknown} r - The value to check. + * @returns {boolean} Returns true if the value is a DeepRef, false otherwise. + */ +export const isDeepRef = (r: DeepRef | unknown): r is DeepRef => { + const visitedObjects = new WeakMap() + return deeplyCheckForRef(r, visitedObjects) +} diff --git a/tests/mount.spec.ts b/tests/mount.spec.ts index 3f457ee38..dc6d71536 100644 --- a/tests/mount.spec.ts +++ b/tests/mount.spec.ts @@ -29,7 +29,7 @@ describe('mount: general tests', () => { }) it('should not warn on readonly hasOwnProperty when mounting a component', () => { - const spy = vi.spyOn(console, 'warn').mockImplementation(() => { }) + const spy = vi.spyOn(console, 'warn').mockImplementation(() => {}) mount(HelloFromVitestPlayground, { props: { count: 2 } }) @@ -71,14 +71,21 @@ describe('mount: general tests', () => { expect(wrapper.get('#oneLayerCountArrayObjectValue').text()).toBe('7') expect(wrapper.get('#oneLayerCountObjectMatrixValue').text()).toBe('8') }) - it('circular reference in prop does not cause a stack overflow', () => { - type ParentItem = { children: any[] } - type Item = { id: number; parent: ParentItem | null } - - const item: Item = { id: 1, parent: null } - const parentItem: ParentItem = { children: [item] } + it('circular references in non-ref props do not cause a stack overflow', () => { + const item = { id: 1, parent: null as any } + const parentItem = { children: [item] } item.parent = parentItem + const wrapper = mount(Hello, { + props: { msg: 'Hello world', item: item } + }) + expect(wrapper.text()).toContain('Hello world') + }) + it('circular references in ref props do not cause a stack overflow', () => { + const item = { id: 1, parent: ref(null) } + const parentItem = { children: [ref(item)] } + item.parent.value = parentItem + const wrapper = mount(Hello, { props: { msg: 'Hello world', item: item } }) From a2948f7648937a80a711e3e020bb09efaa88e6a4 Mon Sep 17 00:00:00 2001 From: Evobaso-J <57828270+Evobaso-J@users.noreply.github.com> Date: Tue, 19 Mar 2024 23:24:03 +0100 Subject: [PATCH 3/4] test: add test cases for isDeepRef --- tests/isDeepRef.spec.ts | 70 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tests/isDeepRef.spec.ts diff --git a/tests/isDeepRef.spec.ts b/tests/isDeepRef.spec.ts new file mode 100644 index 000000000..7fc9a5b16 --- /dev/null +++ b/tests/isDeepRef.spec.ts @@ -0,0 +1,70 @@ +import { isDeepRef } from '../src/utils/isDeepRef' +import { describe, expect, it } from 'vitest' +import { ref } from 'vue' + +describe('isDeepRef', () => { + it('should return true for a Ref value', () => { + const testRef = ref(1) + + expect(isDeepRef(testRef)).toBe(true) + }) + it('should return false for a non-object, non-Ref value', () => { + const nonObject = 1 + + expect(isDeepRef(nonObject)).toBe(false) + }) + it('should return true for an object with a Ref value', () => { + const testObject = { ref: ref(1) } + + expect(isDeepRef(testObject)).toBe(true) + }) + it('should return false for an object without a Ref value', () => { + const testObject = { nonRef: 1 } + + expect(isDeepRef(testObject)).toBe(false) + }) + it('should return true for an array with a Ref value', () => { + const arrayWithRef = [ref(1)] + + expect(isDeepRef(arrayWithRef)).toBe(true) + }) + it('should return false for an array without a Ref value', () => { + const arrayWithoutRef = [1] + + expect(isDeepRef(arrayWithoutRef)).toBe(false) + }) + it('should return true for a nested object with a Ref value', () => { + const nestedObject = { nested: { ref: ref(1) } } + + expect(isDeepRef(nestedObject)).toBe(true) + }) + it('should return false for a nested object without a Ref value', () => { + const nestedObject = { nested: { nonRef: 1 } } + + expect(isDeepRef(nestedObject)).toBe(false) + }) + it('should return true for a nested array with a Ref value', () => { + const nestedArray = [[ref(1)]] + + expect(isDeepRef(nestedArray)).toBe(true) + }) + it('should return false for a nested array without a Ref value', () => { + const nestedArray = [[1]] + + expect(isDeepRef(nestedArray)).toBe(false) + }) + it('should return false for an object that has already been visited and does not contain a Ref', () => { + const item = { parent: null as any } + const parentItem = { children: [item] } + item.parent = parentItem + + expect(isDeepRef(item)).toBe(false) + }) + it('should return true for an object that has already been visited and contains a Ref', () => { + const item = { parent: ref(null) } + const parentItem = { children: [ref(item)] } + item.parent.value = parentItem + + expect(isDeepRef(item)).toBe(true) + }) +}) From d7ec8716202e49d1b833cc403796da3525a86002 Mon Sep 17 00:00:00 2001 From: Evobaso-J <57828270+Evobaso-J@users.noreply.github.com> Date: Wed, 20 Mar 2024 00:02:56 +0100 Subject: [PATCH 4/4] test: add test case for isDeepRef --- tests/isDeepRef.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/isDeepRef.spec.ts b/tests/isDeepRef.spec.ts index 7fc9a5b16..934c3c97d 100644 --- a/tests/isDeepRef.spec.ts +++ b/tests/isDeepRef.spec.ts @@ -67,4 +67,16 @@ describe('isDeepRef', () => { expect(isDeepRef(item)).toBe(true) }) + it('should return false for an object with a dynamic circular reference', () => { + const testObject = {} + Object.defineProperty(testObject, 'circularReference', { + get: function () { + delete this.circularReference + this.circularReference = testObject + return this.circularReference + } + }) + expect(() => isDeepRef(testObject)).not.toThrow() + expect(isDeepRef(testObject)).toBe(false) + }) })