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
23 changes: 22 additions & 1 deletion packages/runtime-core/__tests__/apiCreateApp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
Plugin,
ref,
getCurrentInstance,
defineComponent
defineComponent,
onScopeDispose
} from '@vue/runtime-test'

describe('api: createApp', () => {
Expand Down Expand Up @@ -551,6 +552,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)
})

// config.compilerOptions is tested in packages/vue since it is only
// supported in the full build.
})
5 changes: 4 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 {
ConcreteComponent,
Data,
Expand Down Expand Up @@ -210,6 +211,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?


// TODO remove in 3.4
Expand Down Expand Up @@ -370,6 +372,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 @@ -397,7 +400,7 @@ export function createAppAPI<HostElement>(
runWithContext(fn) {
currentApp = app
try {
return fn()
return scope.run(fn) as ReturnType<typeof fn>
Mini-ghost marked this conversation as resolved.
Show resolved Hide resolved
} finally {
currentApp = null
}
Expand Down