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

Stop using ExportedHandler for TypeScript examples #14006

Closed
Cherry opened this issue Apr 14, 2024 · 1 comment · Fixed by #14007
Closed

Stop using ExportedHandler for TypeScript examples #14006

Cherry opened this issue Apr 14, 2024 · 1 comment · Fixed by #14007
Assignees
Labels
content:edit Request for content edits documentation Documentation edits product:workers Related to Workers product

Comments

@Cherry
Copy link
Contributor

Cherry commented Apr 14, 2024

Existing documentation URL(s)

There are tonnes of places where ExportedHandler is used, but here's just one example:

What changes are you suggesting?

ExportedHandler results in significantly worsened DX when writing Workers in TypeScript. Instead of errors pointing to a place where the error actually occurs, they highlight the entire function instead, and echo a much more complex error stack back to the user. This makes it significantly harder to figure out where an issue might be occurring within a large worker.

Let's take the worker on the page linked above as an example. I've made one tiny edit in the worker that should be quick to see and resolve. Can you see it?

If I scroll up, you'll see the entire fetch declaration is highlighted as an error:

And the following error:

Type '(request: Request<unknown, IncomingRequestCfProperties<unknown>>, env: { PASSWORD: string; }) => Promise<false | Response>' is not assignable to type 'ExportedHandlerFetchHandler<{ PASSWORD: string; }, unknown>'.
  Type 'Promise<false | Response>' is not assignable to type 'Response | Promise<Response>'.
    Type 'Promise<false | Response>' is not assignable to type 'Promise<Response>'.
      Type 'false | Response' is not assignable to type 'Response'.
        Type 'boolean' is not assignable to type 'Response'.

So all I know, if I'm proficient enough in TypeScript to parse that error (it's pretty complex), is that somewhere in my function, I'm returning a boolean instead of a Response.


Now let's look at that, with the more traditional syntax of specifying function args:

All of a sudden, I'm now given the exact line the error occurs on, and a much simpler error to parse:
Type 'boolean' is not assignable to type 'Response'.

It's a little more verbose syntax, yes, but the DX that users get from it so significantly improved

I think the following PR (at least where it removes the req, env, ctx type annotations) should be reverted:

And then further usage of ExportedHandler removed too.

@Cherry
Copy link
Contributor Author

Cherry commented Apr 14, 2024

I've opened a PR to improve this at #14007.

kodster28 added a commit that referenced this issue Apr 23, 2024
* docs: remove ExportedHandler types to improve DX

Closes #14006

* docs: add return type annotations

* docs: add Env interface where appropriate

* docs: remove inline env types

* docs: update for ExportedHandler + return type annotation

* docs: more satisfies ExportedHandler

* fix: upstream merge

---------

Co-authored-by: kodster28 <kody@cloudflare.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content:edit Request for content edits documentation Documentation edits product:workers Related to Workers product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants