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

chore(xstate): Set predictableActionArguments to true #2569

Merged
merged 4 commits into from Sep 12, 2022

Conversation

wlee221
Copy link
Contributor

@wlee221 wlee221 commented Sep 9, 2022

Description of changes

This sets predictableActionArugments to true, as suggested in xstate@4.33.0+:

It is advised to configure predictableActionArguments: true at the top-level of your machine config.

We tried this in #2468, but we found some regressions with Authenticator usages. Xstate has resolved them now:

Issue #, if available

See previous PR for technical details: #2468.

Description of how you validated changes

E2e test passing this time.

Notes

It's worth noting that we have some code that does not follow the best practices (statelyai/xstate#3533 (comment)). That'll be a separate refactor task, but will help prevent these problems in the future.

Checklist

  • PR description included
  • yarn test passes
  • Tests are updated
  • No side effects or sideEffects field updated
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@changeset-bot
Copy link

changeset-bot bot commented Sep 9, 2022

🦋 Changeset detected

Latest commit: 66284cc

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

This PR includes changesets to release 4 packages
Name Type
@aws-amplify/ui Patch
@aws-amplify/ui-react Patch
@aws-amplify/ui-vue Patch
@aws-amplify/ui-angular 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

@wlee221 wlee221 temporarily deployed to ci September 9, 2022 17:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 17:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 17:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 17:17 Inactive
@wlee221 wlee221 marked this pull request as ready for review September 9, 2022 20:36
@wlee221 wlee221 requested a review from a team as a code owner September 9, 2022 20:36
@wlee221 wlee221 changed the title chore(xstate): Use predictableActionArguments to true chore(xstate): Set predictableActionArguments to true Sep 9, 2022
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 21:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 21:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 21:17 Inactive
@wlee221 wlee221 temporarily deployed to ci September 9, 2022 21:17 Inactive
Copy link
Contributor

@reesscot reesscot left a comment

Choose a reason for hiding this comment

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

LGTM. Do we have a separate ticket already for the refactor work we should do here?

Copy link
Contributor

@ErikCH ErikCH left a comment

Choose a reason for hiding this comment

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

👍

@wlee221
Copy link
Contributor Author

wlee221 commented Sep 12, 2022

Do we have a separate ticket already for the refactor work we should do here?

Yes, I'll share it async.

@wlee221 wlee221 merged commit e242980 into main Sep 12, 2022
@wlee221 wlee221 deleted the xstate/predictable-action-arguments branch September 12, 2022 17:07
@github-actions github-actions bot mentioned this pull request Sep 12, 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.

None yet

4 participants