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

typings: visitor typings for remark-stringify #426

Closed
ChristianMurphy opened this issue Jul 19, 2019 · 4 comments
Closed

typings: visitor typings for remark-stringify #426

ChristianMurphy opened this issue Jul 19, 2019 · 4 comments
Labels
👀 no/external This makes more sense somewhere else

Comments

@ChristianMurphy
Copy link
Member

Subject of the feature

Give visitors a type safe way to depend on super types of unist node

Problem

In this example

function gap(this: unified.Processor) {
  const Compiler = this.Compiler as typeof remarkStringify.Compiler
  const visitors = Compiler.prototype.visitors
  const original = visitors.heading

  visitors.heading = heading

  function heading(this: unified.Processor, node: Node, parent?: Parent) {
    // FIXME: remove need for explicit 'as' casting
    const headingNode = node as (Node & {depth: number})
    return (
      (headingNode.depth === 2 ? '\n' : '') +
      original.apply(this, [node, parent])
    )
  }
}

const plugin: unified.Attacher = gap

to get a HeadingNode from the visitor it needs to be cast as (Node & {depth: number}).
Which reduces the type safety since any property can be added in the {} block and typescript will accept it.

Expected behaviour

header should automatically of type HeaderNode

function gap(this: unified.Processor) {
  const Compiler = this.Compiler as typeof remarkStringify.Compiler
  const visitors = Compiler.prototype.visitors
  const original = visitors.heading

  visitors.heading = heading

  function heading(this: unified.Processor, node: HeaderNode, parent?: Parent) {
    return (
      (headingNode.depth === 2 ? '\n' : '') +
      original.apply(this, [node, parent])
    )
  }
}

const plugin: unified.Attacher = gap

This should be safe, because the visitors are staticly defined ahead of time.

proto.visitors = {
root: require('./visitors/root'),
text: require('./visitors/text'),
heading: require('./visitors/heading'),
paragraph: require('./visitors/paragraph'),
blockquote: require('./visitors/blockquote'),
list: require('./visitors/list'),
listItem: require('./visitors/list-item'),
inlineCode: require('./visitors/inline-code'),
code: require('./visitors/code'),
html: require('./visitors/html'),
thematicBreak: require('./visitors/thematic-break'),
strong: require('./visitors/strong'),
emphasis: require('./visitors/emphasis'),
break: require('./visitors/break'),
delete: require('./visitors/delete'),
link: require('./visitors/link'),
linkReference: require('./visitors/link-reference'),
imageReference: require('./visitors/image-reference'),
definition: require('./visitors/definition'),
image: require('./visitors/image'),
footnote: require('./visitors/footnote'),
footnoteReference: require('./visitors/footnote-reference'),
footnoteDefinition: require('./visitors/footnote-definition'),
table: require('./visitors/table'),
tableCell: require('./visitors/table-cell')
}

@ChristianMurphy
Copy link
Member Author

This is a continuation of #383 (comment)

Some previously discussed solutions


One option would be a generic.
Something like

  interface Transformer<I extends Node = Node, O extends Node = Node> {
    (
      node: I,
      file: VFileCompatible,
      next?: (error: Error | null, tree: O, file: VFile) => {}
    ): Error | O | Promise<O>
  }

Which would guarantee that the input and the output are a super type of a unist Node.
But could not guarantee that type I matches what the previous plugin output, meaning it would be possible for JavaScript errors to slip through the typings.


Another would be to add a generic to Processor, and set that generic to a union of all types for the give language.
For example

type AllUnifiedTypes = Node | Parent
const unifiedProcessor = unified<Settings, AllUnifiedTypes>

type AllRemarkTypes = List | ListItem | Table | ...
const remarkProcessor = unified<Settings, AllRemarkTypes>

But then there would be the question of how do plugins participate in the supported node typings?
And how are transitions between languages, like mdast to hast, handled?

wooorm pushed a commit that referenced this issue Jul 20, 2019
Related to unifiedjs/unified#53.
Related to unifiedjs/unified#54.
Related to unifiedjs/unified#56.
Related to unifiedjs/unified#57.
Related to unifiedjs/unified#58.
Related to unifiedjs/unified#59.
Related to unifiedjs/unified#60.
Related to unifiedjs/unified#61.
Related to unifiedjs/unified#62.
Related to unifiedjs/unified#63.
Related to unifiedjs/unified#64.
Related to #426.

Reviewed-by: Titus Wormer <tituswormer@gmail.com>
Reviewed-by: Junyoung Choi <fluke8259@gmail.com>
Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>

Co-authored-by: Junyoung Choi <fluke8259@gmail.com>
Co-authored-by: Christian Murphy <christian.murphy.42@gmail.com>
@wooorm wooorm added 🧒 semver/minor This is backwards-compatible change and removed needs pr labels Aug 12, 2019
@wooorm
Copy link
Member

wooorm commented Aug 25, 2019

@ChristianMurphy Is this actionable, and useful enough?

@ChristianMurphy
Copy link
Member Author

Actionable, yes.
Useful, a bit.
This got a lot easier to work around, now that unist-util-is has typings.
It would still be nice to have here, save an extra step.

@wooorm
Copy link
Member

wooorm commented Oct 13, 2020

We don’t have visitors anymore, so I’ll close this. Type changes can be done to mdast-util-to-markdown.

@wooorm wooorm closed this as completed Oct 13, 2020
@wooorm wooorm added 👀 no/external This makes more sense somewhere else and removed remark-stringify 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change labels Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else
Development

No branches or pull requests

2 participants