re-rxjs / react-rxjs

React bindings for RxJS
https://react-rxjs.org
MIT License
546 stars 18 forks source link

Typing question with createSignal() #211

Closed hoclun-rigsep closed 3 years ago

hoclun-rigsep commented 3 years ago

The following code produces this complaint from the compiler: Expected 0 arguments, but got 1. TS2554

const [resetTo$, doResetTo] = createSignal();
doResetTo(5);

whereas this code will compile:

const [resetTo$, doResetTo] = createSignal(x => x);
doResetTo(5);

and so will this:

const [resetTo$, doResetTo] = createSignal<number>();
doResetTo(5);

I'm not sure I find this intuitive, or understand why that overload at https://github.com/re-rxjs/react-rxjs/blob/6444c03faf43f3d0e6db276cafeeb6a74bbd9e71/packages/utils/src/createSignal.ts#L23 is necessary. Assuming this is all as intended, do you wish me to document it?

voliva commented 3 years ago

Yep, I see we need to clarify this... maybe we can add a few examples in the API reference for createSignal.

There are different ways we currently use createSignal:

const [buttonPress$, buttonPressed] = createSignal();
// buttonPress$ is Observable<void>
// buttonPressed is () => void

// ...
<button onClick={() => buttonPressed()}>...</button>
const [itemComplete$, completeItem] = createSignal<number>();
// itemComplete$ is Observable<number>
// completeItem is (payload: number) => void

// ...

<button onClick={() => completeItem(id)}>...</button>

Note that the payload can be any type - it can also be an object or whatever

const [statusChange$, setStatus] = createSignal((id: number, status: Status) => ({ id, status }));
// statusChange$ is Observable<{id: number, status: Status}>
// setStatus is (id: number, status: Status) => void

// ...

<button onClick={() => setStatus(id, Status.Complete)}>...</button>

For some of these cases I would use createKeyedSignal though, because it's a bit more descriptive when dealing with keyed instances, but it's hard to come with examples without going into much detail.

hoclun-rigsep commented 3 years ago

Okay. So it looks like users should in general only be putting type parameters on the second case, maybe the first if they really feel like being explicit, and rely on genericity in third case. Good?

voliva commented 3 years ago

Yes... Well, it's just the way that's easier to use IMO.

It's not like they should put parameters explicitly, is that if they do, it will be easier to type and read IMO. It was designed like this to make it easier.