-
Notifications
You must be signed in to change notification settings - Fork 83
introduce new initOpenNextCloudflareForDev utility and make getCloudflareContext synchronous
#265
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
introduce new initOpenNextCloudflareForDev utility and make getCloudflareContext synchronous
#265
Conversation
🦋 Changeset detectedLatest commit: ed349ca The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
2447a8a to
f6510ac
Compare
commit: |
ddaa077 to
4753b0a
Compare
6648525 to
09b33de
Compare
|
Thought: Maybe we can This comment says "the function is an async one but it doesn't need to be awaited", it might be nice to add the rationale if we add the implementation to our repo. |
09b33de to
59419ca
Compare
Yes, we could, this would mean that everyone would always have to call but again if you strongly prefer us to go that route I'm ok with it
Yes, given the implementation of |
|
@vicb I've addressed all the feedback, the only thing left is to decide whether we want to force this sort of thing via a up to you, just let me know if you want to me make the change 👍 |
Give me some time to think about it, I'm not settled. Some thoughts I have for now about Cons:
Pros
|
Well there is only one con and many pros, I dislike that we need to then ask users to always call the function, but given the benefits it does seem like it might be worth it By the way, either way I am not too convinced about the name since this is a dev ( |
1c298b1 to
b234e3e
Compare
getCloudflareContext to work in middlewares via a new enableEdgeDevGetCloudflareContext utilityinitOpenNextCloudflareForDev utility and make getCloudflareContext synchronous
b234e3e to
d7f6572
Compare
vicb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cant't wait for this PR to get in 🎉
vicb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
…udflareContext` synchronous
Co-authored-by: Victor Berchet <victor@suumit.com>
Co-authored-by: Victor Berchet <victor@suumit.com>
Co-authored-by: Victor Berchet <victor@suumit.com>
da78a85 to
ed349ca
Compare
|
@dario-piotrowicz, would you mind posting this on Discord once you push a release? (0.4 + getContext sync) 🙏 |
This PR introduces a new
initOpenNextCloudflareForDevutility to add to the Next.js config file that makes thegetCloudflareContextwork in middlewares (or more generally, the edge runtime) during local development (vianext dev)It
getCloudflareContexthas also been converted to be synchronousFor more details see the changeset file:
.changeset/chilly-dryers-begin.mdC3 PR: cloudflare/workers-sdk#7903
Docs relative PR: opennextjs/docs#57
fixes #137
fixes #226