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

Typescript typings of nodes in v3 reduce to Object #206

Open
rictic opened this issue Jul 20, 2017 · 2 comments
Open

Typescript typings of nodes in v3 reduce to Object #206

rictic opened this issue Jul 20, 2017 · 2 comments

Comments

@rictic
Copy link

rictic commented Jul 20, 2017

The | Object in each of the main types is both correct and well intentioned, but it renders the type useless to the type checker, as foo | Object simplifies to Object for most types. (this might not have been true prior to typescript 2.4.1).

I can use parse5.AST.Default.Node and friends in my code, but the moment that I use any method on parse5, even ones on the default tree adapter like parse5.treeAdapters.default.getChildNodes I'm back to having Objects again.

One starting place for an improvement would be to make the TreeAdapter interface generic over its node type.

interface TreeAdapter<Node> {/*...*/}

One could go a step farther and make the interface generic over all of the subtypes, getting something like:

interface TreeAdapter<
    Node, 
    Attribute,
    Document extends Node, 
    DocumentFragment extends Node,
    Element extends Node,
    CommentNode extends Node, 
    ParentNode extends Node,
    TextNode extends Node,
    DocumentTypeNode extends Node> { 
  // ...
}

This is unwieldy, but users would rarely if ever encounter it unless they were writing their own TreeAdapters, as the default and htmlparser2 adapters would have all of the type variables filled in.

@inikulin
Copy link
Owner

inikulin commented Jul 24, 2017

One could go a step farther and make the interface generic over all of the subtypes

I've initially had thoughts about it, but eventually decided to implement it in the simplified manner. If you are keen to implement this, PR is always welcome!

@squidfunk
Copy link
Contributor

squidfunk commented Nov 8, 2018

This is actually solvable even easier. The best way would be to use union types and discriminators for type narrowing, which is already possible with the current design and which I do a lot in my code base because it allows for a lot of flexibility. An example:

export interface DefaultTreeDocumentType {
    nodeName: "#documentType"
    // other fields
}
export interface DefaultTreeDocument {
    nodeName: "#document"
    // other fields
}
export interface DefaultTreeDocumentFragment {
    nodeName: "#document-fragment"
    // other fields
}
export interface DefaultTreeCommendNode {
    nodeName: "#comment"
    // other fields
}
export interface DefaultTreeTextNode {
    nodeName: "#text"
    // other fields
}
export interface DefaultTreeElement {
    nodeName: "string" // catch all remaining nodes
    // other fields
}
export type DefaultNode =
  | DefaultTreeDocumentType
  | DefaultTreeDocument
  | DefaultTreeDocumentFragment
  | DefaultTreeCommendNode
  | DefaultTreeTextNode
  | DefaultTreeElement

Now, if you check for the type, the TypeScript compiler will immediately use the correct type

const node: DefaultNode = ...
if (node.type === "#text") {
  // TypeScript now knows that node is of type DefaultTreeTextNode
  console.log(node.value)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants