Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: allow additional handlers and durable objects #13207
base: main
Are you sure you want to change the base?
feat: allow additional handlers and durable objects #13207
Changes from all commits
b580a8d
a7c9854
2337ab4
05b8ad9
2d15411
8a418d9
69c2ff0
4989a48
b9333f3
c5c3a99
78a6177
5596bec
33f0fab
615909a
d6bdb75
14ca427
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As far as I can tell, the bundler should throw an error at build time if there isn’t a default export in the
handlers
file. I can’t see a nice way to handle there not being a default export, even an empty object is a sensible default/fallback.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.
Should we split the durable objects into a separate file/option to avoid this pitfall?
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.
I don’t think so, although it will be a minor annoyance to anyone only using Durable Objects. It will be a documented requirement and there will be an error if the default export is missing.
Removing the requirement of a default export in the future wouldn’t be a breaking change if there was an easy way at build time to check whether there’s a default export and modify the generated worker accordingly.
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.
We may have to add a check ourselves since the bundling step will be removed and delegated to Cloudflare's wrangler (meaning the error would only surface when deploying).
Is it possible to do something like:
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.
I would assume that would throw an error at build time if the handlers file is written in, e.g., TypeScript or if the handlers file references a module not available at build time (e.g., something available in Cloudflare Workers but not Node). Only trivial workers aren't going to have this problem.
I think this is just an unavoidable problem if bundling is delegated to Wrangler. A possible solution would be to parse the handlers file and look for a default export in the AST, but that seems overly complicated.