Skip to content

Commit

Permalink
feat(reactivity): improve support of getter usage in reactivity APIs (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Apr 2, 2023
1 parent dfb21a5 commit 59e8284
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 35 deletions.
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

0 comments on commit 59e8284

Please sign in to comment.