Skip to content

Commit

Permalink
fix(runtime-core): fix globalProperties in check on instance render p…
Browse files Browse the repository at this point in the history
…roxy
  • Loading branch information
yyx990803 committed Apr 6, 2020
1 parent 4d19636 commit c28a919
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 24 deletions.
44 changes: 42 additions & 2 deletions packages/runtime-core/__tests__/componentProxy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { h, render, getCurrentInstance, nodeOps } from '@vue/runtime-test'
import {
h,
render,
getCurrentInstance,
nodeOps,
createApp
} from '@vue/runtime-test'
import { mockWarn } from '@vue/shared'
import { ComponentInternalInstance } from '../src/component'

Expand Down Expand Up @@ -137,6 +143,33 @@ describe('component: proxy', () => {
expect(instance!.sink.foo).toBe(1)
})

test('globalProperties', () => {
let instance: ComponentInternalInstance
let instanceProxy: any
const Comp = {
setup() {
return () => null
},
mounted() {
instance = getCurrentInstance()!
instanceProxy = this
}
}

const app = createApp(Comp)
app.config.globalProperties.foo = 1
app.mount(nodeOps.createElement('div'))

expect(instanceProxy.foo).toBe(1)

// set should overwrite globalProperties with local
instanceProxy.foo = 2
expect(instanceProxy.foo).toBe(2)
expect(instance!.sink.foo).toBe(2)
// should not affect global
expect(app.config.globalProperties.foo).toBe(1)
})

test('has check', () => {
let instanceProxy: any
const Comp = {
Expand All @@ -158,7 +191,11 @@ describe('component: proxy', () => {
instanceProxy = this
}
}
render(h(Comp, { msg: 'hello' }), nodeOps.createElement('div'))

const app = createApp(Comp, { msg: 'hello' })
app.config.globalProperties.global = 1

app.mount(nodeOps.createElement('div'))

// props
expect('msg' in instanceProxy).toBe(true)
Expand All @@ -168,6 +205,8 @@ describe('component: proxy', () => {
expect('bar' in instanceProxy).toBe(true)
// public properties
expect('$el' in instanceProxy).toBe(true)
// global properties
expect('global' in instanceProxy).toBe(true)

// non-existent
expect('$foobar' in instanceProxy).toBe(false)
Expand All @@ -178,6 +217,7 @@ describe('component: proxy', () => {
expect('baz' in instanceProxy).toBe(true)

// dev mode ownKeys check for console inspection
// should only expose own keys
expect(Object.keys(instanceProxy)).toMatchObject([
'msg',
'bar',
Expand Down
65 changes: 43 additions & 22 deletions packages/runtime-core/src/componentProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,22 +159,6 @@ export const PublicInstanceProxyHandlers: ProxyHandler<any> = {
}
},

has(
{
_: { data, accessCache, renderContext, type, sink }
}: ComponentPublicProxyTarget,
key: string
) {
return (
accessCache![key] !== undefined ||
(data !== EMPTY_OBJ && hasOwn(data, key)) ||
hasOwn(renderContext, key) ||
(type.props && hasOwn(normalizePropsOptions(type.props)[0], key)) ||
hasOwn(publicPropertiesMap, key) ||
hasOwn(sink, key)
)
},

set(
{ _: instance }: ComponentPublicProxyTarget,
key: string,
Expand Down Expand Up @@ -207,6 +191,35 @@ export const PublicInstanceProxyHandlers: ProxyHandler<any> = {
}
}
return true
},

has(
{
_: { data, accessCache, renderContext, type, sink, appContext }
}: ComponentPublicProxyTarget,
key: string
) {
return (
accessCache![key] !== undefined ||
(data !== EMPTY_OBJ && hasOwn(data, key)) ||
hasOwn(renderContext, key) ||
(type.props && hasOwn(normalizePropsOptions(type.props)[0], key)) ||
hasOwn(publicPropertiesMap, key) ||
hasOwn(sink, key) ||
hasOwn(appContext.config.globalProperties, key)
)
}
}

if (__DEV__ && !__TEST__) {
PublicInstanceProxyHandlers.ownKeys = (
target: ComponentPublicProxyTarget
) => {
warn(
`Avoid app logic that relies on enumerating keys on a component instance. ` +
`The keys will be empty in production mode to avoid performance overhead.`
)
return Reflect.ownKeys(target)
}
}

Expand All @@ -232,21 +245,31 @@ export function createDevProxyTarget(instance: ComponentInternalInstance) {

// expose internal instance for proxy handlers
Object.defineProperty(target, `_`, {
configurable: true,
enumerable: false,
get: () => instance
})

// expose public properties
Object.keys(publicPropertiesMap).forEach(key => {
Object.defineProperty(target, key, {
get: () => publicPropertiesMap[key](instance)
configurable: true,
enumerable: false,
get: () => publicPropertiesMap[key](instance),
// intercepted by the proxy so no need for implementation,
// but needed to prevent set errors
set: NOOP
})
})

// expose global properties
const { globalProperties } = instance.appContext.config
Object.keys(globalProperties).forEach(key => {
Object.defineProperty(target, key, {
get: () => globalProperties[key]
configurable: true,
enumerable: false,
get: () => globalProperties[key],
set: NOOP
})
})

Expand All @@ -264,9 +287,8 @@ export function exposePropsOnDevProxyTarget(
Object.keys(normalizePropsOptions(propsOptions)[0]).forEach(key => {
Object.defineProperty(proxyTarget, key, {
enumerable: true,
configurable: true,
get: () => instance.props[key],
// intercepted by the proxy so no need for implementation,
// but needed to prevent set errors
set: NOOP
})
})
Expand All @@ -280,9 +302,8 @@ export function exposeRenderContextOnDevProxyTarget(
Object.keys(toRaw(renderContext)).forEach(key => {
Object.defineProperty(proxyTarget, key, {
enumerable: true,
configurable: true,
get: () => renderContext[key],
// intercepted by the proxy so no need for implementation,
// but needed to prevent set errors
set: NOOP
})
})
Expand Down

0 comments on commit c28a919

Please sign in to comment.