From 41dea100224572a0a9711cbdec60e4658c4b6505 Mon Sep 17 00:00:00 2001 From: Kikuo Emoto Date: Mon, 3 Feb 2025 09:28:11 +0900 Subject: [PATCH] Fix: [SlotComponent] Lifecycle hook won't be removed in Vue 3.5 #274 (#432) * feat(lib): remove event from SlotComponent Removes the `event` prop, and all the tricks to programmatically add / remove an event listener to / from the underlying Vue component. As far as I tested, there is no need to explicitly call `$forceUpdate` when the underlying Vue component is updated. * test(lib): remove meaningless tests for SlotComponent Removes meaningless tests from `src/utils/SlotComponent.spec.ts`. They became obsolete because the `event` prop, and the related tricks were removed from `SlotComponent`. * docs: add a record regarding SlotComponent to CHANGELOG --- CHANGELOG.md | 5 + .../src/utils/SlotComponent.spec.ts | 233 +----------------- .../buefy-next/src/utils/SlotComponent.ts | 106 +------- 3 files changed, 7 insertions(+), 337 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5118d467c..95de3baaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Buefy-next 0.2.1 unreleased +### Fixes + +* [#274](https://github.com/ntohq/buefy-next/issues/274) `Table`, and `Tabs` caused memory leaks on Vue 3.5. + `SlotComponent` which is internally used by `Table`, and `Tabs` ceased adding the update hook to the component specified to the `component` prop, and dropped the `event` prop. + ## Buefy-next 0.2.0 ### New features diff --git a/packages/buefy-next/src/utils/SlotComponent.spec.ts b/packages/buefy-next/src/utils/SlotComponent.spec.ts index a544f4593..cc158b0ac 100644 --- a/packages/buefy-next/src/utils/SlotComponent.spec.ts +++ b/packages/buefy-next/src/utils/SlotComponent.spec.ts @@ -1,10 +1,8 @@ import { h } from 'vue' import { shallowMount } from '@vue/test-utils' -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it } from 'vitest' import BSlotComponent from '@utils/SlotComponent' -type BSlotComponentInstance = InstanceType - describe('BSlotComponent', () => { const MockComponent = { render: () => h('div', {}, 'Hello!') @@ -54,233 +52,4 @@ describe('BSlotComponent', () => { }) expect(wrapper.html()).toBe(`<${tag}>${slot}`) }) - - it('render after emit event', async () => { - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm - } - }) - Component.vm.$forceUpdate() - await wrapper.vm.$nextTick() - expect(wrapper.html()).toBe(`
${slot}
`) - }) - - it('refresh after default event (hook)', async () => { - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm - } - }) - Component.vm.$forceUpdate() - await Component.vm.$nextTick() - expect(spyOnRefresh).toHaveBeenCalledTimes(1) - expect(wrapper.html()).toBe(`
${slot}
`) - spyOnRefresh.mockRestore() - }) - - it('refresh on default event with existing handler', async () => { - const MockComponent = { - render: () => h('div', {}, 'Hello!'), - updated: vi.fn() - } - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm - } - }) - Component.vm.$forceUpdate() - await Component.vm.$nextTick() - expect(MockComponent.updated).toHaveBeenCalledTimes(1) - expect(spyOnRefresh).toHaveBeenCalledTimes(1) - expect(wrapper.html()).toBe(`
${slot}
`) - spyOnRefresh.mockRestore() - }) - - it('refresh on custom event', () => { - const event = 'component-event' - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm, - event - } - }) - Component.vm.$emit(event, {}) - expect(spyOnRefresh).toHaveBeenCalledTimes(1) - expect(wrapper.html()).toBe(`
${slot}
`) - spyOnRefresh.mockRestore() - }) - - it('refresh on custom event with existing handler (default case)', () => { - const event = 'component-event' - const slot = 'Content' - const existingHandler = vi.fn() - const Component = shallowMount(MockComponent, { - props: { - 'onComponent-event': existingHandler - }, - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm, - event - } - }) - Component.vm.$emit(event, {}) - expect(existingHandler).toHaveBeenCalledTimes(1) - expect(spyOnRefresh).toHaveBeenCalledTimes(1) - expect(wrapper.html()).toBe(`
${slot}
`) - spyOnRefresh.mockRestore() - }) - - it('refresh on custom event with existing handler (camelized case)', () => { - const event = 'component-event' - const slot = 'Content' - const existingHandler = vi.fn() - const Component = shallowMount(MockComponent, { - props: { - onComponentEvent: existingHandler - }, - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm, - event - } - }) - Component.vm.$emit(event, {}) - expect(existingHandler).toHaveBeenCalledTimes(1) - expect(spyOnRefresh).toHaveBeenCalledTimes(1) - expect(wrapper.html()).toBe(`
${slot}
`) - spyOnRefresh.mockRestore() - }) - - it('destroy', async () => { - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm - } - }) - wrapper.unmount() - Component.vm.$forceUpdate() - await Component.vm.$nextTick() - expect(spyOnRefresh).toHaveBeenCalledTimes(0) - spyOnRefresh.mockRestore() - }) - - it('destroy with existing handler', async () => { - const MockComponent = { - render: () => h('div', {}, 'Hello!'), - updated: vi.fn() - } - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm - } - }) - wrapper.unmount() - Component.vm.$forceUpdate() - await Component.vm.$nextTick() - expect(MockComponent.updated).toHaveBeenCalledTimes(1) - expect(spyOnRefresh).toHaveBeenCalledTimes(0) - spyOnRefresh.mockRestore() - }) - - it('destroy with custom event', async () => { - const event = 'component-event' - const slot = 'Content' - const Component = shallowMount(MockComponent, { - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm, - event - } - }) - wrapper.unmount() - Component.vm.$emit(event, {}) - await Component.vm.$nextTick() - expect(spyOnRefresh).toHaveBeenCalledTimes(0) - spyOnRefresh.mockRestore() - }) - - it('destroy with custom event and existing handler', async () => { - const event = 'component-event' - const slot = 'Content' - const existingHandler = vi.fn() - const Component = shallowMount(MockComponent, { - props: { - 'onComponent-event': existingHandler - }, - slots: { - default: slot - } - }) - const spyOnRefresh = vi.spyOn(BSlotComponent.methods as BSlotComponentInstance, 'refresh').mockImplementation(() => {}) - const wrapper = shallowMount(BSlotComponent, { - props: { - component: Component.vm, - event - } - }) - wrapper.unmount() - Component.vm.$emit(event, {}) - await Component.vm.$nextTick() - expect(existingHandler).toHaveBeenCalledTimes(1) - expect(spyOnRefresh).toHaveBeenCalledTimes(0) - spyOnRefresh.mockRestore() - }) }) diff --git a/packages/buefy-next/src/utils/SlotComponent.ts b/packages/buefy-next/src/utils/SlotComponent.ts index 3bd72ebb4..dc3658d3f 100644 --- a/packages/buefy-next/src/utils/SlotComponent.ts +++ b/packages/buefy-next/src/utils/SlotComponent.ts @@ -1,25 +1,4 @@ -import { defineComponent, h as createElement, onUpdated } from 'vue' -import { - camelize, - hyphenate, - toHandlerKey -} from '@vue/shared' // eslint-disable-line vue/prefer-import-from-vue -import { isVueComponent } from './helpers' - -// augments ComponentInternalInstance to directly manipulate `onUpdated` hooks -// -// the type signature of `u` different from the one defined in Vue 3 but aligns -// with what `onUpdated` in Vue 3.4 or earlier actually returns. however, it -// should not harm since the definition is only used here. -// -// FIXME: on Vue 3.5 or later, the following trick would no longer work. -// see: https://github.com/ntohq/buefy-next/issues/274 -declare module '@vue/runtime-core' { - interface ComponentInternalInstance { - // eslint-disable-next-line @typescript-eslint/ban-types - u: (boolean | Function | undefined)[] - } -} +import { defineComponent, h as createElement } from 'vue' export default defineComponent({ name: 'BSlotComponent', @@ -41,96 +20,13 @@ export default defineComponent({ tag: { type: String, default: 'div' - }, - event: { - type: String, - default: 'vue:updated' } }, - data: () => ({ - // see: https://github.com/vuejs/core/blob/7976f7044e66b3b7adac4c72a392935704658b10/packages/runtime-core/src/apiLifecycle.ts#L69-L74 - // eslint-disable-next-line @typescript-eslint/ban-types - updatedHook: undefined as boolean | Function | undefined, - handlerKey: undefined as string | undefined - }), methods: { refresh() { this.$forceUpdate() } }, - created() { - if (isVueComponent(this.component)) { - if (this.event === 'vue:updated') { - // lifecycle event cannot be captured as an ordinary event - // FIXME: on Vue 3.5 or later, the following trick would not - // work because `onUpdated` would no longer return a wrapper - // function but nothing (`void`) - // see: https://github.com/ntohq/buefy-next/issues/274 - this.updatedHook = onUpdated(this.refresh, this.component.$) - } else { - // directly manipuates the VNode - // because Vue 3 no longer provides $on - const { vnode } = this.component.$ - let handlerKey = toHandlerKey(this.event) - if (vnode.props == null) { - vnode.props = { [handlerKey]: this.refresh } - } else { - const { props } = vnode - if (props[this.handlerKey!] == null) { - // tries camelCase - handlerKey = toHandlerKey(camelize(this.event)) - if (props[handlerKey] == null) { - // tries kebab-case - handlerKey = toHandlerKey(hyphenate(this.event)) - } - } - if (props[handlerKey] == null) { - handlerKey = toHandlerKey(this.event) - props[handlerKey] = this.refresh - } else { - // multiple handlers may be specified in an array - if (Array.isArray(props[handlerKey])) { - props[handlerKey].push(this.refresh) - } else { - props[handlerKey] = [props[handlerKey], this.refresh] - } - } - } - this.handlerKey = handlerKey - } - } - }, - beforeUnmount() { - if (isVueComponent(this.component)) { - if (this.updatedHook != null) { - // unfortunately, there is no counterpart of `onUpdated`. - // so directly manipulates the internal instance. - // see https://github.com/vuejs/core/blob/2ffe3d5b3e953b63d4743b1e2bc242d50916b545/packages/runtime-core/src/apiLifecycle.ts#L17-L64 - const index = this.component.$.u.indexOf(this.updatedHook) - if (index !== -1) { - // eslint-disable-next-line vue/no-mutating-props - this.component.$.u.splice(index, 1) - } - } else if (this.handlerKey != null) { - // directly maniputates VNode - // because Vue 3 no longer provides $off - const { props } = this.component.$.vnode - if (props != null) { - if (Array.isArray(props[this.handlerKey])) { - const index = props[this.handlerKey].indexOf(this.refresh) - if (index > -1) { - props[this.handlerKey].splice(index, 1) - if (props[this.handlerKey].length === 1) { - props[this.handlerKey] = props[this.handlerKey][0] - } - } - } else { - delete props[this.handlerKey] - } - } - } - } - }, render() { return createElement(this.tag, {}, this.component.$slots