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(reactivity-transform): respect const keyword #6993

Merged
merged 4 commits into from Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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 @@ -1190,8 +1203,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
Expand Up @@ -487,4 +487,27 @@ 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.`)
})
})
115 changes: 76 additions & 39 deletions packages/reactivity-transform/src/reactivityTransform.ts
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' ||
yyx990803 marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -708,7 +742,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