Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(app): support app level effect scope #8801

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
21 changes: 21 additions & 0 deletions packages/runtime-core/__tests__/apiCreateApp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
nextTick,
nodeOps,
onMounted,
onScopeDispose,
provide,
ref,
resolveComponent,
Expand Down Expand Up @@ -556,6 +557,26 @@ describe('api: createApp', () => {
).not.toHaveBeenWarned()
})

test('should invoke onScopeDispose when the app unmount', () => {
const spy = vi.fn(() => {})
const root = nodeOps.createElement('div')

const app = createApp({
setup() {
return () => h('div')
},
})

app.runWithContext(() => {
onScopeDispose(spy)
})

app.mount(root)
app.unmount()

expect(spy).toHaveBeenCalledTimes(1)
})

// #10005
test('flush order edge case on nested createApp', async () => {
const order: string[] = []
Expand Down
8 changes: 7 additions & 1 deletion packages/runtime-core/src/apiCreateApp.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { effectScope } from '@vue/reactivity'
import {
type Component,
type ComponentInternalInstance,
Expand Down Expand Up @@ -28,6 +29,7 @@ import { installAppCompatProperties } from './compat/global'
import type { NormalizedPropsOptions } from './componentProps'
import type { ObjectEmitsOptions } from './componentEmits'
import type { DefineComponent } from './apiDefineComponent'
import type { EffectScope } from '@vue/reactivity'

export interface App<HostElement = any> {
version: string
Expand Down Expand Up @@ -70,6 +72,7 @@ export interface App<HostElement = any> {
_container: HostElement | null
_context: AppContext
_instance: ComponentInternalInstance | null
_scope: EffectScope

/**
* v2 compat only
Expand Down Expand Up @@ -215,6 +218,7 @@ export function createAppAPI<HostElement>(
rootProps = null
}

const scope = effectScope(true)
const context = createAppContext()
Comment on lines +221 to 222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me wonder if the effectScope should be in app context 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@posva In the latest commit (a7141d4), I added scope to the app. Currently, I have handled it as a private property. Should it be made public for user access?

const installedPlugins = new WeakSet()

Expand All @@ -227,6 +231,7 @@ export function createAppAPI<HostElement>(
_container: null,
_context: context,
_instance: null,
_scope: scope,

version,

Expand Down Expand Up @@ -371,6 +376,7 @@ export function createAppAPI<HostElement>(

unmount() {
if (isMounted) {
scope.stop()
render(null, app._container)
if (__DEV__ || __FEATURE_PROD_DEVTOOLS__) {
app._instance = null
Expand Down Expand Up @@ -399,7 +405,7 @@ export function createAppAPI<HostElement>(
const lastApp = currentApp
currentApp = app
try {
return fn()
return app._scope.run(fn)!
} finally {
currentApp = lastApp
}
Expand Down