feat(compiler-core): add openTagLoc and closeTagLoc to element ast node
#12928
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
edison1105
added
ready to merge
WalkthroughThe changes introduce new options and properties to enable and handle precise source location tracking for opening and closing tags in the parser's AST nodes. Updates include AST type extensions, parser logic to populate these locations when enabled, parser options to control the feature, and corresponding test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant SourceCode
participant Parser
participant ASTNode
SourceCode->>Parser: Provide HTML/XML source
Parser->>Parser: Check if tagLocations option is enabled
alt tagLocations enabled
Parser->>ASTNode: Attach openTagLoc during open tag parse
Parser->>ASTNode: Attach closeTagLoc during close tag parse
end
Parser->>ASTNode: Build AST with tag location info (if enabled)
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
KazariEX
changed the title
tagLoc and closeTagLoc to element ast nodeopenTagLoc and closeTagLoc to element ast node
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/compiler-core/src/options.ts (1)
100-104: Minor doc tweakThe comment says “open tag and close tag of each node” – consider clarifying that it records the tag-name locations, not the whole tag range, to avoid ambiguity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/compiler-core/src/ast.ts(1 hunks)packages/compiler-core/src/options.ts(1 hunks)packages/compiler-core/src/parser.ts(3 hunks)packages/compiler-dom/__tests__/parse.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test / unit-test-windows
🔇 Additional comments (1)
packages/compiler-core/src/parser.ts (1)
81-82: Performance toggle added – ensure downstream opts in
tagLocationsdefaults tofalse, yet the DOM-parser tests rely on it beingtrue.
IfparserOptionsinpackages/compiler-dom/src/parserOptions.tsis not explicitly enabling this flag, the new fields will never be set and tests will still fail even after the naming fix.
Please verify & enable where needed.
| openTagLoc?: SourceLocation | ||
| closeTagLoc?: SourceLocation | ||
| tagType: ElementTypes |
There was a problem hiding this comment.
Mismatch with test expectations – property should be tagLoc or an alias must be added
The interface now exposes openTagLoc / closeTagLoc, but the updated unit-test asserts against a tagLoc field.
Unless an alias is added or the tests are updated, the type checker and the runtime equality check (toStrictEqual) will fail.
tag: string
- openTagLoc?: SourceLocation
- closeTagLoc?: SourceLocation
+ // location of `<tag>` name
+ openTagLoc?: SourceLocation
+ // location of `</tag>` name
+ closeTagLoc?: SourceLocation
+ /** @deprecated – kept for backwards-compatibility with existing tooling/tests */
+ tagLoc?: SourceLocationAlternatively, rename the tests to use openTagLoc. Keep one consistent public field, or expose an alias – but make it deliberate.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| openTagLoc?: SourceLocation | |
| closeTagLoc?: SourceLocation | |
| tagType: ElementTypes | |
| tag: string | |
| // location of `<tag>` name | |
| openTagLoc?: SourceLocation | |
| // location of `</tag>` name | |
| closeTagLoc?: SourceLocation | |
| /** @deprecated – kept for backwards-compatibility with existing tooling/tests */ | |
| tagLoc?: SourceLocation | |
| tagType: ElementTypes |
🤖 Prompt for AI Agents
In packages/compiler-core/src/ast.ts around lines 133 to 135, the interface uses
separate properties openTagLoc and closeTagLoc, but the unit tests expect a
single tagLoc property. To fix this, either rename the interface property to
tagLoc to match the tests, add a tagLoc alias that combines or references
openTagLoc, or update the tests to use openTagLoc consistently. Ensure the
property naming is consistent between the interface and tests to avoid type
checker and runtime equality failures.
| if (currentOptions.tagLocations) { | ||
| currentOpenTag.openTagLoc = getLoc(start, end) | ||
| } |
There was a problem hiding this comment.
Set legacy alias for forward compatibility
When the option is active you attach openTagLoc, but not tagLoc.
If you decide to keep the test’s tagLoc, add the alias at the same time:
if (currentOptions.tagLocations) {
currentOpenTag.openTagLoc = getLoc(start, end)
+ // backwards compat
+ ;(currentOpenTag as any).tagLoc = currentOpenTag.openTagLoc
}This avoids breaking userland tools that already consume tagLoc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (currentOptions.tagLocations) { | |
| currentOpenTag.openTagLoc = getLoc(start, end) | |
| } | |
| if (currentOptions.tagLocations) { | |
| currentOpenTag.openTagLoc = getLoc(start, end) | |
| // backwards compat | |
| ;(currentOpenTag as any).tagLoc = currentOpenTag.openTagLoc | |
| } |
🤖 Prompt for AI Agents
In packages/compiler-core/src/parser.ts around lines 150 to 152, the code sets
currentOpenTag.openTagLoc when currentOptions.tagLocations is true but does not
set the legacy alias currentOpenTag.tagLoc. To fix this, add an assignment
setting currentOpenTag.tagLoc equal to currentOpenTag.openTagLoc within the same
conditional block to maintain backward compatibility with userland tools relying
on tagLoc.
| const el = stack.shift()! | ||
| if (j === i && currentOptions.tagLocations) { | ||
| el.closeTagLoc = getLoc(start, end) | ||
| } |
There was a problem hiding this comment.
Same alias needed on close tag
if (j === i && currentOptions.tagLocations) {
el.closeTagLoc = getLoc(start, end)
+ ;(el as any).tagLoc = el.openTagLoc // keep alias symmetrical
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const el = stack.shift()! | |
| if (j === i && currentOptions.tagLocations) { | |
| el.closeTagLoc = getLoc(start, end) | |
| } | |
| const el = stack.shift()! | |
| if (j === i && currentOptions.tagLocations) { | |
| el.closeTagLoc = getLoc(start, end) | |
| ;(el as any).tagLoc = el.openTagLoc // keep alias symmetrical | |
| } |
🤖 Prompt for AI Agents
In packages/compiler-core/src/parser.ts around lines 171 to 174, the close tag
element lacks the same alias as the opening tag. To fix this, ensure that the
close tag element uses the same alias as the corresponding opening tag by
assigning the alias property from the opening tag to the close tag element
within the conditional block where closeTagLoc is set.
| tagLoc: { | ||
| start: { offset: 1, line: 1, column: 2 }, | ||
| end: { offset: 4, line: 1, column: 5 }, | ||
| source: 'img', | ||
| }, |
There was a problem hiding this comment.
Assertion uses tagLoc which parser no longer emits
With the AST now exposing openTagLoc/closeTagLoc, the expectation below will fail:
- tagLoc: {
+ openTagLoc: {
...
},Either update the test or keep a tagLoc alias in the AST node (see previous comments).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tagLoc: { | |
| start: { offset: 1, line: 1, column: 2 }, | |
| end: { offset: 4, line: 1, column: 5 }, | |
| source: 'img', | |
| }, | |
| openTagLoc: { | |
| start: { offset: 1, line: 1, column: 2 }, | |
| end: { offset: 4, line: 1, column: 5 }, | |
| source: 'img', | |
| }, |
🤖 Prompt for AI Agents
In packages/compiler-dom/__tests__/parse.spec.ts around lines 312 to 316, the
test is using the outdated `tagLoc` property which the parser no longer emits,
having replaced it with `openTagLoc` and `closeTagLoc`. To fix this, update the
test assertions to use `openTagLoc` and `closeTagLoc` properties instead of
`tagLoc`, reflecting the current AST structure. Alternatively, if preferred,
modify the AST node to include a `tagLoc` alias that combines `openTagLoc` and
`closeTagLoc` as per previous comments.
Nowadays, language tools rely on string search to determine the offset of tags, which is not reliable and may cause some issues, like:
Summary by CodeRabbit
New Features
Tests