Skip to content

Commit

Permalink
fix(compiler-sfc): treat const reactive() bindings as mutable
Browse files Browse the repository at this point in the history
  • Loading branch information
@yyx990803
yyx990803 committed Feb 10, 2021
1 parent cf67306 commit 03360ce
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/compiler-core/src/transforms/vFor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const transformFor = createStructuralDirectiveTransform(

const isStableFragment =
forNode.source.type === NodeTypes.SIMPLE_EXPRESSION &&
forNode.source.constType > ConstantTypes.CAN_SKIP_PATCH
forNode.source.constType > ConstantTypes.NOT_CONSTANT
const fragmentFlag = isStableFragment
? PatchFlags.STABLE_FRAGMENT
: keyProp
Expand Down
9 changes: 6 additions & 3 deletions packages/compiler-sfc/src/compileScript.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1036,12 +1036,15 @@ function walkDeclaration(
)
if (id.type === 'Identifier') {
let bindingType
if (
const userReactiveBinding = userImportAlias['reactive'] || 'reactive'
if (isCallOf(init, userReactiveBinding)) {
// treat reactive() calls as let since it's meant to be mutable
bindingType = BindingTypes.SETUP_LET
} else if (
// if a declaration is a const literal, we can mark it so that
// the generated render fn code doesn't need to unref() it
isDefineCall ||
(isConst &&
canNeverBeRef(init!, userImportAlias['reactive'] || 'reactive'))
(isConst && canNeverBeRef(init!, userReactiveBinding))
) {
bindingType = BindingTypes.SETUP_CONST
} else if (isConst) {
Expand Down

2 comments on commit 03360ce

@jods4
Copy link
Contributor

@jods4 jods4 commented on 03360ce Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yyx990803 Just noting there's a missed optimization opportunity by only checking
if (isCallOf(init, userImportAlias['ref'] || 'ref'))
It could include shallowRef and computed as well.

Also noting that those optimizations won't play nice with something like auto-ref.
It will work but it won't be able to simplify access with .value instead of unref().

  1. Although I'm gonna tweak the lib to enable import { ref } from "vue-auto-ref"), the called function might still be auto.ref or an alias that won't be known here.
    (I also note that you're only checking names and don't worry about shadowing variables in nested scopes. I agree it's very unlikely but something tells me that with as many users as Vue has, weird things are bound to happen).

  2. Even with 1. working, a plugin like auto-ref assigns to let as users will assign the variable as if it was a plain value. It is effectively a const, but that's special knowledge that compiler-sfc would need to have.

So it looks like there might be a small optimization benefit to having "standardized" magic refs as opposed to independent loaders.

@HcySunYang
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a PR to do something similar #2979 😅
It has a related issue that can be closed

Please sign in to comment.