Skip to content

Fix race condition caused by calling clear immediately after announce#141

Merged
joshblack merged 4 commits intomainfrom
sid/fix-announce-after-clear
Sep 20, 2024
Merged

Fix race condition caused by calling clear immediately after announce#141
joshblack merged 4 commits intomainfrom
sid/fix-announce-after-clear

Conversation

@siddharthkp
Copy link
Member

@siddharthkp siddharthkp commented Sep 20, 2024

updateContainerWithMessage sets this.#pending = true and cleans up once it's done. However, there can be a timeout between finishing work and cleaning up

If liveRegion.clear() is called immediately after liveRegion.announce(), i.e. before the above clean up step, it resets this.#timeout but not this.#pending leaving all future announcements in a pending state.

Caught it in jest tests for SelectPanel

  • Add failing test
  • Add fix

@changeset-bot
Copy link

changeset-bot bot commented Sep 20, 2024

🦋 Changeset detected

Latest commit: b238f8e

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

This PR includes changesets to release 1 package
Name Type
@primer/live-region-element 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

@siddharthkp siddharthkp changed the title Fix announce after clear Fix race condition of calling clear quickly after announce Sep 20, 2024
@siddharthkp siddharthkp marked this pull request as ready for review September 20, 2024 15:15
@siddharthkp siddharthkp changed the title Fix race condition of calling clear quickly after announce Fix race condition caused by calling clear immediately after announce Sep 20, 2024
Copy link
Member

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Wow, amazing catch 🤯

@joshblack joshblack merged commit 273ddb9 into main Sep 20, 2024
@joshblack joshblack deleted the sid/fix-announce-after-clear branch September 20, 2024 15:21
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.

2 participants