Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Make DocumentHandler optionals #75

Closed
wants to merge 1 commit into from
Closed

Make DocumentHandler optionals #75

wants to merge 1 commit into from

Conversation

GregBrimble
Copy link
Member

Just like ElementHandler, DocumentHandler's various functions should be optional, requiring only one of them.

@jayphelps
Copy link
Contributor

jayphelps commented Aug 25, 2021

I also ran into this. I wouldn't normally bump a PR, but we're enterprise customers so we're paying for this hehe 🤡 If Cloudflare is interested in including more outside maintainers of these types, I'd be willing to help! I know it can be hard to keep up.

@GregBrimble
Copy link
Member Author

Hi Jay,

I've recently been (re)hired by Cloudflare, so hopefully I can chase things up for you. Looks like this PR would actually break implementations where you define a class:

// A class can only implement an object type or intersection of object types with statically known members. ts(2422)
class MyDocumentHandler implements DocumentHandler {
  doctype() {}
}

And that means of course, that ElementHandler has the same problem currently. Note that this works fine though:

new HTMLRewriter().onDocument({
  doctype() {}
})

I'll play around with it this weekend/next week and see if there's anything I can do to make that work for everyone.

@mrbbot mrbbot mentioned this pull request Sep 24, 2021
threepointone added a commit that referenced this pull request Sep 30, 2021
Auto-Generated Types

Types are now automatically generated with releases by parsing the runtime's source code, and merging in overrides/docs defined in this repository (for generics, overloads, comments, etc).

webworker no longer needs to (and shouldn't) be included in tsconfig lib.

The final merged AST that's used to render the TypeScript types, workers.json, is also included in the repository. This could be used to generate bindings for other languages that compile to WebAssembly. Rust output is coming soon.

Closes: #55, #75, #76, #81, #84, #96, #97, #100, #101, #102, #105, #106, #107, #108
@mrbbot
Copy link
Contributor

mrbbot commented Sep 30, 2021

#112 has now been merged, which makes all members optional.

@mrbbot mrbbot closed this Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants