-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fixed inference for assign
using PropertyAssigner
#3818
Conversation
🦋 Changeset detectedLatest commit: 48b68ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👇 Click on the image for a new way to code review
Legend |
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 48b68ff:
|
assignment: Assigner<TContext, TEvent> | PropertyAssigner<TContext, TEvent> | ||
assignment: | ||
| Assigner<LowInfer<TContext>, TEvent> | ||
| PropertyAssigner<LowInfer<TContext>, TEvent> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has some drawbacks when it comes to inferring standalone assign
s, like here:
const a1 = assign(/*...*/)
When Assigner
is used then it works just fine:
const a1 = assign((ctx: { counter: number }) => ({})) // TContext inferred to `{ counter: number }`
but when it comes to PropertyAssigner
then it doesn't infer TContext
and it leaves it as unknown
(the internal partial assigned function stays types correctly, it's about what the typeof a1
is):
const a1 = assign({
counter: (context: { counter: number }) => context.counter + 1,
})
It also doesn't work, in this context, with static values:
const a1 = assign({
counter: 100,
})
I think that LowInfer
combined with reverse mapped types is broken in TS, a minimal repro case of this can be found here: TS playground
fixes #3151