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

Improve typings in observable, computed, and map #111

Merged
merged 1 commit into from
May 21, 2020

Conversation

nettyso
Copy link
Contributor

@nettyso nettyso commented May 21, 2020

I'm very into Typescript (and I'll send you a PR of a Typescript+JSX+Snowpack Sinuous example soon!). I noticed it's hard to develop in TS with Sinuous. I'll show some examples:

Before

Here's observable and computed that both resolve to "any", so typing isn't useful:

image

This computed example is from your readme

image

Here's map showing an error about "Item":

image

After

The new typings infer the type given to the functions observable/computed/map which fixes these issues.

Observables won't default to Observable<any>:

image

Computed functions infer their return type too, so errors like this can be caught (where messages is an Observable<string[]>:

image

Also map doesn't use "Item" anymore, and instead infers types correctly from the function or list provided:

image

The only hiccup is people will see const list = observable([]) resolve as never[] if they have strict null checking on (as I do). See microsoft/TypeScript#8944. To fix this, as shown in the map picture just above, use observable([] as string[]).

insert<T>(el: Node, value: T, marker?: Node, current?: T, startNode?: Node): T;
h: typeof h;
hs: typeof hs;
insert<T>(el: Node, value: T, endMark?: Node, current?: T, startNode?: Node): T;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put these in the order of sinuous/packages/sinuous/h/src/index.js. Felt right to include h and hs, and you hadn't updated the parameter of insert's marker to be endMark

Copy link
Owner

@luwes luwes left a comment

Choose a reason for hiding this comment

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

thanks so much for your PR. I never really programmed in Typescript and added the typings based on other libs and after studying the TS docs a little so this is super helpful! LGTM

@luwes luwes merged commit d2b979e into luwes:master May 21, 2020
@nettyso nettyso deleted the ts-work branch June 1, 2020 23:51
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.

2 participants