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

Node data stricter types #1262

Merged
merged 3 commits into from
Jun 30, 2020
Merged

Node data stricter types #1262

merged 3 commits into from
Jun 30, 2020

Conversation

Andarist
Copy link
Member

Addresses #1261

I still need to write changeset for this but I would like to get feedback first.

  1. I've removed all | any that I could find because it's like multiplying by 0 - you always get 0 any, all other union members are just lost
  2. Had to rename StateNode#data to StateNode#doneData because TS couldn't infer things correctly due to overlapping .data properties in both InvokeConfig and StateNode which are both allowed for invoke property. A simplified version of the problem can be checked out here after uncommenting that single comment in the Config type

@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2020

🦋 Changeset is good to go

Latest commit: 36ed8d0

We got this.

This PR includes changesets to release 7 packages
Name Type
xstate Minor
@xstate/analytics Major
@xstate/graph Major
@xstate/immer Major
@xstate/react Major
@xstate/test Major
@xstate/vue Major

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 Jun 30, 2020

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 36ed8d0:

Sandbox Source
nifty-brook-7gzt2 Configuration
hungry-frost-5iy6h Configuration

@davidkpiano
Copy link
Member

davidkpiano commented Jun 30, 2020

Re: changeset, this probably fits into the "minor change" category since a "public" method is being changed from .data to .doneData (which is more accurate). We already have a pending minor version bump so this can just go into that.

@Andarist
Copy link
Member Author

Re: changeset, this probably fits into the "minor change" category since a "public" method is being changed from .data to .doneData

Yeah, I was planning to make it a minor. I assume you are OK with changes here so I'm going to add changesets later and ping you for a final review.

@davidkpiano
Copy link
Member

Yep, all good!

@Andarist
Copy link
Member Author

@davidkpiano ok, i've added changesets - please take a look.

@Andarist Andarist merged commit 0547710 into master Jun 30, 2020
@Andarist Andarist deleted the node-data-stricter-types branch June 30, 2020 18:51
@github-actions github-actions bot mentioned this pull request Jun 30, 2020
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.

None yet

2 participants