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

syncRef: type constraint not precise #3511

Closed
7 tasks done
yec-h opened this issue Oct 29, 2023 · 0 comments · Fixed by #3515
Closed
7 tasks done

syncRef: type constraint not precise #3511

yec-h opened this issue Oct 29, 2023 · 0 comments · Fixed by #3515

Comments

@yec-h
Copy link

yec-h commented Oct 29, 2023

Describe the bug

When left and right types are incompatible, the expected type constraint is that they must implement options.direction corresponding to options.transform. However, in reality, options are optional, which may lead to unexpected runtime behavior.

const num = ref(0) // Ref<number>
const bool = ref(true) // Ref<boolean>

// Expected(in typescript): compile-time error
// Actual: allowed
syncRef(num, bool) 

num.value++ // Causes "bool" to be assigned a number type
expect(bool).toBeTypeOf('boolean') // Error

bool.value = false // Causes "num" to be assigned a boolean type
expect(num).toBeTypeOf('number') // Error

Is this a compromise for some necessary scenario?

If not, try fixing it in the following ways:

The most straightforward way:
type SyncRefOptions<L, R = L> = ConfigurableFlushSync & {
  deep?: boolean
  immediate?: boolean
    
} & IfBothExtend<L, R, {
  direction?: 'both' | 'ltr' | 'rtl'
  transform?: Transform<L, R>
}, {
  direction?: 'both'
  transform: Required<Transform<L, R>>
} | {
  direction: 'ltr'
  transform: Transform<L, R> & { ltr: unknown }
} | {
  direction: 'rtl'
  transform: Transform<L, R> & { rtl: unknown }
}>

type IfBothExtend<T, U, A, B = never> = T extends U ? U extends T ? A : B : B
interface Transform<L, R> {
  ltr?: (left: L) => R
  rtl?: (right: R) => L
}


export function syncRef<L, R = L>(left: Ref<L>, right: Ref<R>, options: SyncRefOptions<L, R> ) :() => void
export function syncRef<T>(left: Ref<T>, right: Ref<T>, options?: SyncRefOptions<T> ) :() => void
Or consider treating direction as a generic:
type SyncRefOptions<L, R = L, D extends Direction = 'both'> = ConfigurableFlushSync & {
  deep?: boolean
  immediate?: boolean
    
  direction?: D
} & {
  transform?: Transform<L, R>
} & IfBothExtend<L, R, {}, {
  transform: Pick<Required<Transform<L, R>>, D extends 'both' ? 'ltr' | 'rtl' : D>
}>

type IfBothExtend<T, U, A, B = never> = T extends U ? U extends T ? A : B : B
type Direction = 'ltr' | 'rtl' | 'both'
interface Transform<L, R> {
  ltr?: (left: L) => R
  rtl?: (right: R) => L
}

export function syncRef<T>(left: Ref<T>, right: Ref<T>, options?: SyncRefOptions<T>): () => void
export function syncRef<L, R = L, D extends Direction = 'both'>(left: Ref<L>, right: Ref<R>, options: SyncRefOptions<L, R, D>): () => void

After the fix, the behavior aligns with expectations:

const num = ref(0) // Ref<number>
const bool = ref(true) // Ref<boolean>

// @ts-expect-error
syncRef(num, bool)()

// @ts-expect-error
syncRef(num, bool, { transform: {
  rtl: right => Number(right),
} })()

// @ts-expect-error
syncRef(num, bool, { transform: {
  ltr: left => !!left,
} })()

// allowed
syncRef(num, bool, { transform: {
  ltr: left => !!left,
  rtl: right => Number(right),
} })()

Reproduction

https://stackblitz.com/edit/vitejs-vite-t3mvtk/?file=src%2FApp.vue,src%2Ftest.ts

System Info

System:
OS: Linux 5.0 undefined
CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Memory: 0 Bytes / 0 Bytes
Shell: 1.0 - /bin/jsh
Binaries:
Node: 18.18.0 - /usr/local/bin/node
Yarn: 1.22.19 - /usr/local/bin/yarn
npm: 9.4.2 - /usr/local/bin/npm
pnpm: 8.9.2 - /usr/local/bin/pnpm
npmPackages:
@vueuse/core: ^10.5.0 => 10.5.0 
vue: ^3.2.25 => 3.2.31

Used Package Manager

npm

Validations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant