Should we care about the workflows? Or can we merge directly? @OrbisK

Should we care about the workflows? Or can we merge directly? @OrbisK

looks like an typecheck issue (https://app.netlify.com/sites/vueuse/deploys/67a08230f106910008a97622). So we should care.

@OrbisK

the test that fails is this one

  it('should pass union type', () => {
    let count = 0

    const { on: onResult, trigger } = createEventHook<number | string>()

    // union type should be inferred
    onResult(value => count = 2)
    trigger(1)
    trigger(2)

    expect(count).toBe(2)
  })

Adding
T extends any[] ? (...param: T) => void

makes the type more complex so in order to interfere union types it should be declared
createEventHook<[number | string]>()

we should find a common ground for this case

@OrbisK @ilyaliao

changed the case from T extends any[] ? (...param: T) => void to [T] extends [any[]] ? (...param: T) => void

this keeps the backward capability also does not require [] for union types

i wonder if we should simplify slightly and reduce our use of any

something like this playground

type Callback<T> = IsAny<T> extends true
  ? (...param: unknown[]) => void
  : (
      [T] extends [void]
        ? () => void
        : T extends unknown[]
            ? (...param: T) => void
            : (param: T) => void
    )

its a weird type at the minute in main and in this PR because:

• Callback<void> is (...params: unknown[]) => void which seems odd
  • who is using this and why?
  • trigger is nonsense in main, because the first param is then void (i.e. (param0: void) => void)
• Callback<any> has ...params: any but should really at least be unknown[] IMO
  • though it could cause confusion since the type param is still any. people just shouldn't be passing this really
• Callback<string> results in (param0: string, ...params: unknown[])
  • is this because people couldn't specify multiple params before?
  • seems like this should be (param0: string) without the made up rest params, and the caller is responsible for defining any extra params

you can merge this PR as it currently stands, don't count me as a blocker. but you should probably at least change [T] extends [unknown[]] to T extends unknown[].

does anyone know why we test against [void] and [unknown[]]? what usage fails if we change it to void and unknown[]?

i wonder if we should simplify slightly and reduce our use of any

something like this playground

type Callback<T> = IsAny<T> extends true
  ? (...param: unknown[]) => void
  : (
      [T] extends [void]
        ? () => void
        : T extends unknown[]
            ? (...param: T) => void
            : (param: T) => void
    )

its a weird type at the minute in main and in this PR because:

• Callback<void> is (...params: unknown[]) => void which seems odd

  • who is using this and why?
  • trigger is nonsense in main, because the first param is then void (i.e. (param0: void) => void)

• Callback<any> has ...params: any but should really at least be unknown[] IMO

  • though it could cause confusion since the type param is still any. people just shouldn't be passing this really

• Callback<string> results in (param0: string, ...params: unknown[])

  • is this because people couldn't specify multiple params before?
  • seems like this should be (param0: string) without the made up rest params, and the caller is responsible for defining any extra params

you can merge this PR as it currently stands, don't count me as a blocker. but you should probably at least change [T] extends [unknown[]] to T extends unknown[].

does anyone know why we test against [void] and [unknown[]]? what usage fails if we change it to void and unknown[]?

For me, I would go with the simplest and cleanest solution:
type Callback<T> = [T] extends [any[]] ? (...params: T) => void : (param: T) => void;

with this one typing arguments is easy and clean you get what you expect and you are not limited

example: TsPlayground

yes but the conditional types are there to allow Callback<any> and Callback<void>

it'd be a breaking change to the types if we drop those

Callback<any>;
// actually represents (...params: any[]) => void
// DOES NOT represent (param: any) => void

Callback<void>;
// actually represents (...params: unknown[]) => void
// DOES NOT represent (param: void) => void

for whatever reason, we have these two special cases. and that's why the type looks so wonky

that's why I added the change as an extra condition not to have breaking changes

but if I change [T] extends [any[]] to T extends unknown[]. It will not infer with union types like this test case on

packages/shared/createEventHook/index.test.ts

  it('should pass union type', () => {
    let count = 0

    const { on: onResult, trigger } = createEventHook<number | string>()

    // union type should be inferred
    onResult(value => count = 2)
    trigger(1)
    trigger(2)

    expect(count).toBe(2)
  })

i wonder if we should simplify slightly and reduce our use of any

something like this playground

type Callback<T> = IsAny<T> extends true
  ? (...param: unknown[]) => void
  : (
      [T] extends [void]
        ? () => void
        : T extends unknown[]
            ? (...param: T) => void
            : (param: T) => void(...param: T) => void
    )

its a weird type at the minute in main and in this PR because:

• Callback<void> is (...params: unknown[]) => void which seems odd

  • who is using this and why?
  • trigger is nonsense in main, because the first param is then void (i.e. (param0: void) => void)

• Callback<any> has ...params: any but should really at least be unknown[] IMO

  • though it could cause confusion since the type param is still any. people just shouldn't be passing this really

• Callback<string> results in (param0: string, ...params: unknown[])

  • is this because people couldn't specify multiple params before?
  • seems like this should be (param0: string) without the made up rest params, and the caller is responsible for defining any extra params

you can merge this PR as it currently stands, don't count me as a blocker. but you should probably at least change [T] extends [unknown[]] to T extends unknown[].

does anyone know why we test against [void] and [unknown[]]? what usage fails if we change it to void and unknown[]?

That is a trick to tackle the way the conditional type expressions work. T extends void ? M : N would, when faced with Callback<never> resolve to never, regardless of M or N. When we check against [void], Callback<never> works a bit more intuitively, and it resolves to (...param: unknown[]) => void. Similarly, for union types (never is just a case of this, since it is a union of no types), the conditional distributes over the unioned types. So, Callback<string | number> resolves to ((param_0: string, ...param: unknown[]) => void) | (param_0: number, ...param: unknown[]) => void) which, I think, is not as intuitive (and neither intended) compared to (param_0: number | string, ...param: unknown[]) => void.

Other than that, I do agree that adding the additional ...param: unknown[] seems a bit superfluous.

but if I change [T] extends [any[]] to T extends unknown[]. It will not infer with union types like this test case on

aha yeah, ok makes sense!

otherwise it just applies to each member of the union, so would be something like (p0: string) => void | (p0: number) => void

Callback works a bit more intuitively, and it resolves to (param: unknown) => void

to be honest i'd love to see these three weird cases vanish in the next major version

Callback<never>; // (...params: unknown[]) => void
Callback<any>; // (...params: any) => void
Callback<void>; // (..params: unknown[]) => void

i actually don't think it is intuitive that any of these resolve to (param: unknown) => void. i think they're all very strange and are the sole reason Callback isn't a much simpler type

i would've expected this:

Callback<never>; // (param: never) => void
Callback<any>; // (param: any) => void
Callback<void>; // (param: void) => void

then i'd ask the consumer what they're trying to do, as the never and void examples here seem like nonsense

the void one seems like a hack to produce Callback<[]> (i.e. no args - but it actually produces one with any args!), and never, who knows.

i've taken this on a bit of a tangent so maybe we should discuss outside of this PR. but i do feel like this type is a bit of a mess right now

but i do feel like this type is a bit of a mess right now

maybe we can fine tune this, to solve the problem with v12 and do a follow up with the breaking changes for v13?

i think so

i think this PR is all good and we can discuss other changes for the next major