Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: [SlotComponent] Lifecycle hook won't be removed in Vue 3.5 #274 #432

Merged
merged 3 commits into from
Feb 3, 2025
Merged

Fix: [SlotComponent] Lifecycle hook won't be removed in Vue 3.5 #274 #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97c7b54
feat(lib): remove event from SlotComponent
kikuomax Jan 27, 2025
091d815
test(lib): remove meaningless tests for SlotComponent
kikuomax Jan 27, 2025
b8dc7df
docs: add a record regarding SlotComponent to CHANGELOG
kikuomax Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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