Skip to content

Inconsistency in quoted and mustache attribute value AST #8647

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

Closed
kwangure opened this issue May 27, 2023 · 3 comments
Closed

Inconsistency in quoted and mustache attribute value AST #8647

kwangure opened this issue May 27, 2023 · 3 comments
Milestone

Comments

@kwangure
Copy link
Contributor

Describe the bug

Given the following element:

<div class='a' value={a}/>

Svelte outputs the following in the AST.
For 'a':

value: [{
	start: 12,
	end: 13,
	type: "Text",
	...
}]

For {a}:

value: [{
	start: 21,
	end: 24,
	type: "MustacheTag",
	...
}]

Note how according the AST one attribute value is one character long and other is three characters long. They should both be three characters long, where for 'a', the quotes are included, resulting in:

value: [{
	start: 11,
	end: 14,
	type: "Text",
	...
}]

This aligns with how other parsers like Acorn JSX and Babel parse attribute values too.

Reproduction

Compare Svelte, Acorn JSX, Babel JSX and friends here. https://astexplorer.net/#/gist/43f92f399852b126ac6d21755f65cd6f/317b9a09a9af33f93fbbbed822d59b0429d852d4

Logs

No response

System Info

N/A

Severity

annoyance

@kwangure kwangure changed the title Inconsistency in value and mustache attribute value AST Inconsistency in quoted and mustache attribute value AST May 27, 2023
@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 3, 2024
@dummdidumm
Copy link
Member

Before merging a related PR we should check if prettier, eslint and language-tools don't rely on the behavior in some way, or work around it (and fixing it breaking the workaround). Maybe it would be safer to only do this for the modern AST, if we can reliably detect these cases to undo the fixes for the legacy AST.

@dummdidumm
Copy link
Member

This would be a pretty substantial AST change for little practical benefit. I don't think we should change this.

@Rich-Harris
Copy link
Member

Recapping a discussion from today: the current behaviour is correct, so closing this as not planned. A non-boolean attribute is a sequence of one or more Text or ExpressionTag chunks, and that sequence may or may not be wrapped in quotes (though we intend to be stricter in future about what must and must not be quoted — it's weird that x={y}{z} is permitted, and conversely per #7925 it's confusing that x="{y}" doesn't stringify y).

So in the example above it's natural that the surrounding quotes aren't included in the Text chunk. Put differently, it would be very strange indeed if in a case like x="a{b}c{d}e" the start of the a and the end of the e were adjusted to include the opening and closing quote marks, giving them both a length of 2, but the c had a length of 1.

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants