-
Notifications
You must be signed in to change notification settings - Fork 124
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
Add new helpers
sub-module with getRequestExecutionContext
#450
Conversation
🦋 Changeset detectedLatest commit: 488ab56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
helpers
sum-module with getRequestExecutionContext
helpers
sub-module with getRequestExecutionContext
3901708
to
a9c09fb
Compare
🧪 Prereleases are available for testing 🧪 @cloudflare/next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/6497892763/npm-package-next-on-pages-450 @cloudflare/eslint-plugin-next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/6497892763/npm-package-eslint-plugin-next-on-pages-450 |
a9c09fb
to
e9576f4
Compare
Thanks a lot for the PR @Karibash 🙂 Sorry I haven't updated you in #233, anyways as I mentioned there your solution seems valid but I think we might go a different direction, maybe your solution could be a temporary one if nothing else but it depends on how things proceed with the other alternatives (if we can soon go with the team's preferred alternative we might just go straight to that one) On our side there aren't many updates (I've also been OOO for a couple of weeks) I'll need to chase the team up and try to get to the bottom of things, sorry for this taking a long time 😓 |
e9576f4
to
488ab56
Compare
Any updates on this? :) |
Hi @dihmeetree, sorry no, not yet, I've still waiting to see what happens with cloudflare/workerd#1213 Since, if that runtime API were to be released then any changes in next-on-pages would be unnecessary. (But the PR has been stale for a while and I am not sure if we are going to land it 😓) Let's wait a bit longer and see what happens. Also the changes in this PR do not give access to the request object which is also something that users might need access to, so I am not too sure about it 🤔 I'll try to speed things up with the team and see if the runtime API is going to be added or not, if it's not let's discuss further here and adjust the API accordingly 🙂 (I also need to check with @GregBrimble as I think he's also interested in this and I think he's considering a potential generic/non next-on-pages spoecific approach). (PS: so sorry this it taken so long 😓) |
Hi @Karibash, I wanted to let you know, eventually (finally! 😅) we've decided to implement a The utility basically works the exact same way yours does and is implemented in a similar way. Sorry for going ahead and opening a new PR for it, I hope you don't mind too much 🙇 Please have a look to see if like it 🙂 PS: since we've got #659 I'm closing this PR, I hope that's fine, if not please feel free to reopen it and discuss |
Add a helper function where ExecutionContext can be retrieved.
If you don't like this API, it's not a problem to close it.
We are currently using it in our project, and it's working well.
https://github.com/looks-to-me/looks-to-me/blob/3b5393ecc95e6e1af3e8bbd949ba5a246a23ec47/app/src/app/(main)/images/posts/%5Bid%5D/route.ts#L60
This implementation is based on the following pull request for reference.
cloudflare
platform object containing all the cloudflare context relative to the request #233