fix(runtime-core): fix globalProperties in check on instance render proxy

Author: yyx990803

packages/runtime-core/__tests__/componentProxy.spec.ts

@@ -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'
 
@@ -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 = {
@@ -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)
@@ -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)
@@ -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',

packages/runtime-core/src/componentProxy.ts

@@ -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,
@@ -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)
   }
 }
 
@@ -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
     })
   })
 
@@ -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
       })
     })
@@ -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
     })
   })