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

Fixed an issue with action objects not receiving correct event types #3090

Merged
merged 4 commits into from
Apr 27, 2022

Conversation

Andarist
Copy link
Member

fixes #3026

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2022

🦋 Changeset detected

Latest commit: 555d712

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
xstate Patch

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 24, 2022

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 555d712:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration
XState TS types issue Issue #3026

@ghost
Copy link

ghost commented Feb 24, 2022

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

TEvent,
ResolveTypegenMeta<TTypesMeta, TEvent, BaseActionObject, TServiceMap>
>
options?: TTypesMeta extends infer EvaluatedTypesMeta
Copy link
Member Author

@Andarist Andarist Feb 24, 2022

Choose a reason for hiding this comment

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

This somehow forces TS to "evaluate" the final~ type for TTypesMeta quicker. My hunch is that it couldn't be discovered soon enough without typegen because tsTypes is an optional property, so it was missing then TS tried to "explore" the stuff further instead of using the fallback (TypegenDisabled default) immediately and then it couldn't compute stuff further down the road.

By doing it we kinda force it to utilize that TypegenDisabled default and it can proceed computing with that additional knowledge and that in turn allow for other stuff to be computed correctly.

@@ -165,7 +165,7 @@ type AllowAllEvents = {
};

export type ResolveTypegenMeta<
TTypesMeta extends TypegenConstraint,
TTypesMeta,
Copy link
Member Author

Choose a reason for hiding this comment

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

ResolveTypegenMeta isn't supposed to be used publicly, so we can just trust its input. The problem was that after this infer trick above TS couldn't provide that the EvaluatedTypesMeta actually satisfies this constraint. An alternative approach to removing this generic would be to use Cast<EvaluatedTypesMeta, TypegenConstraint> at those "call sites". This seemed to be easier though.

Simplified demo of the problem:
TS playground

@Andarist
Copy link
Member Author

Ok, the CI has actually found some problems with this... so I'll have to convert this to a draft 😭

@Andarist Andarist marked this pull request as draft February 24, 2022 21:35
@Andarist
Copy link
Member Author

I think this might be a somewhat minimal repro for the problem:
TS playground

Can't be completely sure until this gets fixed in TS though. I will be investigating this further.

…traint` when inferring for action object calls
@Andarist
Copy link
Member Author

That is wild... the workaround that I've discovered when debugging through TypeScript compiler... works.

I feel like it's the greatest victory of man over a computer. Gonna be dancing to loud music today.

@Andarist Andarist marked this pull request as ready for review April 27, 2022 09:36
# Conflicts:
#	packages/core/src/typegenTypes.ts
#	packages/core/test/types.test.ts
# Conflicts:
#	packages/core/test/types.test.ts
@Andarist Andarist merged commit c4f73ca into main Apr 27, 2022
@Andarist Andarist deleted the andarist/fix-3026 branch April 27, 2022 14:07
@github-actions github-actions bot mentioned this pull request Apr 27, 2022
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.

Bug: actions not properly typed when using schema and assign
2 participants