Skip to content

feat(@textlint/ast-node-types): add missing LinkReference, ImageReference and Definition node types #1459

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

Conversation

lumirlumir
Copy link
Contributor

@lumirlumir lumirlumir commented Mar 4, 2025

@azu Hello,

Thank you for your hard work and contributions to open source!

In this PR, I've added LinkReference, ImageReference, and Definition node types to @textlint/ast-node-types.

Rationale

Please take a look at the AST Tree of this example.

You'll notice that the AST includes LinkReference, ImageReference, and Definition nodes, but they are missing from @textlint/ast-node-types.

  1. LinkReference
  • line: 29, 35, 41

image

image

image

  1. ImageReference
  • line: 57, 63, 69

image

image

image

  1. Definition
  • line: 43, 71

image

image

Since these node types exist in the AST but are missing from @textlint/ast-node-types, I believe adding them will improve consistency and support for processing these structures.

Looking forward to your feedback!

@lumirlumir lumirlumir changed the title feat(@textlint/ast-node-types): add LinkReference, ImageReference and Definition Node types feat(@textlint/ast-node-types): add LinkReference, ImageReference and Definition node types Mar 4, 2025
@lumirlumir lumirlumir changed the title feat(@textlint/ast-node-types): add LinkReference, ImageReference and Definition node types feat(@textlint/ast-node-types): add missing LinkReference, ImageReference and Definition node types Mar 4, 2025
@azu azu requested a review from Copilot March 7, 2025 00:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR adds support for the missing AST node types—LinkReference, ImageReference, and Definition—to ensure that @textlint/ast-node-types fully reflects the nodes produced by the AST.

  • Updated TypeofTxtNode.ts to include mappings for LinkReference, ImageReference, and Definition nodes (with their respective "exit" types).
  • Extended NodeType.ts and ASTNodeTypes.ts with new interface definitions and enum values.
  • Updated test files and documentation to cover the new node types.

Reviewed Changes

File Description
packages/@textlint/ast-node-types/src/TypeofTxtNode.ts Adds node type mappings for LinkReference, ImageReference, and Definition nodes
packages/@textlint/ast-node-types/src/NodeType.ts Introduces new interfaces for TxtLinkReferenceNode, TxtImageReferenceNode, and TxtDefinitionNode, including the TxtReference mixin
packages/@textlint/ast-node-types/src/ASTNodeTypes.ts Updates enum values to include the new node types
packages/@textlint/types/test/Rule/TxtNode-test.ts Adds test cases to validate the type mappings for the new node types
packages/@textlint/ast-node-types/src/index.ts Exports the newly added node types
docs/txtnode.md Updates documentation to reflect the newly supported AST node types

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

@azu
Copy link
Member

azu commented Mar 7, 2025

I didn't include LinkReference, ImageReference, etc. because they are unique features of Markdown.
For example, many languages do not have any support for these Nodes when creating a non-Markdown parser.
However, I am beginning to feel that there is no problem even if there is.

I'll have to think a bit more about whether it would be a breaking change. If it is, we might want to bring it out with the next major version.

@azu azu added the Type: Feature New Feature label Mar 7, 2025
@lumirlumir
Copy link
Contributor Author

However, I am beginning to feel that there is no problem even if there is.

@azu, thanks for your response!

Actually, I’ve developed a rule called textlint-rule-allowed-uris, and I definitely need the LinkReference, ImageReference, and Definition types for proper type referencing. Currently, my refactoring process is stuck because there aren’t any relevant types available.

I'll have to think a bit more about whether it would be a breaking change.

Regarding your comment, I believe that adding more types won’t cause any breaking changes. Since these types weren’t implemented in earlier versions of Textlint, adding them now won’t break existing builds—it’s simply a feature request. Moreover, the README states that adding a new type is considered a minor change.

image

Additionally, Textlint is already able to access LinkReference and other nodes, as shown in the screenshot below:

image

This means that previous users who have used these nodes won’t experience any build-breaking issues.


I really hope this feature will be included in the next minor update! If you have any other opinions, please feel free to let me know!

…ence-imagereference-and-definition-node-types
@azu
Copy link
Member

azu commented Mar 11, 2025

OK, Thanks for details.

breaking change

I was thinking there might be an implementation like Object.keys(ASTNodeTypes), but since there is only a reference in the test code I felt it was ok.

https://github.com/search?q=Object.keys%28ASTNodeTypes%29&type=code&ref=simplesearch

Copy link
Member

@azu azu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@azu azu merged commit 5e724d6 into textlint:master Mar 11, 2025
10 checks passed
@github-actions github-actions bot mentioned this pull request Mar 11, 2025
@azu
Copy link
Member

azu commented Mar 12, 2025

@lumirlumir lumirlumir deleted the feat-textlint-ast-node-types-add-linkreference-imagereference-and-definition-node-types branch March 12, 2025 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants