fix: correct parent typing for rule visitors #571
Conversation
| * - For nodes, include every parent `P` where `T` is assignable to one of | ||
| * `P['children'][number]` (i.e. `T` can appear in `P.children`). | ||
| */ | ||
| type ParentOf<T extends MarkdownNode> = T extends Root |
There was a problem hiding this comment.
I think this should be static for performance and simplicity reasons as the parent of the various markdown nodes should be easily groupable (inside paragraph (e.g. Strong and Emphasis) and root (e.g. Heading and Code).
There was a problem hiding this comment.
Agreed. Also, deeply nested conditional types make the code somewhat unclear to me.
There was a problem hiding this comment.
Hi, @jaymarvelz
Could you share any PRs or issues that motivated this change, and also update the PR description to include the bug templates?
I'm having trouble understanding what's happening here.
Also, if this PR includes bug fixes, could you update tests/types/types.test.ts to verify the behavior before and after the change?
| Definition, | ||
| Emphasis, | ||
| Heading, | ||
| Html, | ||
| Image, | ||
| ImageReference, | ||
| InlineCode, | ||
| Link, | ||
| LinkReference, | ||
| List, | ||
| ListItem, | ||
| Paragraph, | ||
| Root, | ||
| Strong, | ||
| Text, | ||
| ThematicBreak, | ||
| // Extensions (GFM) | ||
| Delete, | ||
| FootnoteDefinition, | ||
| FootnoteReference, | ||
| Table, | ||
| TableCell, | ||
| TableRow, | ||
| // Extensions (front matter) | ||
| Yaml, |
There was a problem hiding this comment.
Originally, there was consensus not to use the RootContent type.
Currently, the Nodes type is equivalent to Root | RootContent, so I'm not sure that using Nodes here is the desired approach.
I personally prefer not to use the Nodes type and to keep the original enumeration style.
Ref: #334 (comment)
3 participants
Prerequisites checklist
What is the purpose of this pull request?
This PR updates
MarkdownRuleVisitorso that the parent parameter has a precise type for non-root nodes.What changes did you make? (Give an overview)
ParentOf<T>utility to infer correct parent types.parentparameter is now precisely typed; for root, only thenodeis passed.MarkdownSyntaxElement, consistent with the CSS and JSON plugins.MarkdownSourceCodeto referenceMarkdownSyntaxElement.Related Issues
Is there anything you'd like reviewers to focus on?