Chore: upgrade remark-parse to 9.0.0

[aduh95]

Changelog: https://github.com/remarkjs/remark/releases

[btmills]

@aduh95 thank you for opening this PR. It led me to a fix in v7 of the parser that fixes two long-outstanding bugs! v8 also appears to produce the same results as v7 including the fixes. v9 being a complete rewrite makes me a bit nervous, however, and I commented on two places that might have issues.

[btmills]

Aha! This reminded me of both #77 and #78. After some digging, it turns out [email protected] includes remarkjs/remark#423, which fixes both of those issues. I opened #175 with a few additional tests specifically demonstrating those fixes.

[btmills]

This test is... strange. It says it "preserves" the original line endings, yet the assertion normalized them to \n. However, it was consistent. Here I see there's one \r\n and another \n. Any idea why that is? If v9 of the parser now preserves original line endings, I'd expect both to be \r\n. If this behavior changes, I'll want to add some autofix tests that span lines to make sure we're using correct substitutions. That'd be a likely place to have an off-by-one error that causes syntax errors in the output.

[aduh95]

Yep, I agree it's weird. I think the last line of the code block is always normalized to \n, while the other line endings are preserved. It can be observed by adding another line inside the code block. Looks like an upstream issue.

[aduh95]

Oh no, it's actually due to this:

https://github.com/eslint/eslint-plugin-markdown/blob/f6a8ca0824b8a62d0af2a9eb512104564588b49e/lib/processor.js#L256-L260

It always adds a blank line and to the node value, and use \n without checking if it's relevant. I'm not sure why we add a blank line here…

For reference, here's the snippet I used to check the upstream implementation is working as expected:

console.log(
  require('mdast-util-from-markdown')([
    "```js",
    "var answer = 6 * 7;",
    "console.log(answer);",
    "```"
  ].join("\r\n")).children[0].value
); // 'var answer = 6 * 7;\r\nconsole.log(answer);'

[btmills]

I don't think this change is valid in the case of "negative" indents, i.e. lines of code in a code block that are less indented than the opening fence. The parser previously gave us those on a per-line basis so we could make the correct adjustment. That shows up in the change to the "should translate indented column numbers" test, where one of the messages' columns (4) is now greater than the line length (2).

[JounQin]

Can we support remark-parser<9 and remark-parse>=9 at the same time in case the change seems very small?

Besides, should unified be upgraded too?

[btmills]

Update on this: I really want to at least get to v7 to fix those two bugs you made me realize were related. I'm waiting for feedback from the rest of the team in the discussion for #175, which demonstrates those exact fixes, about how comfortable we are bumping parser major versions in a semver-minor release of eslint-plugin-markdown. While v7 fixes two known bugs in this plugin, I'm not sure yet about going all the way to v9, partially because it's a rewrite, which introduces more risk, and partially because of the negative indent changes that would require further work.

Can we support remark-parser<9 and remark-parse>=9 at the same time in case the change seems very small?

As long as we're comfortable bumping the dependency to at least v7 in a semver-minor release (hypothetically [email protected]), then we can do that immediately to fix those two parser-related bugs. If for some reason we don't want to bump the minimum dependency past v7 but need to support v9, we could look at supporting multiple major versions. That would bring significant implementation and test complexity, so there would need to be some compelling advantage to justify that tradeoff.

Besides, should unified be upgraded too?

That's lower priority than upgrading remark-parse because I'm not aware of any bugs caused by unified, but if someone wanted to look through the unified changelogs and verify that none of the changes will break eslint-plugin-markdown, sure.

[btmills]

I just merged #175, which includes the two fixes for #77 and #78 that you discovered, so thank you! Some of the changes with the new parser in remark-parse v9 don't seem to be compatible with this plugin's use case, particularly the code block indentation stuff. I don't yet know how non-trivial that work would be, but I'd welcome the research if someone wants to figure out. I think that's as as we can go for now, so I'll close this. Thanks for the push!