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

feat(reactivity): improve support of getter usage in reactivity APIs #7997

Merged
merged 11 commits into from Apr 2, 2023
Expand Up @@ -294,7 +294,7 @@ describe('sfc props transform', () => {
).toThrow(`Cannot assign to destructured props`)
})

test('should error when watching destructured prop', () => {
test('should error when passing destructured prop into certain methods', () => {
expect(() =>
compile(
`<script setup>
Expand All @@ -303,7 +303,9 @@ describe('sfc props transform', () => {
watch(foo, () => {})
</script>`
)
).toThrow(`"foo" is a destructured prop and cannot be directly watched.`)
).toThrow(
`"foo" is a destructured prop and should not be passed directly to watch().`
)

expect(() =>
compile(
Expand All @@ -313,7 +315,33 @@ describe('sfc props transform', () => {
w(foo, () => {})
</script>`
)
).toThrow(`"foo" is a destructured prop and cannot be directly watched.`)
).toThrow(
`"foo" is a destructured prop and should not be passed directly to watch().`
)

expect(() =>
compile(
`<script setup>
import { toRef } from 'vue'
const { foo } = defineProps(['foo'])
toRef(foo)
</script>`
)
).toThrow(
`"foo" is a destructured prop and should not be passed directly to toRef().`
)

expect(() =>
compile(
`<script setup>
import { toRef as r } from 'vue'
const { foo } = defineProps(['foo'])
r(foo)
</script>`
)
).toThrow(
`"foo" is a destructured prop and should not be passed directly to toRef().`
)
})

// not comprehensive, but should help for most common cases
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-sfc/src/compileScript.ts
Expand Up @@ -1442,7 +1442,7 @@ export function compileScript(
startOffset,
propsDestructuredBindings,
error,
vueImportAliases.watch
vueImportAliases
)
}

Expand Down
27 changes: 16 additions & 11 deletions packages/compiler-sfc/src/compileScriptPropsDestructure.ts
Expand Up @@ -32,7 +32,7 @@ export function transformDestructuredProps(
offset = 0,
knownProps: PropsDestructureBindings,
error: (msg: string, node: Node, end?: number) => never,
watchMethodName = 'watch'
vueImportAliases: Record<string, string>
) {
const rootScope: Scope = {}
const scopeStack: Scope[] = [rootScope]
Expand Down Expand Up @@ -152,6 +152,19 @@ export function transformDestructuredProps(
return false
}

function checkUsage(node: Node, method: string, alias = method) {
if (isCallOf(node, alias)) {
const arg = unwrapTSNode(node.arguments[0])
if (arg.type === 'Identifier') {
error(
`"${arg.name}" is a destructured prop and should not be passed directly to ${method}(). ` +
`Pass a getter () => ${arg.name} instead.`,
arg
)
}
}
}

// check root scope first
walkScope(ast, true)
;(walk as any)(ast, {
Expand All @@ -169,16 +182,8 @@ export function transformDestructuredProps(
return this.skip()
}

if (isCallOf(node, watchMethodName)) {
const arg = unwrapTSNode(node.arguments[0])
if (arg.type === 'Identifier') {
error(
`"${arg.name}" is a destructured prop and cannot be directly watched. ` +
`Use a getter () => ${arg.name} instead.`,
arg
)
}
}
checkUsage(node, 'watch', vueImportAliases.watch)
checkUsage(node, 'toRef', vueImportAliases.toRef)

// function scopes
if (isFunctionType(node)) {
Expand Down
71 changes: 70 additions & 1 deletion packages/dts-test/ref.test-d.ts
Expand Up @@ -7,10 +7,15 @@ import {
reactive,
proxyRefs,
toRef,
toValue,
toRefs,
ToRefs,
shallowReactive,
readonly
readonly,
MaybeRef,
MaybeRefOrGetter,
ComputedRef,
computed
} from 'vue'
import { expectType, describe } from './utils'

Expand All @@ -26,6 +31,8 @@ function plainType(arg: number | Ref<number>) {

// ref unwrapping
expectType<number>(unref(arg))
expectType<number>(toValue(arg))
expectType<number>(toValue(() => 123))

// ref inner type should be unwrapped
const nestedRef = ref({
Expand Down Expand Up @@ -203,6 +210,13 @@ expectType<Ref<string>>(p2.obj.k)
// Should not distribute Refs over union
expectType<Ref<number | string>>(toRef(obj, 'c'))

expectType<Ref<number>>(toRef(() => 123))
expectType<Ref<number | string>>(toRef(() => obj.c))

const r = toRef(() => 123)
// @ts-expect-error
r.value = 234

// toRefs
expectType<{
a: Ref<number>
Expand Down Expand Up @@ -319,3 +333,58 @@ describe('reactive in shallow ref', () => {

expectType<number>(x.value.a.b)
})

describe('toRef <-> toValue', () => {
function foo(
a: MaybeRef<string>,
b: () => string,
c: MaybeRefOrGetter<string>,
d: ComputedRef<string>
) {
const r = toRef(a)
expectType<Ref<string>>(r)
// writable
r.value = 'foo'

const rb = toRef(b)
expectType<Readonly<Ref<string>>>(rb)
// @ts-expect-error ref created from getter should be readonly
rb.value = 'foo'

const rc = toRef(c)
expectType<Readonly<Ref<string> | Ref<string>>>(rc)
// @ts-expect-error ref created from MaybeReadonlyRef should be readonly
rc.value = 'foo'

const rd = toRef(d)
expectType<ComputedRef<string>>(rd)
// @ts-expect-error ref created from computed ref should be readonly
rd.value = 'foo'

expectType<string>(toValue(a))
expectType<string>(toValue(b))
expectType<string>(toValue(c))
expectType<string>(toValue(d))

return {
r: toValue(r),
rb: toValue(rb),
rc: toValue(rc),
rd: toValue(rd)
}
}

expectType<{
r: string
rb: string
rc: string
rd: string
}>(
foo(
'foo',
() => 'bar',
ref('baz'),
computed(() => 'hi')
)
)
})
27 changes: 26 additions & 1 deletion packages/reactivity/__tests__/ref.spec.ts
Expand Up @@ -11,7 +11,12 @@ import {
} from '../src/index'
import { computed } from '@vue/runtime-dom'
import { shallowRef, unref, customRef, triggerRef } from '../src/ref'
import { isShallow, readonly, shallowReactive } from '../src/reactive'
import {
isReadonly,
isShallow,
readonly,
shallowReactive
} from '../src/reactive'

describe('reactivity/ref', () => {
it('should hold a value', () => {
Expand Down Expand Up @@ -275,6 +280,15 @@ describe('reactivity/ref', () => {
expect(toRef(r, 'x')).toBe(r.x)
})

test('toRef on array', () => {
const a = reactive(['a', 'b'])
const r = toRef(a, 1)
expect(r.value).toBe('b')
r.value = 'c'
expect(r.value).toBe('c')
expect(a[1]).toBe('c')
})

test('toRef default value', () => {
const a: { x: number | undefined } = { x: undefined }
const x = toRef(a, 'x', 1)
Expand All @@ -287,6 +301,17 @@ describe('reactivity/ref', () => {
expect(x.value).toBe(1)
})

test('toRef getter', () => {
const x = toRef(() => 1)
expect(x.value).toBe(1)
expect(isRef(x)).toBe(true)
expect(unref(x)).toBe(1)
//@ts-expect-error
expect(() => (x.value = 123)).toThrow()

expect(isReadonly(x)).toBe(true)
})

test('toRefs', () => {
const a = reactive({
x: 1,
Expand Down
3 changes: 3 additions & 0 deletions packages/reactivity/src/index.ts
Expand Up @@ -3,12 +3,15 @@ export {
shallowRef,
isRef,
toRef,
toValue,
toRefs,
unref,
proxyRefs,
customRef,
triggerRef,
type Ref,
type MaybeRef,
type MaybeRefOrGetter,
type ToRef,
type ToRefs,
type UnwrapRef,
Expand Down