refactor: only wrap in quotes when needed

vuejs · May 13, 2022 · c113fa0 · c113fa0
1 parent 5f3f8e3
commit c113fa0
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 26 deletions.
diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/transformExpressions.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/transformExpressions.spec.ts.snap
@@ -2,7 +2,7 @@
 
 exports[`compiler: expression transform bindingMetadata inline mode 1`] = `
 "(_ctx, _cache) => {
-  return (_openBlock(), _createElementBlock(\\"div\\", null, _toDisplayString(__props['props']) + \\" \\" + _toDisplayString(_unref(setup)) + \\" \\" + _toDisplayString(setupConst) + \\" \\" + _toDisplayString(_ctx.data) + \\" \\" + _toDisplayString(_ctx.options), 1 /* TEXT */))
+  return (_openBlock(), _createElementBlock(\\"div\\", null, _toDisplayString(__props.props) + \\" \\" + _toDisplayString(_unref(setup)) + \\" \\" + _toDisplayString(setupConst) + \\" \\" + _toDisplayString(_ctx.data) + \\" \\" + _toDisplayString(_ctx.options), 1 /* TEXT */))
 }"
 `;
 

diff --git a/packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts b/packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts
@@ -524,7 +524,7 @@ describe('compiler: expression transform', () => {
         `<div>{{ props }} {{ setup }} {{ setupConst }} {{ data }} {{ options }}</div>`,
         { inline: true }
       )
-      expect(code).toMatch(`__props['props']`)
+      expect(code).toMatch(`__props.props`)
       expect(code).toMatch(`_unref(setup)`)
       expect(code).toMatch(`_toDisplayString(setupConst)`)
       expect(code).toMatch(`_ctx.data`)

diff --git a/packages/compiler-core/src/transforms/transformExpression.ts b/packages/compiler-core/src/transforms/transformExpression.ts
@@ -24,7 +24,13 @@ import {
   walkIdentifiers
 } from '../babelUtils'
 import { advancePositionWithClone, isSimpleIdentifier } from '../utils'
-import { isGloballyWhitelisted, makeMap, hasOwn, isString } from '@vue/shared'
+import {
+  isGloballyWhitelisted,
+  makeMap,
+  hasOwn,
+  isString,
+  genPropsAccessExp
+} from '@vue/shared'
 import { createCompilerError, ErrorCodes } from '../errors'
 import {
   Node,
@@ -185,10 +191,10 @@ export function processExpression(
       } else if (type === BindingTypes.PROPS) {
         // use __props which is generated by compileScript so in ts mode
         // it gets correct type
-        return `__props['${raw}']`
+        return genPropsAccessExp(raw)
       } else if (type === BindingTypes.PROPS_ALIASED) {
         // prop with a different local alias (from defineProps() destructure)
-        return `__props['${bindingMetadata.__propsAliases![raw]}']`
+        return genPropsAccessExp(bindingMetadata.__propsAliases![raw])
       }
     } else {
       if (type && type.startsWith('setup')) {

diff --git a/packages/compiler-sfc/__tests__/__snapshots__/compileScriptPropsTransform.spec.ts.snap b/packages/compiler-sfc/__tests__/__snapshots__/compileScriptPropsTransform.spec.ts.snap
@@ -30,10 +30,10 @@ export default {
 
       
       let x = foo
-      let y = __props['foo']
+      let y = __props.foo
       
 return (_ctx, _cache) => {
-  return _toDisplayString(__props['foo'] + __props['foo'])
+  return _toDisplayString(__props.foo + __props.foo)
 }
 }
 
@@ -49,10 +49,10 @@ export default {
   setup(__props) {
 
       
-      let x = __props['foo.bar']
+      let x = __props[\\"foo.bar\\"]
       
 return (_ctx, _cache) => {
-  return _toDisplayString(__props['foo.bar'])
+  return _toDisplayString(__props[\\"foo.bar\\"])
 }
 }
 
@@ -68,10 +68,10 @@ export default {
   setup(__props) {
 
       
-      console.log(__props['foo'])
+      console.log(__props.foo)
       
 return (_ctx, _cache) => {
-  return _toDisplayString(__props['foo'])
+  return _toDisplayString(__props.foo)
 }
 }
 
@@ -144,7 +144,7 @@ exports[`sfc props transform nested scope 1`] = `
       
       function test(foo) {
         console.log(foo)
-        console.log(__props['bar'])
+        console.log(__props.bar)
       }
       
 return () => {}
@@ -153,6 +153,25 @@ return () => {}
 }"
 `;
 
+exports[`sfc props transform non-identifier prop names 1`] = `
+"import { toDisplayString as _toDisplayString } from \\"vue\\"
+
+
+export default {
+  props: ['onUpdate:foo'],
+  setup(__props) {
+
+      
+      console.log(__props[\\"onUpdate:foo\\"])
+      
+return (_ctx, _cache) => {
+  return _toDisplayString(__props[\\"onUpdate:foo\\"])
+}
+}
+
+}"
+`;
+
 exports[`sfc props transform rest spread 1`] = `
 "import { createPropsRestProxy as _createPropsRestProxy } from 'vue'
 

diff --git a/packages/compiler-sfc/__tests__/__snapshots__/cssVars.spec.ts.snap b/packages/compiler-sfc/__tests__/__snapshots__/cssVars.spec.ts.snap
@@ -157,7 +157,7 @@ export default {
 _useCssVars(_ctx => ({
   \\"xxxxxxxx-color\\": (color),
   \\"xxxxxxxx-size\\": (size.value),
-  \\"xxxxxxxx-foo\\": (__props['foo'])
+  \\"xxxxxxxx-foo\\": (__props.foo)
 }))
 
         const color = 'red'

diff --git a/packages/compiler-sfc/__tests__/compileScriptPropsTransform.spec.ts b/packages/compiler-sfc/__tests__/compileScriptPropsTransform.spec.ts
@@ -20,8 +20,8 @@ describe('sfc props transform', () => {
       <template>{{ foo }}</template>
     `)
     expect(content).not.toMatch(`const { foo } =`)
-    expect(content).toMatch(`console.log(__props['foo'])`)
-    expect(content).toMatch(`_toDisplayString(__props['foo'])`)
+    expect(content).toMatch(`console.log(__props.foo)`)
+    expect(content).toMatch(`_toDisplayString(__props.foo)`)
     assertCode(content)
     expect(bindings).toStrictEqual({
       foo: BindingTypes.PROPS
@@ -40,7 +40,7 @@ describe('sfc props transform', () => {
     `)
     expect(content).not.toMatch(`const { foo, bar } =`)
     expect(content).toMatch(`console.log(foo)`)
-    expect(content).toMatch(`console.log(__props['bar'])`)
+    expect(content).toMatch(`console.log(__props.bar)`)
     assertCode(content)
     expect(bindings).toStrictEqual({
       foo: BindingTypes.PROPS,
@@ -112,9 +112,9 @@ describe('sfc props transform', () => {
     `)
     expect(content).not.toMatch(`const { foo: bar } =`)
     expect(content).toMatch(`let x = foo`) // should not process
-    expect(content).toMatch(`let y = __props['foo']`)
-    // should convert bar to __props['foo'] in template expressions
-    expect(content).toMatch(`_toDisplayString(__props['foo'] + __props['foo'])`)
+    expect(content).toMatch(`let y = __props.foo`)
+    // should convert bar to __props.foo in template expressions
+    expect(content).toMatch(`_toDisplayString(__props.foo + __props.foo)`)
     assertCode(content)
     expect(bindings).toStrictEqual({
       x: BindingTypes.SETUP_LET,
@@ -181,6 +181,20 @@ describe('sfc props transform', () => {
     assertCode(content)
   })
 
+  // #5425
+  test('non-identifier prop names', () => {
+    const { content } = compile(`
+      <script setup>
+      const { 'onUpdate:foo': foo } = defineProps(['onUpdate:foo'])
+      console.log(foo)
+      </script>
+      <template>{{ foo }}</template>
+    `)
+    expect(content).toMatch(`console.log(__props["onUpdate:foo"])`)
+    expect(content).toMatch(`toDisplayString(__props["onUpdate:foo"])`)
+    assertCode(content)
+  })
+
   describe('errors', () => {
     test('should error on deep destructure', () => {
       expect(() =>

diff --git a/packages/compiler-sfc/__tests__/cssVars.spec.ts b/packages/compiler-sfc/__tests__/cssVars.spec.ts
@@ -66,7 +66,7 @@ describe('CSS vars injection', () => {
     expect(content).toMatch(`_useCssVars(_ctx => ({
   "${mockId}-color": (color),
   "${mockId}-size": (size.value),
-  "${mockId}-foo": (__props['foo'])
+  "${mockId}-foo": (__props.foo)
 })`)
     expect(content).toMatch(
       `import { useCssVars as _useCssVars, unref as _unref } from 'vue'`

diff --git a/packages/reactivity-transform/src/reactivityTransform.ts b/packages/reactivity-transform/src/reactivityTransform.ts
@@ -21,7 +21,7 @@ import {
   walkFunctionParams
 } from '@vue/compiler-core'
 import { parse, ParserPlugin } from '@babel/parser'
-import { hasOwn, isArray, isString } from '@vue/shared'
+import { hasOwn, isArray, isString, genPropsAccessExp } from '@vue/shared'
 
 const CONVERT_SYMBOL = '$'
 const ESCAPE_SYMBOL = '$$'
@@ -489,17 +489,17 @@ export function transformAST(
             if (isProp) {
               if (escapeScope) {
                 // prop binding in $$()
-                // { prop } -> { prop: __prop_prop }
+                // { prop } -> { prop: __props_prop }
                 registerEscapedPropBinding(id)
                 s.appendLeft(
                   id.end! + offset,
                   `: __props_${propsLocalToPublicMap[id.name]}`
                 )
               } else {
-                // { prop } -> { prop: __prop.prop }
+                // { prop } -> { prop: __props.prop }
                 s.appendLeft(
                   id.end! + offset,
-                  `: __props['${propsLocalToPublicMap[id.name]}']`
+                  `: ${genPropsAccessExp(propsLocalToPublicMap[id.name])}`
                 )
               }
             } else {
@@ -518,11 +518,11 @@ export function transformAST(
                 `__props_${propsLocalToPublicMap[id.name]}`
               )
             } else {
-              // x --> __props['x']
+              // x --> __props.x
               s.overwrite(
                 id.start! + offset,
                 id.end! + offset,
-                `__props['${propsLocalToPublicMap[id.name]}']`
+                genPropsAccessExp(propsLocalToPublicMap[id.name])
               )
             }
           } else {

diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts
@@ -171,3 +171,11 @@ export const getGlobalThis = (): any => {
         : {})
   )
 }
+
+const identRE = /^[_$a-zA-Z\xA0-\uFFFF][_$a-zA-Z0-9\xA0-\uFFFF]*$/
+
+export function genPropsAccessExp(name: string) {
+  return identRE.test(name)
+    ? `__props.${name}`
+    : `__props[${JSON.stringify(name)}]`
+}