-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Upgrade TypeScript version used in the repo #3559
Conversation
🦋 Changeset detectedLatest commit: 78a84bf 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 78a84bf:
|
@@ -440,7 +440,7 @@ describe('context', () => { | |||
}); | |||
|
|||
it('should work with generic context', () => { | |||
function createMachineWithExtras<TContext>( | |||
function createMachineWithExtras<TContext extends {}>( |
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.
It probably was a slight bug in the past and it got uncovered by the new, more conservative, rules around unconstrained generics. To fix some inference problems we have introduced LowInfer
here:
xstate/packages/core/src/types.ts
Line 983 in 9f78588
context?: LowInfer<TContext | (() => TContext)>; |
We didn't really want to constrain the TContext
to be a subtype of {}
but that's the effect with the current version of TS. In practice, this shouldn't be a big problem because we only accept object contexts at runtime. Some people might be forced to add extends {}
in their generic declarations, just like I've done here, but that's just about it.
@davidkpiano I think this is ready to be reviewed and that we can roll with this |
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.
🚀
fixes #3557
The applied
NonNullable
is at the wrong level (I think?) but, for some reason, it works - I'm still rechecking why. It should be applied onTStateSchema['states']
and not onTStateSchema['states'][K]
. However, this doesn't work because there is a bug In TypeScript. I've already created a fix for it but I'm not sure how long will it take to land it: microsoft/TypeScript#50540Let's wait a couple of days to see what happens with that PR. In the meantime, we can recommend using
skipLibCheck: true
as a workaround.