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

No implicit any #147

Merged
merged 3 commits into from
Jun 7, 2024
Merged

No implicit any #147

merged 3 commits into from
Jun 7, 2024

Conversation

gustavoguichard
Copy link
Collaborator

Solves #146

This seemed to be impossible, I've done a lot of research and created an X thread with some experts.

The solutions seemed to be:

  • Forbid any at all which seems reasonable
  • Swap (...args: any[]) => any for (...args: never[]) => unknown. This one was suggested by our contributor @jly36963 which is a thread on TS repo itself, and it seems nice because if you don't declare the types the arguments are gonna be never so you can't use them. I was a bit worried about some comments about problems with default arguments - as you can read in the last comment on that thread.

This morning I decided to try and use the Function primitive. I know we are not supposed to use it for several reasons (it could mean any constructor, or callable object, etc) but it seems to do it here.

WDYT?

@@ -110,7 +116,7 @@ describe('fromSuccess', () => {
const a = composable(() => 1)

const c = fromSuccess(a)
type _R = Expect<Equal<typeof c, () => Promise<number>>>
type _R = Expect<Equal<typeof c, () => Promise<1>>>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@diogob I don't know why it changed.. narrower seems better. Didn't break any other test... any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand, but LGTM

Copy link
Contributor

@diogob diogob left a comment

Choose a reason for hiding this comment

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

looks awesome

@diogob diogob merged commit c540af3 into main Jun 7, 2024
1 check passed
@diogob diogob deleted the no-implicit-any branch June 7, 2024 18:20
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