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.

Sign up for GitHub

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

Jump to bottom

Fix nested waitUntil #606

Merged
merged 5 commits into from Nov 13, 2023
Merged

Fix nested waitUntil #606

merged 5 commits into from Nov 13, 2023

Conversation

hansottowirtz
Copy link
Contributor

@hansottowirtz hansottowirtz commented Jun 14, 2023 •
edited

Fixes #605

  • I've tested that this behavior also occurs in actual Workers (see screenshot)
  • I've added a test
  • The values in await res.waitUntil() are in the order they have been waitUntil'ed
  • @miniflare/core now exports waitUntilAll, which handles awaiting arrays that change in length
Screenshot 2023-06-14 at 11 26 33

@changeset-bot
Copy link

changeset-bot bot commented Jun 14, 2023 •
edited

⚠️ No Changeset found

Latest commit: 2c8254c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hansottowirtz hansottowirtz changed the title wip: fix nested waitUntil Fix nested waitUntil Jun 14, 2023
@hansottowirtz hansottowirtz marked this pull request as draft June 14, 2023 08:38
@hansottowirtz hansottowirtz marked this pull request as ready for review June 14, 2023 08:54
@hansottowirtz hansottowirtz force-pushed the patch-1 branch 4 times, most recently from ab0581e to 78c2e0a Compare June 14, 2023 11:24
mrbbot
mrbbot reviewed Jun 28, 2023
View reviewed changes
Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

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

Hey! 👋 Thanks so much for fixing this and including a test! 😃 I've added a few suggestions.

packages/core/test/index.spec.ts Outdated Show resolved Hide resolved
packages/core/src/wait-until-all.ts Outdated Show resolved Hide resolved
packages/core/test/index.spec.ts Outdated Show resolved Hide resolved
.changeset/wet-taxis-cough.md Outdated Show resolved Hide resolved
packages/core/test/index.spec.ts Outdated Show resolved Hide resolved
packages/core/test/index.spec.ts Outdated Show resolved Hide resolved
@mrbbot
Copy link
Contributor

mrbbot commented Nov 13, 2023

Hey @hansottowirtz! Thanks again for this PR! I'm hoping to do a release today, and would like to include this change, so I've gone ahead and applied the fixups. Going to merge this now 👍

@mrbbot mrbbot merged commit 4725ccd into cloudflare:master Nov 13, 2023
6 checks passed
@mrbbot
Copy link
Contributor

mrbbot commented Jan 17, 2024

Hey! 👋 Apologies for the delay, but this has now been released as part of miniflare@2.14.2. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrbbot mrbbot mrbbot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Nested waitUntil not awaited
2 participants
@hansottowirtz @mrbbot