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

Readme example doesn't work #12

Closed
4 tasks done
nvlang opened this issue Jun 15, 2024 · 6 comments
Closed
4 tasks done

Readme example doesn't work #12

nvlang opened this issue Jun 15, 2024 · 6 comments
Labels
💪 phase/solved Post is done

Comments

@nvlang
Copy link

nvlang commented Jun 15, 2024

Initial checklist

Affected packages and versions

[email protected]

Link to runnable example

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

Steps to reproduce

  1. Go to minimal reproduction
  2. 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.

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jun 15, 2024
@ChristianMurphy
Copy link
Member

The type error is expected

rehype-retext/test.js

Lines 52 to 53 in ee9c7d8

// @ts-expect-error: to do: remove this when `retext-stringify` is released, hopefully.
.use(rehypeRetext, unified().use(retextEnglish))

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

@ChristianMurphy ChristianMurphy added the 🙉 open/needs-info This needs some more info label Jun 15, 2024

This comment has been minimized.

@nvlang
Copy link
Author

nvlang commented Jun 15, 2024

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
Copy link
Author

nvlang commented Jun 15, 2024

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 added a commit that referenced this issue Jun 17, 2024
@wooorm
Copy link
Member

wooorm commented Jun 17, 2024

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
Copy link
Member

wooorm commented Jun 17, 2024

Fixed in 5.0.0!

@wooorm wooorm added the 💪 phase/solved Post is done label Jun 17, 2024
@wooorm wooorm closed this as completed Jun 17, 2024
@github-actions github-actions bot removed 🤞 phase/open Post is being triaged manually 🙉 open/needs-info This needs some more info labels Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done
Development

No branches or pull requests

3 participants