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(syncRef): enhance syncRef type restrict #3515

Merged
merged 5 commits into from Nov 9, 2023

Conversation

Doctor-wu
Copy link
Member

@Doctor-wu Doctor-wu commented Oct 31, 2023

fix#3511

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.
⚠️ Slowing down new functions

Warning: Slowing down new functions

As the VueUse audience continues to grow, we have been inundated with an overwhelming number of feature requests and pull requests. As a result, maintaining the project has become increasingly challenging and has stretched our capacity to its limits. As such, in the near future, we may need to slow down our acceptance of new features and prioritize the stability and quality of existing functions. Please note that new features for VueUse may not be accepted at this time. If you have any new ideas, we suggest that you first incorporate them into your own codebase, iterate on them to suit your needs, and assess their generalizability. If you strongly believe that your ideas are beneficial to the community, you may submit a pull request along with your use cases, and we would be happy to review and discuss them. Thank you for your understanding.


Description

fix #3511
From the set theory perspective to restrict the option's type

Additional context


🤖 Generated by Copilot at 8495e56

This pull request improves the syncRef function and its TypeScript support. It adds more type checks and constraints for the function options and arguments, and it covers various use cases with a new test file.

🤖 Generated by Copilot at 8495e56

  • Restrict and infer the types of the syncRef function options based on the direction and the ref types (link, link)
  • Simplify the syncRef function implementation and handle the optional options parameter (link)
  • Add a test case for the syncRef function in packages/shared/syncRef/index.test.ts (link)

Copy link

@yec-h yec-h left a comment

Choose a reason for hiding this comment

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

From machine translation

// It seems that this implementation has better readability, but some details need further consideration.
export type SyncRefOptions<
  L,
  R,
  D extends Direction,
> = ConfigurableFlushSync & {
  /**
   * Watch deeply
   *
   * @default false
   */
  deep?: boolean;
  /**
   * Sync values immediately
   *
   * @default true
   */
  immediate?: boolean;

  /**
   * Direction of syncing. Value will be redefined if you define syncConvertors
   *
   * @default 'both'
   */
  direction?: D;
} & TransformFinal<L, R, D>;

type IfNever<T, A, B> = [T] extends [never] ? A : B;

// Determine the direction with data updates based on 'direction (D)'.
type DirectionNeedSync<D extends Direction> = D extends "both"
  ? "ltr" | "rtl"
  : D;

// Determine the direction with incompatible types when updating data based on 'L' and 'R'.
type DirectionIncompatibleType<L, R> =
  | (L extends R ? never : "ltr")
  | (R extends L ? never : "rtl");

// When there are data updates in a certain direction, and the types of data updates in that direction are incompatible, it requires the implementation of the corresponding transformation method.
type DirectionRequired<L, R, D extends Direction> = Extract<
  DirectionNeedSync<D>,
  DirectionIncompatibleType<L, R>
>;

type TransformFinal<L, R, D extends Direction> = {
  transform?: Partial<Transform<L, R>>;
  // transform?: Partial<Pick<Transform<L, R>, DirectionNeedSync<D>>>; Needs further consideration.
} & IfNever<
  DirectionRequired<L, R, D>,
  {},
  { transform: Pick<Transform<L, R>, DirectionRequired<L, R, D>> }
>;

中文

// 似乎这种实现的可读性更好一些,但部分细节有待推敲
export type SyncRefOptions<
  L,
  R,
  D extends Direction,
> = ConfigurableFlushSync & {
  /**
   * Watch deeply
   *
   * @default false
   */
  deep?: boolean;
  /**
   * Sync values immediately
   *
   * @default true
   */
  immediate?: boolean;

  /**
   * Direction of syncing. Value will be redefined if you define syncConvertors
   *
   * @default 'both'
   */
  direction?: D;
} & TransformFinal<L, R, D>;

type IfNever<T, A, B> = [T] extends [never] ? A : B;

// 根据 direction(D),确定有数据更新的方向
type DirectionNeedSync<D extends Direction> = D extends "both"
  ? "ltr" | "rtl"
  : D;

// 根据 L 和 R,确定数据更新时类型不兼容的方向
type DirectionIncompatibleType<L, R> =
  | (L extends R ? never : "ltr")
  | (R extends L ? never : "rtl");

// 当存在某方向的数据更新,且该方向的数据更新的类型不兼容时,则要求实现对应的转换方法
type DirectionRequired<L, R, D extends Direction> = Extract<
  DirectionNeedSync<D>,
  DirectionIncompatibleType<L, R>
>;

type TransformFinal<L, R, D extends Direction> = {
  transform?: Partial<Transform<L, R>>;
  // transform?: Partial<Pick<Transform<L, R>, DirectionNeedSync<D>>>; 有待推敲
} & IfNever<
  DirectionRequired<L, R, D>,
  {},
  { transform: Pick<Transform<L, R>, DirectionRequired<L, R, D>> }
>;

@Doctor-wu
Copy link
Member Author

Such clean codes! But in your way, it can't restrict users from passing more directions such as

    syncRef(ref0, ref1, {
      direction: 'rtl',
      transform: {
        ltr: v => v, // useless here, maybe it's users wrong, but if we can hint users, it will be great
      },
    })()

I will try to use a more elegant way to implement 😂

@Doctor-wu
Copy link
Member Author

Such clean codes! But in your way, it can't restrict users from passing more directions such as

    syncRef(ref0, ref1, {
      direction: 'rtl',
      transform: {
        ltr: v => v, // useless here, maybe it's users wrong, but if we can hint users, it will be great
      },
    })()

I will try to use a more elegant way to implement 😂

And now we have a cleaner implement 🎉

@yec-h
Copy link

yec-h commented Nov 6, 2023

Such clean codes! But in your way, it can't restrict users from passing more directions such as

    syncRef(ref0, ref1, {
      direction: 'rtl',
      transform: {
        ltr: v => v, // useless here, maybe it's users wrong, but if we can hint users, it will be great
      },
    })()

I will try to use a more elegant way to implement 😂

From machine translation

I made changes to my previous code, and this time it passed the test

type Equal<A, B> = A extends B ? (B extends A ? true : false) : false

type IfNever<T, A, B> = [T] extends [never] ? A : B

export type SyncRefOptions<
  L,
  R,
  D extends Direction,
> = ConfigurableFlushSync & {
  /**
   * Watch deeply
   *
   * @default false
   */
  deep?: boolean

  /**
   * Sync values immediately
   *
   * @default true
   */
  immediate?: boolean

  /**
   * Direction of syncing. Value will be redefined if you define syncConvertors
   *
   * @default 'both'
   */
  direction?: D
} & TransformFinal<L, R, D>

type Direction = 'ltr' | 'rtl' | 'both'

interface Transform<L, R> {
  ltr: (left: L) => R
  rtl: (right: R) => L
}

// Determine the direction of data updates based on the 'direction' (D).
type DirectionNeedSync<D extends Direction> = D extends 'both' ? 'ltr' | 'rtl' : D

// Determine the direction of data updates that are incompatible based on the types of L and R.
type DirectionIncompatibleType<L, R> =
  | (L extends R ? never : 'ltr')
  | (R extends L ? never : 'rtl')

// When there are data updates in a certain direction, and the types of data updates in that direction are incompatible,
// it requires the implementation of the corresponding transformation methods.
type DirectionRequired<L, R, D extends Direction> = Extract<
  DirectionNeedSync<D>,
  DirectionIncompatibleType<L, R>
>

// If D doesn't extend any type, TypeScript will consider that the 'transform' property doesn't exist properties
// (even though you can check the existence of the property using 'in', TypeScript would infer the property as 'unknown').
type TransformFinal<L, R, D extends Direction> = D extends any ? ({
  transform?: Pick<Partial<Transform<L, R>>, DirectionNeedSync<D>>
} & IfNever<
  DirectionRequired<L, R, D>,
  { },
  { transform: Pick<Transform<L, R>, DirectionRequired<L, R, D>> }
>) : never

@Doctor-wu
Copy link
Member Author

Such clean codes! But in your way, it can't restrict users from passing more directions such as

    syncRef(ref0, ref1, {
      direction: 'rtl',
      transform: {
        ltr: v => v, // useless here, maybe it's users wrong, but if we can hint users, it will be great
      },
    })()

I will try to use a more elegant way to implement 😂

From machine translation

I made changes to my previous code, and this time it passed the test

type Equal<A, B> = A extends B ? (B extends A ? true : false) : false

type IfNever<T, A, B> = [T] extends [never] ? A : B

export type SyncRefOptions<
  L,
  R,
  D extends Direction,
> = ConfigurableFlushSync & {
  /**
   * Watch deeply
   *
   * @default false
   */
  deep?: boolean

  /**
   * Sync values immediately
   *
   * @default true
   */
  immediate?: boolean

  /**
   * Direction of syncing. Value will be redefined if you define syncConvertors
   *
   * @default 'both'
   */
  direction?: D
} & TransformFinal<L, R, D>

type Direction = 'ltr' | 'rtl' | 'both'

interface Transform<L, R> {
  ltr: (left: L) => R
  rtl: (right: R) => L
}

// Determine the direction of data updates based on the 'direction' (D).
type DirectionNeedSync<D extends Direction> = D extends 'both' ? 'ltr' | 'rtl' : D

// Determine the direction of data updates that are incompatible based on the types of L and R.
type DirectionIncompatibleType<L, R> =
  | (L extends R ? never : 'ltr')
  | (R extends L ? never : 'rtl')

// When there are data updates in a certain direction, and the types of data updates in that direction are incompatible,
// it requires the implementation of the corresponding transformation methods.
type DirectionRequired<L, R, D extends Direction> = Extract<
  DirectionNeedSync<D>,
  DirectionIncompatibleType<L, R>
>

// If D doesn't extend any type, TypeScript will consider that the 'transform' property doesn't exist properties
// (even though you can check the existence of the property using 'in', TypeScript would infer the property as 'unknown').
type TransformFinal<L, R, D extends Direction> = D extends any ? ({
  transform?: Pick<Partial<Transform<L, R>>, DirectionNeedSync<D>>
} & IfNever<
  DirectionRequired<L, R, D>,
  { },
  { transform: Pick<Transform<L, R>, DirectionRequired<L, R, D>> }
>) : never

That's a suitable implement, but I prefer the set theory, I think it's more scientific 😂

@yec-h
Copy link

yec-h commented Nov 6, 2023

Such clean codes! But in your way, it can't restrict users from passing more directions such as

    syncRef(ref0, ref1, {
      direction: 'rtl',
      transform: {
        ltr: v => v, // useless here, maybe it's users wrong, but if we can hint users, it will be great
      },
    })()

I will try to use a more elegant way to implement 😂

From machine translation
I made changes to my previous code, and this time it passed the test

type Equal<A, B> = A extends B ? (B extends A ? true : false) : false

type IfNever<T, A, B> = [T] extends [never] ? A : B

export type SyncRefOptions<
  L,
  R,
  D extends Direction,
> = ConfigurableFlushSync & {
  /**
   * Watch deeply
   *
   * @default false
   */
  deep?: boolean

  /**
   * Sync values immediately
   *
   * @default true
   */
  immediate?: boolean

  /**
   * Direction of syncing. Value will be redefined if you define syncConvertors
   *
   * @default 'both'
   */
  direction?: D
} & TransformFinal<L, R, D>

type Direction = 'ltr' | 'rtl' | 'both'

interface Transform<L, R> {
  ltr: (left: L) => R
  rtl: (right: R) => L
}

// Determine the direction of data updates based on the 'direction' (D).
type DirectionNeedSync<D extends Direction> = D extends 'both' ? 'ltr' | 'rtl' : D

// Determine the direction of data updates that are incompatible based on the types of L and R.
type DirectionIncompatibleType<L, R> =
  | (L extends R ? never : 'ltr')
  | (R extends L ? never : 'rtl')

// When there are data updates in a certain direction, and the types of data updates in that direction are incompatible,
// it requires the implementation of the corresponding transformation methods.
type DirectionRequired<L, R, D extends Direction> = Extract<
  DirectionNeedSync<D>,
  DirectionIncompatibleType<L, R>
>

// If D doesn't extend any type, TypeScript will consider that the 'transform' property doesn't exist properties
// (even though you can check the existence of the property using 'in', TypeScript would infer the property as 'unknown').
type TransformFinal<L, R, D extends Direction> = D extends any ? ({
  transform?: Pick<Partial<Transform<L, R>>, DirectionNeedSync<D>>
} & IfNever<
  DirectionRequired<L, R, D>,
  { },
  { transform: Pick<Transform<L, R>, DirectionRequired<L, R, D>> }
>) : never

That's a suitable implement, but I prefer the set theory, I think it's more scientific 😂

Yes, your implementation looks more scientifically rigorous. But it's too hard for me to read😂, perhaps because I don't understand the type well enough.

@Doctor-wu
Copy link
Member Author

It's okay, as long as it works normally, why don't you contact me on Twitter, maybe we can make friends

@antfu antfu enabled auto-merge November 9, 2023 14:59
@antfu antfu added this pull request to the merge queue Nov 9, 2023
Merged via the queue into vueuse:main with commit 892666b Nov 9, 2023
4 checks passed
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 this pull request may close these issues.

syncRef: type constraint not precise
3 participants