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

refactor: waitUntil passed around via ALS #733

Merged
merged 4 commits into from
Feb 10, 2025
Merged

refactor: waitUntil passed around via ALS #733

merged 4 commits into from
Feb 10, 2025

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Feb 10, 2025

@conico974 this a POC to fix #713

I tested locally that

  • waitUntil is available for middleware and the server
  • waitUntil is fixed when there is a nested request inside a long running request

The handler API is changed so this is a breaking change.

What do you think?

Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: 40fca1c

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

This PR includes changesets to release 3 packages
Name Type
@opennextjs/aws Minor
app-pages-router Patch
app-router 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

Copy link

pkg-pr-new bot commented Feb 10, 2025

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/aws@733

commit: 4f7c210

Copy link
Contributor

github-actions bot commented Feb 10, 2025

Copy link
Contributor

@conico974 conico974 left a comment

Choose a reason for hiding this comment

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

LGTM.
It should probably be in a minor release though, a big warning in the release note about the breaking change for custom wrapper should be enough.
We'll need to update the docs as well

@vicb
Copy link
Contributor Author

vicb commented Feb 10, 2025

LGTM. It should probably be in a minor release though, a big warning in the release note about the breaking change for custom wrapper should be enough. We'll need to update the docs as well

Thanks for the review.

I'll polish the code, add a BREAKING CHANGE changeset, and prepare a doc PR before this is released.

@vicb vicb changed the title [POC] refactor: store WaitUntil in ALS refactor: waitUntil passed around via ALS Feb 10, 2025
@vicb vicb force-pushed the waitUntil branch 2 times, most recently from a32d5fc to ea04483 Compare February 10, 2025 14:01
@vicb vicb requested a review from conico974 February 10, 2025 14:01
@vicb
Copy link
Contributor Author

vicb commented Feb 10, 2025

@conico974 added a commit to polish the code and add a changeset, PTAL

Copy link
Contributor

@conico974 conico974 left a comment

Choose a reason for hiding this comment

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

If you could just fix the biome linting issue and see my 2 little comments, other than that LGTM

packages/open-next/src/utils/promise.ts Outdated Show resolved Hide resolved
.changeset/pink-papayas-smoke.md Outdated Show resolved Hide resolved
@vicb
Copy link
Contributor Author

vicb commented Feb 10, 2025

Thanks for the review @conico974, I have addressed the comment.
I'll work on the doc PR before this is released.

@vicb vicb merged commit b59027a into main Feb 10, 2025
3 checks passed
@vicb vicb deleted the waitUntil branch February 10, 2025 14:34
@github-actions github-actions bot mentioned this pull request Feb 10, 2025
vicb added a commit to opennextjs/opennextjs-cloudflare that referenced this pull request Feb 10, 2025
vicb added a commit to opennextjs/opennextjs-cloudflare that referenced this pull request Feb 10, 2025
vicb added a commit to opennextjs/docs that referenced this pull request Feb 11, 2025
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.

[Bug] Concurrent requests with the cloudflare wrappers
2 participants