Readme example doesn't work

nvlang commented 5 months ago

Initial checklist

[X] I read the support docs
[X] I read the contributing guide
[X] I agree to follow the code of conduct
[X] I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

rehype-retext@4.0.0

Link to runnable example

https://stackblitz.com/edit/github-mfdkiw?file=example.js

Steps to reproduce

Go to minimal reproduction
Run node example.js

Expected behavior

As per readme, I expected

example.html
5:3-5:4   warning Use `An` before `implicit`, not `A` retext-indefinite-article retext-indefinite-article
6:12-6:19 warning Expected `and` once, not twice      and                       retext-repeated-words

⚠ 2 warnings

and

<!doctypehtml><meta charset=utf8><title>Hello!</title><article>A implicit sentence.<h1>This and and that.</h1></article>

to be logged.

Actual behavior

The following error is logged.

Error: parser.tokenize is not a function
    at _evaluate (https://zzmimxpqlgithub-wfd2.w-corp-staticblitz.com/blitz.9e2d28a3.js:40:786645)

Node.js 18.20.3

Note: When using TypeScript, there's also the following type error:

Argument of type '[Processor<Root, Root, undefined, undefined, undefined>]' is not assignable to parameter of type '[boolean] | [parser: Parser]'.
  Type '[Processor<Root, Root, undefined, undefined, undefined>]' is not assignable to type '[boolean]'.
    Type 'Processor<Root, Root, undefined, undefined, undefined>' is not assignable to type 'boolean'.

I removed TS from the reproduction to keep it truly minimal, however.

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

Addendum: Curiously enough, on Safari the minimal repro doesn't consistently log the error for me (it still doesn't work, though). On Chrome and Firefox the error is logged consistently. This feels like it's probably unrelated to this project, however.

ChristianMurphy commented 5 months ago

The type error is expected https://github.com/rehypejs/rehype-retext/blob/ee9c7d85e52d5fc4f58f68d091302c7dff4529a9/test.js#L52-L53

For the tokenize error, could you try isolating it some more? There's nothing in this code base which calls that property/method https://github.com/search?q=repo%3Arehypejs%2Frehype-retext%20tokenize&type=code

github-actions[bot] commented 5 months ago

Hi! Thanks for taking the time to contribute! This has been marked by a maintainer as needing more info. It’s not clear yet whether this is an issue. Here are a couple tips:

Spend time framing the issue! The more time you put into it, the more we will
Often, maintainers respond with why for several back and forths; rubber duck debugging might help avoid that
Folks posting issues sometimes fall for xy problems: asking for a certain solution instead of raising the root problem

Thanks, — bb

nvlang commented 5 months ago

The error seems to stem from hast-util-to-nlcst (repro — run pnpm i && node example.js):

[...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:350
      replacement = parser.tokenize(node.value)

TypeError: parser.tokenize is not a function
    at one ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:350:28)
    at all ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:389:22)
    at add ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:261:42)
    at implicit ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:323:34)
    at find ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:194:9)
    at findAll ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:214:7)
    at find ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:197:9)
    at findAll ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:214:7)
    at find ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:186:7)
    at toNlcst ([...]/node_modules/[...]/hast-util-to-nlcst/lib/index.js:164:3)

Node.js v22.3.0

I'll try to investigate some more.

nvlang commented 5 months ago

I just ran the readme example from hast-util-to-nlcst:

import {fromHtml} from 'hast-util-from-html'
import {toNlcst} from 'hast-util-to-nlcst'
import {ParseEnglish} from 'parse-english'
import {read} from 'to-vfile'
import {inspect} from 'unist-util-inspect'

const file = await read('example.html')
const tree = fromHtml(file)

console.log(inspect(toNlcst(tree, file, ParseEnglish)))

It seems to work fine. In any event, it seems the parser in parser.tokenize is the ParseEnglish export from parse-english, so even though the error is thrown in hast-util-to-nlcst, maybe it has more to do with parse-english.

On the other hand, since ParseEnglish works fine in the hast-util-to-nlcst example, maybe it's about how that parser is passed to toNlcst internally within the unified().use(...).use(...)... chain in the rehype-retext example?

wooorm commented 5 months ago

I had a local note somewhere about how I thought there was a bug here, which was likely fixed in remark-retext. That’s indeed what’s happening here.

There remains a TypeScript error here though, where it cannot handle the two overloads of this plugin in the use call.

wooorm commented 5 months ago

Fixed in 5.0.0!

rehypejs / rehype-retext