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

[TS] Void function interop #400

Merged
merged 1 commit into from Oct 13, 2020
Merged

Conversation

doasync
Copy link
Member

@doasync doasync commented Oct 9, 2020

Use case:

  type voidFn = () => void
  const fn = (callback: voidFn) => {
    if(callback) callback()
  }
  const voidEvent: Event<void> = createEvent();
  fn(voidEvent); // TS2345: Argument of type 'Event ' is not assignable to parameter of type 'voidFn'.

This PR adds void function interop:

  const voidFn: () => void = createEvent<void>()
  const voidFn1: () => void = createEvent<undefined>()
  const voidFn2: () => void = createEvent<any>()

  const event1 = createEvent<undefined>()
  event1()
  const event2 = createEvent<any>()
  event2()

The only weak spot is the text of the message:

  const event1 = createEvent<number>()
  event1() /* Error
    The 'this' context of type 'void' is not assignable to method's 'this' of type '"Error:
	Expected 1 argument, but got 0"'.
  */

All tests pass!

@doasync doasync changed the title Void function interop (TypeScript) [TS] Void function interop Oct 9, 2020
@doasync doasync force-pushed the void-function-interop branch from 7ff07f2 to dac78a2 Compare Oct 9, 2020
Copy link
Member

@zerobias zerobias left a comment

Choose a reason for hiding this comment

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

Tests indeed failed

@@ -83,6 +83,7 @@ export type CompositeName = {

export interface Event<Payload> extends Unit<Payload> {
(payload: Payload): Payload
(this: Payload extends void ? void : `Error: Expected 1 argument, but got 0`): void
Copy link
Member

Choose a reason for hiding this comment

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

What is this?

Copy link
Member Author

Choose a reason for hiding this comment

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

"this" is a type of context, it is void by default
CleanShot 2020-10-09 at 08 15 57@2x

Copy link
Member

Choose a reason for hiding this comment

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

What is `Error: Expected 1 argument, but got 0`?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just a text to show in the error message

Copy link
Member Author

Choose a reason for hiding this comment

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

According to tests, it will be shown only if Event expects an argument, but no parameters given.

@@ -355,9 +355,6 @@ describe('#filterMap', () => {
"
--typescript--
Type 'Event<number>' is not assignable to type 'Event<number | void>'.
Copy link
Member Author

@doasync doasync Oct 9, 2020

Choose a reason for hiding this comment

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

It is important to put "this" overload after the first one to keep this kind of messages right.

@doasync doasync force-pushed the void-function-interop branch from dac78a2 to 80ee403 Compare Oct 12, 2020
@doasync
Copy link
Member Author

doasync commented Oct 12, 2020

I've updated the snapshot, tests pass

@zerobias
Copy link
Member

Interesting solution, okay, lets try it. Thanks! 👍

@zerobias zerobias merged commit f4076e5 into effector:master Oct 13, 2020
2 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.

None yet

2 participants