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.

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

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