Skip to content

Commit

Permalink
Fix: [SlotComponent] Lifecycle hook won't be removed in Vue 3.5 #274 (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@kikuomax
kikuomax authored Feb 3, 2025
1 parent 368a8a1 commit 41dea10
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 337 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
233 changes: 1 addition & 232 deletions packages/buefy-next/src/utils/SlotComponent.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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<typeof BSlotComponent>

describe('BSlotComponent', () => {
const MockComponent = {
render: () => h('div', {}, 'Hello!')
Expand Down Expand Up @@ -54,233 +52,4 @@ describe('BSlotComponent', () => {
})
expect(wrapper.html()).toBe(`<${tag}>${slot}</${tag}>`)
})

it('render after emit event', async () => {
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
})

it('refresh after default event (hook)', async () => {
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
spyOnRefresh.mockRestore()
})

it('refresh on default event with existing handler', async () => {
const MockComponent = {
render: () => h('div', {}, 'Hello!'),
updated: vi.fn()
}
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
spyOnRefresh.mockRestore()
})

it('refresh on custom event', () => {
const event = 'component-event'
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
spyOnRefresh.mockRestore()
})

it('refresh on custom event with existing handler (default case)', () => {
const event = 'component-event'
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
spyOnRefresh.mockRestore()
})

it('refresh on custom event with existing handler (camelized case)', () => {
const event = 'component-event'
const slot = '<span>Content</span>'
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(`<div>${slot}</div>`)
spyOnRefresh.mockRestore()
})

it('destroy', async () => {
const slot = '<span>Content</span>'
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 = '<span>Content</span>'
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 = '<span>Content</span>'
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 = '<span>Content</span>'
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()
})
})
106 changes: 1 addition & 105 deletions packages/buefy-next/src/utils/SlotComponent.ts
View file
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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
Expand Down

0 comments on commit 41dea10

Please sign in to comment.