Skip to content

fix(runtime-vapor): detach effect scope & component instance #174

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.

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

Merged
merged 2 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions packages/runtime-vapor/__tests__/dom/prop.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ import {
setCurrentInstance,
} from '../../src/component'
import { getMetadata, recordPropMetadata } from '../../src/componentMetadata'
import { getCurrentScope } from '@vue/reactivity'

let removeComponentInstance = NOOP
beforeEach(() => {
const reset = setCurrentInstance(
createComponentInstance((() => {}) as any, {}),
)
const instance = createComponentInstance((() => {}) as any, {})
const reset = setCurrentInstance(instance)
const prev = getCurrentScope()
instance.scope.on()
removeComponentInstance = () => {
instance.scope.prevScope = prev
instance.scope.off()
reset()
removeComponentInstance = NOOP
}
Expand Down
29 changes: 29 additions & 0 deletions packages/runtime-vapor/__tests__/renderEffect.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {
EffectScope,
getCurrentScope,
nextTick,
onBeforeUpdate,
onEffectCleanup,
Expand All @@ -10,6 +12,10 @@ import {
watchPostEffect,
watchSyncEffect,
} from '../src'
import {
type ComponentInternalInstance,
currentInstance,
} from '../src/component'
import { makeRender } from './_utils'

const define = makeRender<any>()
Expand Down Expand Up @@ -207,4 +213,27 @@ describe('renderEffect', () => {
'[Vue warn] Unhandled error during execution of updated',
).toHaveBeenWarned()
})

test('should be called with the current instance and current scope', async () => {
const source = ref(0)
const scope = new EffectScope()
let instanceSnap: ComponentInternalInstance | null = null
let scopeSnap: EffectScope | undefined = undefined
const { instance } = define(() => {
scope.run(() => {
renderEffect(() => {
instanceSnap = currentInstance
scopeSnap = getCurrentScope()
})
})
}).render()

expect(instanceSnap).toBe(instance)
expect(scopeSnap).toBe(scope)

source.value++
await nextTick()
expect(instanceSnap).toBe(instance)
expect(scopeSnap).toBe(scope)
})
})
4 changes: 3 additions & 1 deletion packages/runtime-vapor/src/apiLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ const injectHook = (
}
pauseTracking()
const reset = setCurrentInstance(target)
const res = callWithAsyncErrorHandling(hook, target, type, args)
const res = target.scope.run(() =>
callWithAsyncErrorHandling(hook, target, type, args),
)
reset()
resetTracking()
return res
Expand Down
2 changes: 2 additions & 0 deletions packages/runtime-vapor/src/apiRender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ export function setupComponent(
instance.setupState = proxyRefs(stateOrNode)
}
if (!block && component.render) {
pauseTracking()
block = component.render(instance.setupState)
resetTracking()
}

if (block instanceof DocumentFragment) {
Expand Down
2 changes: 0 additions & 2 deletions packages/runtime-vapor/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ export const getCurrentInstance: () => ComponentInternalInstance | null = () =>
export const setCurrentInstance = (instance: ComponentInternalInstance) => {
const prev = currentInstance
currentInstance = instance
instance.scope.on()
return () => {
instance.scope.off()
currentInstance = prev
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime-vapor/src/componentLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function invokeLifecycle(
if (hooks) {
const fn = () => {
const reset = setCurrentInstance(instance)
invokeArrayFns(hooks)
instance.scope.run(() => invokeArrayFns(hooks))
reset()
}
post ? queuePostRenderEffect(fn) : fn()
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime-vapor/src/componentProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ function resolvePropValue(
// value = propsDefaults[key]
// } else {
const reset = setCurrentInstance(instance)
value = defaultValue.call(null, props)
instance.scope.run(() => (value = defaultValue.call(null, props)))
reset()
// }
} else {
Expand Down
24 changes: 19 additions & 5 deletions packages/runtime-vapor/src/renderEffect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ReactiveEffect,
type SchedulerJob,
SchedulerJobFlags,
getCurrentScope,
} from '@vue/reactivity'
import { invokeArrayFns } from '@vue/shared'
import {
Expand All @@ -16,6 +17,7 @@ import { invokeDirectiveHook } from './directives'

export function renderEffect(cb: () => void) {
const instance = getCurrentInstance()
const scope = getCurrentScope()

let effect: ReactiveEffect

Expand Down Expand Up @@ -53,11 +55,23 @@ export function renderEffect(cb: () => void) {
}
}

effect = new ReactiveEffect(() => {
const reset = instance && setCurrentInstance(instance)
callWithAsyncErrorHandling(cb, instance, VaporErrorCodes.RENDER_FUNCTION)
reset?.()
})
if (scope) {
const baseCb = cb
cb = () => scope.run(baseCb)
}

if (instance) {
const baseCb = cb
cb = () => {
const reset = setCurrentInstance(instance)
baseCb()
reset()
}
}

effect = new ReactiveEffect(() =>
callWithAsyncErrorHandling(cb, instance, VaporErrorCodes.RENDER_FUNCTION),
)

effect.scheduler = () => {
if (instance) job.id = instance.uid
Expand Down