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

update react-automata #87

Merged
merged 2 commits into from
Oct 11, 2018
Merged

update react-automata #87

merged 2 commits into from
Oct 11, 2018

Conversation

kriswep
Copy link
Contributor

@kriswep kriswep commented Oct 9, 2018

This tackles #82 .
Updated the react-automata version, as well as react, react-dom and react-scripts.
The update to react-automata required some API changes, which were done in index.js. It also required react-test-renderer as a dependency. See react-automata #32 for a discussion.

Also, as asked in #82 I don't know what you mean by out of date, including the tests. Can you elaborate on that?

@GantMan
Copy link
Owner

GantMan commented Oct 10, 2018

Awesome work @kriswep - any way you could also update the React Native version?

@GantMan GantMan added the hacktoberfest-accepted Good for October credit in Hacktoberfest label Oct 10, 2018
@kriswep
Copy link
Contributor Author

kriswep commented Oct 10, 2018

Oh, I see, you were refering to the RN version with the test. Totally missed that.

Afraid I don't have a plain RN environment set up.
Would you be on board with converting to expo/create-react-native-app? I could work on that. If yes, should that happen in here, or in a new PR?

@GantMan
Copy link
Owner

GantMan commented Oct 10, 2018

No worries!

Since there's an existing react native app without expo, I'd like to keep it how it is. If you can file a ticket for a new issue saying "Update react-automata for React native" link to this, I think we can merge and close this one knowing there is a placeholder.

@kriswep
Copy link
Contributor Author

kriswep commented Oct 10, 2018

Please wait with merging. Just noticed an error with the clear transition after the upgrade. Will investigate.

@kriswep
Copy link
Contributor Author

kriswep commented Oct 10, 2018

Alright, should work now.
I had to basically force the transition actions on the CLEAR event to happen, without an actual transition.
That's why there is a cond on the CLEAR events and a value on the transition.

Also the issue regarding RN update was added, #88

@GantMan GantMan merged commit 997f30d into GantMan:master Oct 11, 2018
@GantMan
Copy link
Owner

GantMan commented Oct 11, 2018

Thanks so much @kriswep - welcome to the contributors!

@kriswep kriswep deleted the update-automata branch October 14, 2018 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Good for October credit in Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants