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(ui): Set predictableActionArguments to true #2468

Merged
merged 5 commits into from
Aug 19, 2022

Conversation

wlee221
Copy link
Contributor

@wlee221 wlee221 commented Aug 16, 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.

predictableActionArguments Feature Flag

This flag fixes two things (docs) to ensure that actions are executed in the right order with the right event:

  • runs actions in order they're declared in.
    • This does not affect us because we do not rely on action orders in auth machine, as suggested by xstate best practices.
  • XState will always call an action with the event directly responsible for the related transition.
    • This is applicable for transient ("always") transitions that are triggered with raise actions.
    • This does not affect us because we do not use this pattern in our state machine.
    • Also got response from XState team that this is not breaking.

Issue #, if available

Description of how you validated changes

Existing e2e tests. These test all the workflows, so I'm confident in this not being feature breaking.

Checklist

  • PR description included
  • yarn test passes
  • Tests are updated

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 Aug 16, 2022

🦋 Changeset detected

Latest commit: 71a2986

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 August 16, 2022 00:26 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 00:26 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 00:26 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 00:26 Inactive
@wlee221 wlee221 marked this pull request as ready for review August 16, 2022 20:03
@wlee221 wlee221 requested a review from a team as a code owner August 16, 2022 20:03
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 20:20 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 20:20 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 20:20 Inactive
@wlee221 wlee221 temporarily deployed to ci August 16, 2022 20:20 Inactive
ErikCH
ErikCH previously approved these changes Aug 16, 2022
hbuchel
hbuchel previously approved these changes Aug 17, 2022
@wlee221 wlee221 dismissed stale reviews from hbuchel and ErikCH via 71a2986 August 17, 2022 18:57
Copy link
Contributor

@ioanabrooks ioanabrooks left a comment

Choose a reason for hiding this comment

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

LGTM!

@wlee221 wlee221 enabled auto-merge (squash) August 17, 2022 19:27
@wlee221 wlee221 merged commit 665c426 into main Aug 19, 2022
@wlee221 wlee221 deleted the xstate-use-predictableActionArugments branch August 19, 2022 17:56
This was referenced Aug 19, 2022
@wlee221 wlee221 restored the xstate-use-predictableActionArugments branch August 19, 2022 22:26
@wlee221
Copy link
Contributor Author

wlee221 commented Aug 19, 2022

Closing until we find a better path forward.

wlee221 added a commit that referenced this pull request Aug 19, 2022
wlee221 added a commit that referenced this pull request Aug 19, 2022
calebpollman added a commit that referenced this pull request Aug 25, 2022
…latest release commit (#2485)

* chore: Update yarn.lock (#2440)

* update yarn.lock

* updated yarn.lock to include geocoder update

Co-authored-by: Joe Buono <joebuono@amazon.com>

* Fix location search bug (#2450)

* fix: update maplibre-gl-js-amplify to v2.0.3 to fix location search bug

* fix: update to maplibre-gl-js-amplify v2.0.4

* fix: pin maplibre-gl-js-amplify version

* Create hip-horses-arrive.md

* fix(react): hide svg icon images in a11y tree in Rating component (#2469)

* fix(react): hide svg icon images in a11y tree in Rating component

* Version Packages (#2461)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix(docs): type error on fragment (#2477)

Co-authored-by: 0618 <o.6180339@gmail.com>

* add @types/react to resolutions field

* chore(ui): Set `predictableActionArguments` to true (#2468)

* Set `predictableActionArguments` to true

* Create clean-suns-fix.md

* Add predictableActionArguments to `true`

* Delete the auth-with-username environment (#2489)

* Revert "chore(ui): Set `predictableActionArguments` to true (#2468)" (#2492)

This reverts commit 665c426.

* fix(ci): Change e2e environments to auth-with-username-no-attributes (#2491)

* Change e2e environments to auth-with-username-no-attributes

* Remove additional attribute from sign-up test

Co-authored-by: William Lee <43682783+wlee221@users.noreply.github.com>

* chore: remove stale chmod line in package.json (#2497)

* Remove chmod in package.json

* remove chmod in canary as well

* [REVERT] poc run without chmod

* [REVERT]: add missing environement

* [REVERT]: Add amplify cli

* trigger-ci

* [REVERT]: Remove unneeded codes in POC workflow

* use yarn environments pull instead of yarn pull

* Why doesn't this work?

* This should work!

* Remove duplciate shell call

* Remove poc workflow

* chore(hosting): Skip cypress binary installation (#2508)

* chore(environments): move environments to their own folder (#2501)

* Split environments into respective folders

* chore(environments): Update scripts for environments split (#2502)

* Initial approach

* Simpler solution

* Add auth/geo/datastore scripts

* Add general pull command

* Explicitly get path to the shell file

* fix typo

* chore(environments): update `awsExports` to point to new paths (#2504)

* Update react aws-exports

* Vue aws-exports update

* Angular aws-exports updates

* Dial code (#2459)

* rename countryCode to dialCode

* begin deprecating countryCode by adding in duplicate dialCode props and updating all examples to use dialCode props

* update props table

* duplicate phone number field tests using dialCode

* update tests

* add required field to phoneNumberFieldProps, added deprecation warning

* update e2e tests

* update phone number field documentation

* rename countrycode files to dialcode

* update angular and vue dial code select name

* update angular and vue to dial code

* return default label to country code and undo e2e test changes

* update unit tests

* update e2e tests to revert change to dial code

* Create swift-kids-help.md

* Update docs/src/pages/[platform]/components/phonenumberfield/react.mdx

add `role="none"` to docs alert

Co-authored-by: Heather Buchel <hbuchel@gmail.com>

* Update swift-kids-help.md

Co-authored-by: Jacob Logan <lognjc@amazon.com>
Co-authored-by: Heather Buchel <hbuchel@gmail.com>

* Add troubleshooting steps for CRA and geo components (#2511)

* fix(environments): fix environments pull on CI (#2512)

* POC run against guides

* Try bash instead of sh

* Add debug line

* Use printf instead of echo

* Add more comments

* Update import path in guides

* Adjust comments

* Adjust comment

* Remove extraneous comments

* More comments adjustment

* Revert "POC run against guides"

This reverts commit 88bceca.

* update changeset (#2515)

Co-authored-by: Jacob Logan <lognjc@amazon.com>

Co-authored-by: Joe Buono <joebuono724@gmail.com>
Co-authored-by: Joe Buono <joebuono@amazon.com>
Co-authored-by: thaddmt <68032955+thaddmt@users.noreply.github.com>
Co-authored-by: Heather Buchel <hbuchel@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: MJ <0618@users.noreply.github.com>
Co-authored-by: 0618 <o.6180339@gmail.com>
Co-authored-by: William Lee <43682783+wlee221@users.noreply.github.com>
Co-authored-by: Shane Laymance <shane.laymance@gmail.com>
Co-authored-by: jacoblogan <jacob.maiola.logan@gmail.com>
Co-authored-by: Jacob Logan <lognjc@amazon.com>
calebpollman pushed a commit to calebpollman/amplify-ui that referenced this pull request Aug 25, 2022
* Set `predictableActionArguments` to true

* Create clean-suns-fix.md

* Add predictableActionArguments to `true`
calebpollman pushed a commit to calebpollman/amplify-ui that referenced this pull request Aug 25, 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.

6 participants