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

fix(runtime-core): ensure app instance can be garbage collected after unmount. (close #2907) #2908

Closed
wants to merge 2 commits into from
Closed
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
18 changes: 13 additions & 5 deletions packages/reactivity/__tests__/readonly.spec.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {
reactive,
readonly,
toRaw,
effect,
isProxy,
isReactive,
isReadonly,
markRaw,
effect,
reactive,
readonly,
ref,
shallowReadonly,
isProxy
toRaw
} from '../src'

/**
Expand Down Expand Up @@ -461,5 +461,13 @@ describe('reactivity/readonly', () => {
`Set operation on key "foo" failed: target is readonly.`
).not.toHaveBeenWarned()
})

test('should allow shallow und normal reactive for same target', () => {
const target = { foo: 1 }
const shallowProxy = shallowReadonly(target)
const normalProxy = readonly(target)

expect(normalProxy).not.toBe(shallowProxy)
})
})
})
11 changes: 10 additions & 1 deletion packages/reactivity/__tests__/shallowReactive.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { shallowReactive, isReactive, reactive } from '../src/reactive'
import { isReactive, reactive, shallowReactive } from '../src/reactive'

import { effect } from '../src/effect'

describe('shallowReactive', () => {
Expand All @@ -13,6 +14,14 @@ describe('shallowReactive', () => {
expect(isReactive(props.n)).toBe(true)
})

test('should allow shallow und normal reactive for same target', () => {
const target = { foo: 1 }
const shallowProxy = shallowReactive(target)
const normalProxy = reactive(target)

expect(normalProxy).not.toBe(shallowProxy)
})

describe('collections', () => {
test('should be reactive', () => {
const shallowSet = shallowReactive(new Set())
Expand Down
43 changes: 27 additions & 16 deletions packages/reactivity/src/baseHandlers.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,32 @@
import {
reactive,
readonly,
toRaw,
ITERATE_KEY,
pauseTracking,
resetTracking,
track,
trigger
} from './effect'
import {
ReactiveFlags,
Target,
reactive,
reactiveMap,
readonly,
readonlyMap,
reactiveMap
shallowReactiveMap,
shallowReadonlyMap,
toRaw
} from './reactive'
import { TrackOpTypes, TriggerOpTypes } from './operations'
import {
track,
trigger,
ITERATE_KEY,
pauseTracking,
resetTracking
} from './effect'
import {
isObject,
hasOwn,
isSymbol,
extend,
hasChanged,
hasOwn,
isArray,
isIntegerKey,
extend
isObject,
isSymbol
} from '@vue/shared'

import { isRef } from './ref'

const builtInSymbols = new Set(
Expand Down Expand Up @@ -77,7 +80,15 @@ function createGetter(isReadonly = false, shallow = false) {
return isReadonly
} else if (
key === ReactiveFlags.RAW &&
receiver === (isReadonly ? readonlyMap : reactiveMap).get(target)
receiver ===
(isReadonly
? shallow
? shallowReadonlyMap
: readonlyMap
: shallow
Copy link
Member

Choose a reason for hiding this comment

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

Here mixed other unrelated code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah damn. Somehow some changes meant for another branch slipped in here.

Wil fix it in a minute

? shallowReactiveMap
: reactiveMap
).get(target)
) {
return target
}
Expand Down
32 changes: 19 additions & 13 deletions packages/reactivity/src/reactive.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { isObject, toRawType, def } from '@vue/shared'
import { Ref, UnwrapRef } from './ref'
import { def, isObject, toRawType } from '@vue/shared'
import {
mutableCollectionHandlers,
readonlyCollectionHandlers,
shallowCollectionHandlers
} from './collectionHandlers'
import {
mutableHandlers,
readonlyHandlers,
shallowReactiveHandlers,
shallowReadonlyHandlers
} from './baseHandlers'
import {
mutableCollectionHandlers,
readonlyCollectionHandlers,
shallowCollectionHandlers
} from './collectionHandlers'
import { UnwrapRef, Ref } from './ref'

export const enum ReactiveFlags {
SKIP = '__v_skip',
Expand All @@ -27,7 +27,9 @@ export interface Target {
}

export const reactiveMap = new WeakMap<Target, any>()
export const shallowReactiveMap = new WeakMap<Target, any>()
export const readonlyMap = new WeakMap<Target, any>()
export const shallowReadonlyMap = new WeakMap<Target, any>()

const enum TargetType {
INVALID = 0,
Expand Down Expand Up @@ -91,7 +93,8 @@ export function reactive(target: object) {
target,
false,
mutableHandlers,
mutableCollectionHandlers
mutableCollectionHandlers,
reactiveMap
)
}

Expand All @@ -105,7 +108,8 @@ export function shallowReactive<T extends object>(target: T): T {
target,
false,
shallowReactiveHandlers,
shallowCollectionHandlers
shallowCollectionHandlers,
shallowReactiveMap
)
}

Expand Down Expand Up @@ -142,7 +146,8 @@ export function readonly<T extends object>(
target,
true,
readonlyHandlers,
readonlyCollectionHandlers
readonlyCollectionHandlers,
readonlyMap
)
}

Expand All @@ -159,15 +164,17 @@ export function shallowReadonly<T extends object>(
target,
true,
shallowReadonlyHandlers,
readonlyCollectionHandlers
readonlyCollectionHandlers,
shallowReadonlyMap
)
}

function createReactiveObject(
target: Target,
isReadonly: boolean,
baseHandlers: ProxyHandler<any>,
collectionHandlers: ProxyHandler<any>
collectionHandlers: ProxyHandler<any>,
proxyMap: WeakMap<Target, any>
) {
if (!isObject(target)) {
if (__DEV__) {
Expand All @@ -184,7 +191,6 @@ function createReactiveObject(
return target
}
// target already has corresponding Proxy
const proxyMap = isReadonly ? readonlyMap : reactiveMap
const existingProxy = proxyMap.get(target)
if (existingProxy) {
return existingProxy
Expand Down
1 change: 1 addition & 0 deletions packages/runtime-core/src/apiCreateApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ export function createAppAPI<HostElement>(
if (__DEV__ || __FEATURE_PROD_DEVTOOLS__) {
devtoolsUnmountApp(app)
}
delete app._container.__vue_app__
} else if (__DEV__) {
warn(`Cannot unmount an app that is not mounted.`)
}
Expand Down