@richardtowers/remark-abbr

[richardTowers]

Hello!

I previously discussed building a micromark extension to support abbreviations in the style of PHP markdown extras and kramdown.

@wooorm and I came to the conclusion that doing this in pure micromark is tricky at the moment, so instead I've used a combination of micromark and mdast extensions to build a remark-abbr plugin.

For the most part, I'm pretty happy with it. The abbr syntax it supports is a bit weird, in that abbreviation calls don't have any syntax at all - they're just "whatever the label in the definition is". This meant I needed one pretty kludgy syntax tree transform, but I think the syntax is always going to require something a bit ugly so I'm okay with it.

You can get a feel for the syntax it supports by having a look at the fixtures.

I've published it under my npm user's namespace for now (@richardtowers/remark-abbr), because zmarkdown already have the main remark-abbr package. It sounds like they're open to contributions, but I thought I'd post here first to see if there were any comments before seeing if they're interested in taking on ownership of the code (or handing me the remark-abbr package name).

[ChristianMurphy]

Hey @richardTowers! 👋
Thanks for sharing your results, it looks good!

Some thoughts exploring the code.

• this type of a plugin is Processor (source: https://github.com/richardTowers/remark-abbr/blob/793f4492c1ad1306fe8ebe42864b86cf574f4c11/dev/index.js#L10, example https://github.com/remarkjs/remark-gfm/blob/d8e110edf0799741e0954a92dd22fc143b803bdb/lib/index.js#L34)
• It looks like some of the generated .d.ts files are included in source control, those can be ignored/removed (https://github.com/richardTowers/remark-abbr/blob/793f4492c1ad1306fe8ebe42864b86cf574f4c11/dev/index.d.ts#L1-L7)
• This is defining a new node type (source: https://github.com/richardTowers/remark-abbr/blob/793f4492c1ad1306fe8ebe42864b86cf574f4c11/dev/lib/mdast-util-abbr/index.js#L98) the new node type should be registered with TypeScript so completions work correct and utilities like unist-util-visit can correctly infer node information (example: https://github.com/syntax-tree/mdast-util-gfm-strikethrough/blob/8c8c836171d1b74242399f82866ef4d74bc20961/index.d.ts#L6-L19)
• It looks like robustness is a focus of this implementation, consider running a fuzz test to look for inputs which could cause parsing to fail (example: https://github.com/micromark/micromark/blob/08df4bcc15b2ad01cb297951f298c872ccdd4d35/package.json#L82)
• If you want to support formatting or transformations, consider adding a toMarkdown implementation so that the new AST node can be compiled back to markdown

[richardTowers]

Thanks for the comments @ChristianMurphy ! The this type was an easy fix, so I've done that. I'll have a think about the other suggestions when I get a minute.

[ChristianMurphy]

Another hopefully quick one.
I see you have type coverage as part of the pipeline https://github.com/richardTowers/remark-abbr/blob/998d930fae5dfaaf2f8e5ddc388bb9725530fd9b/package.json#L6
but I'm not seeing a coverage floor set

remark/package.json

Lines 58 to 63 in 97e6f14

	"typeCoverage": {
	"atLeast": 100,
	"detail": true,
	"ignoreCatch": true,
	"strict": true
	},

[richardTowers]

Thanks again for these suggestions - I've implemented them all, except the fuzzing one. It was a bit of a faff trying to work out how to type things correctly, but I think the implementation is better for it. I'm at 100% test and type coverage now 🎉

In terms of robustness, what I really care about is just that this implementation behaves close to the kramdown implementation, as I've got a large corpus of markdown using that currently. I'm (eventually) planning on testing that by bulk processing the entire corpus and checking that the HTML ASTs for every document match. There's some more context in the original discussion, but I'll need to implement a lot of other gross govspeak-specific syntax extensions before I'm able to do that 😅

[richardTowers]

One question I had was whether asking remark-rehype users to do:

  .use(remarkRehype, {
    handlers: {
      // Prevent empty divs
      abbrDefinition: () => undefined,
    },
  })

was the best I can achieve?

It feels like it should be possible to flag to the hast compiler that some mdast nodes should be skipped when compiling to hast...

Maybe something like:

data: {
  hIgnore: true,
},

?

Or alternatively, some way for remark-abbr to set the handler for abbrDefinitions? We can pass params to micromark and mdast-util-from-markdown, but I couldn't find a way for a plugin to pass config to remark-rehype.

[ChristianMurphy]

For now the top option.
I'd be curious to hear @wooorm's thoughts on adding a new .data.h type.

[wooorm]

Not supported yet, indeed.

I would probably think to support hName: null, to be handled as a sort of fragment?
null has such a meaning in several of our other tools (https://github.com/syntax-tree/hastscript#hselector-properties-children).
And, null is not allowed by the types yet, so it can be added.

hIgnore has problems with what hName: 'main', hChildren: […], hIgnore: true means.

[wooorm]

Nice work!

Trying out the example code:

• remark-gfm is used but not imported in example — is it needed?
• remark-abbr is imported but not used in example — i guess you meant to replace one with the other

---

There’s currently a type error. I made the error a bit more readable with this example code:

/**
 * @import {Options} from 'remark-rehype'
 */

import rehypeStringify from 'rehype-stringify'
import remarkAbbr from '@richardtowers/remark-abbr'
import remarkParse from 'remark-parse'
import remarkRehype from 'remark-rehype'
import {read} from 'to-vfile'
import {unified} from 'unified'

/** @type {Options} */
const options = {
  handlers: {
    abbrDefinition() {
      return undefined
    }
  }
}

const file = await unified()
  .use(remarkParse)
  .use(remarkAbbr)
  .use(remarkRehype, options)
  .use(rehypeStringify)
  .process(await read('example.md'))

console.log(String(file))

Object literal may only specify known properties, but 'abbrDefinition' does not exist in type 'Partial<Record<"blockquote" | "break" | "code" | "definition" | "delete" | "emphasis" | "footnoteDefinition" | "footnoteReference" | "heading" | "html" | "image" | "imageReference" | "inlineCode" | ... 12 more ... | "root", Handler>>'. Did you mean to write 'definition'?ts(2561)

TS doesn’t know the Node yet, it seems.

Reading through the node types:

• I don’t think AbbrDefinition is a parent, perhaps like a regular definition, it’s a void, or, if you’d use value instead of title, a literal
• Should Abbr really be a parent node? If it can only contain texts, is it not instead self a literal, just like text? It cannot contain two texts, right?
• You do not have to define position on Abbr, it comes through Parent (or Literal) from Node
• I recommend typing data as separate interfaces, for both nodes
• To make an AST that’s useful for others, you also have to think about what if a field is missing when someone passes a node, some fields may be typed as optional
• there are different content types, where nodes have to be registered: depending on where these things can occur (in headings? In block quotes? In lists?)
• might it be better to use the full word, abbreviation, instead of abbr?
• some of the fields, label, abbr, reference, seems like they can be streamlined?
• make sure to define a type in the types of your nodes, so it can be used as a descriminating union

Perhaps, like this:

import type {Data, Literal} from 'mdast'

export interface AbbrDefinitionData extends Data {}

export interface AbbrDefinition extends Literal {
  type: 'abbrDefinition'
  identifier: string
  data?: AbbrDefinitionData | undefined
}

export interface AbbrData extends Data {
  hName?: 'abbr' | null | undefined
  hProperties?: {
    title?: string | null | undefined
  }
}

export interface Abbr extends Literal {
  type: 'abbr'
  identifier: string
  data?: AbbrData | undefined
}

declare module 'mdast' {
  interface DefinitionContentMap {
    abbrDefinition: AbbrDefinition
  }

  interface PhrasingContentMap {
    abbr: Abbr
  }

  interface RootContentMap {
    abbrDefinition: AbbrDefinition
    abbr: Abbr
  }
}

Starting out with improved, clear types, should help you clean the types in your mdast utility.

---

Also, some more tests for you:

A&B, C&D, E&F.
G\&H, I\&J, K\&L.
M&N, O&P, Q&R.

*[A&B]: alpha
*[C\&D]: bravo
*[E&F]: charlie
*[G&H]: delta
*[I\&J]: echo
*[K&L]: foxtrot
*[M&N]: golf
*[O\&P]: hotel
*[Q&R]: india

This tests whether the 2 different escape mechanisms are used or not and how.

A B.
C
D.
E F.
G
H.

*[A B]: alpha
*[C D]: bravo
*[E
F]: charlie
*[G
H]: delta

This tests whether whether spaces/line endings are collapsed.

[richardTowers]

Thank you so much for this! I was very much channelling this guy's energy for a lot of the types:

But your suggestions make total sense. I've adopted those types in richardTowers/remark-abbr@7d7f1c7 and I think it cleans the code up a fair bit.

... unfortunately, I'm still getting the same type error in my attempt at using the package myself - maybe I've got something wrong in my package.json? Do I need to export something to make it pick up the module declaration?

You're correct that remarkGfm was a typo in the README - no points for guessing which README I shamelessly plagiarised 🙈 fix

I've also added the tests you suggested, and a line that checks [backslash\]escaped\]square\]brackets] in richardTowers/remark-abbr@484509b .

I think the other implementations (PHP markdown extra / kramdown) don't support backslash escapes, so this is a bit of a deviation from the de facto standard for this feature, but I think it's an improvement to allow these escapes.

[wooorm]

What’s likely missing is a /// <reference types="mdast-util-the-package" /> in your remark plugin types.

https://github.com/remarkjs/remark-directive/blob/876a45ad4c021c74bb0b65383e838c8f3181e1eb/lib/index.js#L3

The types have to be loaded somehow. Also actually importing another type from it should work:

https://github.com/remarkjs/remark-math/blob/88a9497e1ede93b958237c85edbf5651faeca7af/packages/remark-math/lib/index.js#L2

This also shows that you can use an @import {} from '...'

[richardTowers]

Ah, I'd tried a bit of those, but the key trick seems to be to get tsc to put them in the index.d.ts file - it's not enough for them to just be in the index.js file.

I couldn't find a nice way to do it that tsc wouldn't strip, so instead I've gone for this grim hack, which does at least work 🤷🏻. I'm sure there's a better way to do it, but I'll tackle that another time.