improve typing for unbound functions

[ajcrites]

This updates the creator APIs to more accurately express types for the returned function objects. For example, match(): would be treated as a bound function relying on 'this', but it is now updated to match: () =>. This will not trigger the @typescript-eslint/unbound-method rule when passing actions.act ionName.match.

[netlify]

✔️ Deploy Preview for redux-starter-kit-docs ready!

🔨 Explore the source changes: 0b767e6

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/60e62e1a3d335a00076738c4

😎 Browse the preview: https://deploy-preview-1263--redux-starter-kit-docs.netlify.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0b767e6:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration

[phryneas]

I'm still pretty much of the opinion that the ESLint authors do actually not understand how TypeScript works.

This rule is not matching up with anything the language TypeScript does. It's a behaviour the rule authors have fabricated on top of TypeScript without any correlation to the language.

Please check my comment here:
#1130 (comment)

There are differences between the two notations, but this is not one of them on the TypeScript level.

Quoting the TypeScript technical lead Ryan Cavanaugh: (also read the whole threads below)

Two differences: The second one is bivariant on its parameter types, and can have additional overloads added via interface augmentation

Actually, introducing that bivariance here would from my understanding make the type system weaker here, not stronger.

[ajcrites]

@phryneas after looking into it further, it seems like the preferred way of doing this would be the this: void annotation for the functions that don't use this which would still allow keeping the method syntax. That may have other ramifications though.

[phryneas]

Honestly is is just insane that some ESLint author thinks they add additional meaning to a programming language and now unrelated upstream libraries have to conform to their weird interpretation to not hurt their users. 🙁

I gotta sleep over this, but right now I'm honestly not really happy about either of those changes.

[ajcrites]

That's okay... I could be wrong on this, but the inability to augment the interface also prevents a local type definition from using the this: void annotation as well. If the only value that updating the typings here is to allow conforming to that rule, then maybe it isn't worth it. I think the rule itself is fairly innocuous since it just provides a warning that the method could require binding to the host object but there is no way to know.

I'm not the best TypeScripter, but in createAction, for example, the match property is assigned to the object as a function and is not part of a method declaration on the object. I think that this would make the match: () => syntax more accurate.

[phryneas]

Well, given that the changes themselves are innocent enough I'll go ahead and merge this in.

I still don't agree with the reason for it and would rather see that ESLint rule deprecated (or at least adjusted, even though I don't see how that intent could reasonably be done in a correct way), but given my previous interactions with ESLint, I don't see that happening.

And since I don't want to have this discussion again and again... Let's just get it in 😅

Thanks for the contribution - sorry for me being so grumpy 🙂