Skip to content

Commit

Permalink
fix(reactivity-transform): prohibit const assignment at compile time (#…
Browse files Browse the repository at this point in the history
…6993)

close #6992
  • Loading branch information
sxzz authored Nov 14, 2022
1 parent 87c72ae commit 3427052
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -247,5 +247,16 @@ describe('sfc props transform', () => {
)
).toThrow(`cannot reference locally declared variables`)
})

test('should error if assignment to constant variable', () => {
expect(() =>
compile(
`<script setup>
const { foo } = defineProps(['foo'])
foo = 'bar'
</script>`
)
).toThrow(`Assignment to constant variable.`)
})
})
})
29 changes: 21 additions & 8 deletions packages/compiler-sfc/src/compileScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ import {
Program,
ObjectMethod,
LVal,
Expression
Expression,
VariableDeclaration
} from '@babel/types'
import { walk } from 'estree-walker'
import { RawSourceMap } from 'source-map'
Expand Down Expand Up @@ -310,6 +311,7 @@ export function compileScript(
{
local: string // local identifier, may be different
default?: Expression
isConst: boolean
}
> = Object.create(null)

Expand Down Expand Up @@ -404,7 +406,11 @@ export function compileScript(
}
}

function processDefineProps(node: Node, declId?: LVal): boolean {
function processDefineProps(
node: Node,
declId?: LVal,
declKind?: VariableDeclaration['kind']
): boolean {
if (!isCallOf(node, DEFINE_PROPS)) {
return false
}
Expand Down Expand Up @@ -442,6 +448,7 @@ export function compileScript(
}

if (declId) {
const isConst = declKind === 'const'
if (enablePropsTransform && declId.type === 'ObjectPattern') {
propsDestructureDecl = declId
// props destructure - handle compilation sugar
Expand All @@ -468,12 +475,14 @@ export function compileScript(
// store default value
propsDestructuredBindings[propKey] = {
local: left.name,
default: right
default: right,
isConst
}
} else if (prop.value.type === 'Identifier') {
// simple destructure
propsDestructuredBindings[propKey] = {
local: prop.value.name
local: prop.value.name,
isConst
}
} else {
error(
Expand All @@ -494,11 +503,15 @@ export function compileScript(
return true
}

function processWithDefaults(node: Node, declId?: LVal): boolean {
function processWithDefaults(
node: Node,
declId?: LVal,
declKind?: VariableDeclaration['kind']
): boolean {
if (!isCallOf(node, WITH_DEFAULTS)) {
return false
}
if (processDefineProps(node.arguments[0], declId)) {
if (processDefineProps(node.arguments[0], declId, declKind)) {
if (propsRuntimeDecl) {
error(
`${WITH_DEFAULTS} can only be used with type-based ` +
Expand Down Expand Up @@ -1197,8 +1210,8 @@ export function compileScript(
if (decl.init) {
// defineProps / defineEmits
const isDefineProps =
processDefineProps(decl.init, decl.id) ||
processWithDefaults(decl.init, decl.id)
processDefineProps(decl.init, decl.id, node.kind) ||
processWithDefaults(decl.init, decl.id, node.kind)
const isDefineEmits = processDefineEmits(decl.init, decl.id)
if (isDefineProps || isDefineEmits) {
if (left === 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,4 +531,34 @@ describe('errors', () => {
`does not support rest element`
)
})

test('assignment to constant variable', () => {
expect(() =>
transform(`
const foo = $ref(0)
foo = 1
`)
).toThrow(`Assignment to constant variable.`)

expect(() =>
transform(`
const [a, b] = $([1, 2])
a = 1
`)
).toThrow(`Assignment to constant variable.`)

expect(() =>
transform(`
const foo = $ref(0)
foo++
`)
).toThrow(`Assignment to constant variable.`)

expect(() =>
transform(`
const foo = $ref(0)
bar = foo
`)
).not.toThrow()
})
})
115 changes: 76 additions & 39 deletions packages/reactivity-transform/src/reactivityTransform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ export function shouldTransform(src: string): boolean {
return transformCheckRE.test(src)
}

type Scope = Record<string, boolean | 'prop'>
interface Binding {
isConst?: boolean
isProp?: boolean
}
type Scope = Record<string, Binding | false>

export interface RefTransformOptions {
filename?: string
Expand Down Expand Up @@ -118,6 +122,7 @@ export function transformAST(
{
local: string // local identifier, may be different
default?: any
isConst?: boolean
}
>
): {
Expand Down Expand Up @@ -168,17 +173,20 @@ export function transformAST(
let escapeScope: CallExpression | undefined // inside $$()
const excludedIds = new WeakSet<Identifier>()
const parentStack: Node[] = []
const propsLocalToPublicMap = Object.create(null)
const propsLocalToPublicMap: Record<string, string> = Object.create(null)

if (knownRefs) {
for (const key of knownRefs) {
rootScope[key] = true
rootScope[key] = {}
}
}
if (knownProps) {
for (const key in knownProps) {
const { local } = knownProps[key]
rootScope[local] = 'prop'
const { local, isConst } = knownProps[key]
rootScope[local] = {
isProp: true,
isConst: !!isConst
}
propsLocalToPublicMap[local] = key
}
}
Expand Down Expand Up @@ -218,7 +226,7 @@ export function transformAST(
return false
}

function error(msg: string, node: Node) {
function error(msg: string, node: Node): never {
const e = new Error(msg)
;(e as any).node = node
throw e
Expand All @@ -229,10 +237,10 @@ export function transformAST(
return `_${msg}`
}

function registerBinding(id: Identifier, isRef = false) {
function registerBinding(id: Identifier, binding?: Binding) {
excludedIds.add(id)
if (currentScope) {
currentScope[id.name] = isRef
currentScope[id.name] = binding ? binding : false
} else {
error(
'registerBinding called without active scope, something is wrong.',
Expand All @@ -241,7 +249,8 @@ export function transformAST(
}
}

const registerRefBinding = (id: Identifier) => registerBinding(id, true)
const registerRefBinding = (id: Identifier, isConst = false) =>
registerBinding(id, { isConst })

let tempVarCount = 0
function genTempVar() {
Expand Down Expand Up @@ -296,7 +305,12 @@ export function transformAST(
isCall &&
(refCall = isRefCreationCall((decl as any).init.callee.name))
) {
processRefDeclaration(refCall, decl.id, decl.init as CallExpression)
processRefDeclaration(
refCall,
decl.id,
decl.init as CallExpression,
stmt.kind === 'const'
)
} else {
const isProps =
isRoot && isCall && (decl as any).init.callee.name === 'defineProps'
Expand All @@ -316,7 +330,8 @@ export function transformAST(
function processRefDeclaration(
method: string,
id: VariableDeclarator['id'],
call: CallExpression
call: CallExpression,
isConst: boolean
) {
excludedIds.add(call.callee as Identifier)
if (method === convertSymbol) {
Expand All @@ -325,16 +340,16 @@ export function transformAST(
s.remove(call.callee.start! + offset, call.callee.end! + offset)
if (id.type === 'Identifier') {
// single variable
registerRefBinding(id)
registerRefBinding(id, isConst)
} else if (id.type === 'ObjectPattern') {
processRefObjectPattern(id, call)
processRefObjectPattern(id, call, isConst)
} else if (id.type === 'ArrayPattern') {
processRefArrayPattern(id, call)
processRefArrayPattern(id, call, isConst)
}
} else {
// shorthands
if (id.type === 'Identifier') {
registerRefBinding(id)
registerRefBinding(id, isConst)
// replace call
s.overwrite(
call.start! + offset,
Expand All @@ -350,6 +365,7 @@ export function transformAST(
function processRefObjectPattern(
pattern: ObjectPattern,
call: CallExpression,
isConst: boolean,
tempVar?: string,
path: PathSegment[] = []
) {
Expand Down Expand Up @@ -384,21 +400,27 @@ export function transformAST(
// { foo: bar }
nameId = p.value
} else if (p.value.type === 'ObjectPattern') {
processRefObjectPattern(p.value, call, tempVar, [...path, key])
processRefObjectPattern(p.value, call, isConst, tempVar, [
...path,
key
])
} else if (p.value.type === 'ArrayPattern') {
processRefArrayPattern(p.value, call, tempVar, [...path, key])
processRefArrayPattern(p.value, call, isConst, tempVar, [
...path,
key
])
} else if (p.value.type === 'AssignmentPattern') {
if (p.value.left.type === 'Identifier') {
// { foo: bar = 1 }
nameId = p.value.left
defaultValue = p.value.right
} else if (p.value.left.type === 'ObjectPattern') {
processRefObjectPattern(p.value.left, call, tempVar, [
processRefObjectPattern(p.value.left, call, isConst, tempVar, [
...path,
[key, p.value.right]
])
} else if (p.value.left.type === 'ArrayPattern') {
processRefArrayPattern(p.value.left, call, tempVar, [
processRefArrayPattern(p.value.left, call, isConst, tempVar, [
...path,
[key, p.value.right]
])
Expand All @@ -412,7 +434,7 @@ export function transformAST(
error(`reactivity destructure does not support rest elements.`, p)
}
if (nameId) {
registerRefBinding(nameId)
registerRefBinding(nameId, isConst)
// inject toRef() after original replaced pattern
const source = pathToString(tempVar, path)
const keyStr = isString(key)
Expand All @@ -437,6 +459,7 @@ export function transformAST(
function processRefArrayPattern(
pattern: ArrayPattern,
call: CallExpression,
isConst: boolean,
tempVar?: string,
path: PathSegment[] = []
) {
Expand All @@ -462,12 +485,12 @@ export function transformAST(
// [...a]
error(`reactivity destructure does not support rest elements.`, e)
} else if (e.type === 'ObjectPattern') {
processRefObjectPattern(e, call, tempVar, [...path, i])
processRefObjectPattern(e, call, isConst, tempVar, [...path, i])
} else if (e.type === 'ArrayPattern') {
processRefArrayPattern(e, call, tempVar, [...path, i])
processRefArrayPattern(e, call, isConst, tempVar, [...path, i])
}
if (nameId) {
registerRefBinding(nameId)
registerRefBinding(nameId, isConst)
// inject toRef() after original replaced pattern
const source = pathToString(tempVar, path)
const defaultStr = defaultValue ? `, ${snip(defaultValue)}` : ``
Expand Down Expand Up @@ -520,9 +543,18 @@ export function transformAST(
parentStack: Node[]
): boolean {
if (hasOwn(scope, id.name)) {
const bindingType = scope[id.name]
if (bindingType) {
const isProp = bindingType === 'prop'
const binding = scope[id.name]

if (binding) {
if (
binding.isConst &&
((parent.type === 'AssignmentExpression' && id === parent.left) ||
parent.type === 'UpdateExpression')
) {
error(`Assignment to constant variable.`, id)
}

const { isProp } = binding
if (isStaticProperty(parent) && parent.shorthand) {
// let binding used in a property shorthand
// skip for destructure patterns
Expand Down Expand Up @@ -638,18 +670,20 @@ export function transformAST(
return this.skip()
}

if (
node.type === 'Identifier' &&
// if inside $$(), skip unless this is a destructured prop binding
!(escapeScope && rootScope[node.name] !== 'prop') &&
isReferencedIdentifier(node, parent!, parentStack) &&
!excludedIds.has(node)
) {
// walk up the scope chain to check if id should be appended .value
let i = scopeStack.length
while (i--) {
if (rewriteId(scopeStack[i], node, parent!, parentStack)) {
return
if (node.type === 'Identifier') {
const binding = rootScope[node.name]
if (
// if inside $$(), skip unless this is a destructured prop binding
!(escapeScope && (!binding || !binding.isProp)) &&
isReferencedIdentifier(node, parent!, parentStack) &&
!excludedIds.has(node)
) {
// walk up the scope chain to check if id should be appended .value
let i = scopeStack.length
while (i--) {
if (rewriteId(scopeStack[i], node, parent!, parentStack)) {
return
}
}
}
}
Expand Down Expand Up @@ -729,7 +763,10 @@ export function transformAST(
})

return {
rootRefs: Object.keys(rootScope).filter(key => rootScope[key] === true),
rootRefs: Object.keys(rootScope).filter(key => {
const binding = rootScope[key]
return binding && !binding.isProp
}),
importedHelpers: [...importedHelpers]
}
}
Expand Down

0 comments on commit 3427052

Please sign in to comment.