-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: createWebWorkerHandler
for Cloudflare and Deno. Deprecates createCloudflareHandler
#287
Conversation
You seem to have included a commit which is not needed, as it's changes are already present on the repository c632013 should be removed from the PR |
33aa5b8
to
7f7a9ab
Compare
7f7a9ab
to
0e6f333
Compare
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 good to me!
Just remove the package-lock.json
to avoid any conflicts
0e6f333
to
718663f
Compare
createWebWorkerHandler
generic to support both client typescreateWebWorkerHandler
for Cloudflare and Deno
718663f
to
0a6db47
Compare
3819a91
to
d272b01
Compare
I wish it is ready to be merged. Thanks @wolfy1339 and @gr2m for your help. ❤️ |
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.
Thank you Baoshan!
f680a93
to
6e1dc40
Compare
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.
one last thing, promise! We should test that createCloudflareHandler
is still working and that it logs out a deprecation message. I like to create a dedicated file to test deprecations: test/deprecations.test.ts
. That way it's easy to remove all deprecations each time we release a breaking version.
Would you mind adding the file, testing that createCloudflareHandler
still works as expected and that it logs the warning?
6e1dc40
to
51a3536
Compare
Thanks for the suggestion! Done. Feel free to let me know if there is still room to improve. |
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 good
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.
great PR, thank you Baoshan!
createWebWorkerHandler
for Cloudflare and DenocreateWebWorkerHandler
for Cloudflare and Deno. Deprecates createCloudflareHandler
🎉 This PR is included in version 3.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 3.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The current implementation of
createCloudflareHandler
is not generic, preventing it from accepting anOAuthApp
withclientType: "github-app"
.This PR also renames
createCloudflareHandler
tocreateWebWorkerHandler
. The same handler supports both Cloudflare workers and Deno. Documentation is updated to reflect the change.