msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

feat: allow comments in tags (#17671)

Browse Source 
Alternative to #17188. I prefer this syntax, it's lighter and feels much
more natural to me. For me it's less about commenting things _out_ than
about just, well... commenting — I frequently want to do this sort of
thing:

```svelte
<button
  // when the user clicks the button, the thing should happen
  onclick={doTheThing}
>click me</button>
```

One difference between this and #17188 is that this doesn't add a node
to the AST, just like comments in CSS/JS. Haven't decided if that's
desirable or not. I think it's more correct (it's an AST, not a CST;
HTML comments are different insofar as they _can_ represent 'real'
nodes) but it might be less convenient when (for example)
pretty-printing.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
pull/17747/head
Rich Harris 3 days ago committed by GitHub
parent 2661513cd3
commit 92e2fc1209
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 892 additions and 37 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/angry-suns-call.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': minor
---
feat: allow comments in tags

7
packages/svelte/src/compiler/legacy.js
Unescape View File

@ -101,7 +101,12 @@ export function convert(source, ast) {
},
instance,
module,
css: ast.css ? visit(ast.css) : undefined
css: ast.css ? visit(ast.css) : undefined,
// put it on _comments not comments because the latter is checked by prettier and then fails
// if we don't adjust stuff accordingly in our prettier plugin, and so it would be kind of an
// indirect breaking change for people updating their Svelte version but not their prettier plugin version.
// We can keep it as comments for the modern AST because the modern AST is not used in the plugin yet.
_comments: ast.comments?.length > 0 ? ast.comments : undefined
};
},
AnimateDirective(node) {

53
packages/svelte/src/compiler/phases/1-parse/state/element.js
Unescape View File

@ -506,6 +506,15 @@ function read_static_attribute(parser) {
* @returns {AST.Attribute | AST.SpreadAttribute | AST.Directive | AST.AttachTag | null}
*/
function read_attribute(parser) {
/** @type {AST.JSComment | null} */
// eslint-disable-next-line no-useless-assignment -- it is, in fact, eslint that is useless
let comment = null;
while ((comment = read_comment(parser))) {
parser.root.comments.push(comment);
parser.allow_whitespace();
}
const start = parser.index;
if (parser.eat('{')) {
@ -702,6 +711,50 @@ function read_attribute(parser) {
return create_attribute(tag.name, tag.loc, start, end, value);
}
/**
* @param {Parser} parser
* @returns {AST.JSComment | null}
*/
function read_comment(parser) {
const start = parser.index;
if (parser.eat('//')) {
const value = parser.read_until(/\n/);
const end = parser.index;
return {
type: 'Line',
start,
end,
value,
loc: {
start: locator(start),
end: locator(end)
}
};
}
if (parser.eat('/*')) {
const value = parser.read_until(/\*\//);
parser.eat('*/');
const end = parser.index;
return {
type: 'Block',
start,
end,
value,
loc: {
start: locator(start),
end: locator(end)
}
};
}
return null;
}
/**
* @param {string} name
* @returns {any}

105
packages/svelte/src/compiler/print/index.js
Unescape View File

@ -18,15 +18,17 @@ const LINE_BREAK_THRESHOLD = 50;
* @param {import('./types.js').Options | undefined} options
*/
export function print(ast, options = undefined) {
const comments = (ast.type === 'Root' && ast.comments) || [];
return esrap.print(
ast,
/** @type {Visitors<AST.SvelteNode>} */ ({
...ts({
comments: ast.type === 'Root' ? ast.comments : [],
comments,
getLeadingComments: options?.getLeadingComments,
getTrailingComments: options?.getTrailingComments
}),
...svelte_visitors,
...svelte_visitors(comments),
...css_visitors
})
);
@ -57,35 +59,72 @@ function block(context, node, allow_inline = false) {
}
/**
* @param {AST.BaseNode} node
* @param {AST.BaseElement['attributes']} attributes
* @param {Context} context
* @param {AST.JSComment[]} comments
* @returns {boolean} true if attributes were formatted on multiple lines
*/
function attributes(attributes, context) {
function attributes(node, attributes, context, comments) {
if (attributes.length === 0) {
return false;
}
// Measure total width of all attributes when rendered inline
const child_context = context.new();
let length = -1;
for (const attribute of attributes) {
child_context.write(' ');
child_context.visit(attribute);
let comment_index = comments.findIndex((comment) => comment.start > node.start);
if (comment_index === -1) {
comment_index = comments.length;
}
const multiline = child_context.measure() > LINE_BREAK_THRESHOLD;
const separator = context.new();
const children = attributes.map((attribute) => {
const child_context = context.new();
while (comment_index < comments.length) {
const comment = comments[comment_index];
if (comment.start < attribute.start) {
if (comment.type === 'Line') {
child_context.write('//' + comment.value);
child_context.newline();
} else {
child_context.write('/*' + comment.value + '*/'); // TODO match indentation?
child_context.append(separator);
}
comment_index += 1;
} else {
break;
}
}
child_context.visit(attribute);
length += child_context.measure() + 1;
return child_context;
});
let multiline = context.multiline || length > LINE_BREAK_THRESHOLD;
if (multiline) {
separator.newline();
context.indent();
for (const attribute of attributes) {
for (const child of children) {
context.newline();
context.visit(attribute);
context.append(child);
}
context.dedent();
context.newline();
} else {
context.append(child_context);
separator.write(' ');
for (const child of children) {
context.write(' ');
context.append(child);
}
}
return multiline;
@ -94,8 +133,9 @@ function attributes(attributes, context) {
/**
* @param {AST.BaseElement} node
* @param {Context} context
* @param {AST.JSComment[]} comments
*/
function base_element(node, context) {
function base_element(node, context, comments) {
const child_context = context.new();
child_context.write('<' + node.name);
@ -111,7 +151,7 @@ function base_element(node, context) {
child_context.write('}');
}
const multiline_attributes = attributes(node.attributes, child_context);
const multiline_attributes = attributes(node, node.attributes, child_context, comments);
const is_doctype_node = node.name.toLowerCase() === '!doctype';
const is_self_closing =
is_void(node.name) || (node.type === 'Component' && node.fragment.nodes.length === 0);
@ -284,8 +324,11 @@ const css_visitors = {
}
};
/** @type {Visitors<AST.SvelteNode>} */
const svelte_visitors = {
/**
* @param {AST.JSComment[]} comments
* @returns {Visitors<AST.SvelteNode>}
*/
const svelte_visitors = (comments) => ({
Root(node, context) {
if (node.options) {
context.write('<svelte:options');
@ -315,7 +358,7 @@ const svelte_visitors = {
Script(node, context) {
context.write('<script');
attributes(node.attributes, context);
attributes(node, node.attributes, context, comments);
context.write('>');
block(context, node.content);
context.write('</script>');
@ -545,7 +588,7 @@ const svelte_visitors = {
},
Component(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
ConstTag(node, context) {
@ -681,7 +724,7 @@ const svelte_visitors = {
},
RegularElement(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
RenderTag(node, context) {
@ -691,7 +734,7 @@ const svelte_visitors = {
},
SlotElement(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SnippetBlock(node, context) {
@ -747,7 +790,7 @@ const svelte_visitors = {
StyleSheet(node, context) {
context.write('<style');
attributes(node.attributes, context);
attributes(node, node.attributes, context, comments);
context.write('>');
if (node.children.length > 0) {
@ -774,7 +817,7 @@ const svelte_visitors = {
},
SvelteBoundary(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SvelteComponent(node, context) {
@ -783,7 +826,7 @@ const svelte_visitors = {
context.write(' this={');
context.visit(node.expression);
context.write('}');
attributes(node.attributes, context);
attributes(node, node.attributes, context, comments);
if (node.fragment && node.fragment.nodes.length > 0) {
context.write('>');
block(context, node.fragment, true);
@ -794,7 +837,7 @@ const svelte_visitors = {
},
SvelteDocument(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SvelteElement(node, context) {
@ -803,7 +846,7 @@ const svelte_visitors = {
context.write('this={');
context.visit(node.tag);
context.write('}');
attributes(node.attributes, context);
attributes(node, node.attributes, context, comments);
if (node.fragment && node.fragment.nodes.length > 0) {
context.write('>');
@ -815,19 +858,19 @@ const svelte_visitors = {
},
SvelteFragment(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SvelteHead(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SvelteSelf(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
SvelteWindow(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
Text(node, context) {
@ -835,7 +878,7 @@ const svelte_visitors = {
},
TitleElement(node, context) {
base_element(node, context);
base_element(node, context, comments);
},
TransitionDirective(node, context) {
@ -865,4 +908,4 @@ const svelte_visitors = {
context.write('}');
}
}
};
});

420
packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json
Unescape View File

@ -880,5 +880,423 @@
],
"sourceType": "module"
}
}
},
"_comments": [
{
"type": "Line",
"value": " a leading comment",
"start": 10,
"end": 30,
"loc": {
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 21
}
}
},
{
"type": "Line",
"value": " a trailing comment",
"start": 45,
"end": 66,
"loc": {
"start": {
"line": 3,
"column": 14
},
"end": {
"line": 3,
"column": 35
}
}
},
{
"type": "Block",
"value": "* a comment ",
"start": 83,
"end": 99,
"loc": {
"start": {
"line": 6,
"column": 1
},
"end": {
"line": 6,
"column": 17
}
}
},
{
"type": "Line",
"value": " trailing",
"start": 125,
"end": 136,
"loc": {
"start": {
"line": 8,
"column": 7
},
"end": {
"line": 8,
"column": 18
}
}
},
{
"type": "Block",
"value": " leading comment 1 ",
"start": 139,
"end": 162,
"loc": {
"start": {
"line": 9,
"column": 2
},
"end": {
"line": 9,
"column": 25
}
}
},
{
"type": "Block",
"value": " leading comment 2 ",
"start": 165,
"end": 188,
"loc": {
"start": {
"line": 10,
"column": 2
},
"end": {
"line": 10,
"column": 25
}
}
},
{
"type": "Block",
"value": " leading comment 3 ",
"start": 191,
"end": 214,
"loc": {
"start": {
"line": 11,
"column": 2
},
"end": {
"line": 11,
"column": 25
}
}
},
{
"type": "Block",
"value": " trailing comment 1 ",
"start": 224,
"end": 248,
"loc": {
"start": {
"line": 13,
"column": 2
},
"end": {
"line": 13,
"column": 26
}
}
},
{
"type": "Block",
"value": " trailing comment 2 ",
"start": 251,
"end": 275,
"loc": {
"start": {
"line": 14,
"column": 2
},
"end": {
"line": 14,
"column": 26
}
}
},
{
"type": "Block",
"value": " trailing comment 3 ",
"start": 278,
"end": 302,
"loc": {
"start": {
"line": 15,
"column": 2
},
"end": {
"line": 15,
"column": 26
}
}
},
{
"type": "Line",
"value": " leading comment 1",
"start": 326,
"end": 346,
"loc": {
"start": {
"line": 19,
"column": 2
},
"end": {
"line": 19,
"column": 22
}
}
},
{
"type": "Line",
"value": " leading comment 2",
"start": 349,
"end": 369,
"loc": {
"start": {
"line": 20,
"column": 2
},
"end": {
"line": 20,
"column": 22
}
}
},
{
"type": "Line",
"value": " trailing comment 1",
"start": 375,
"end": 396,
"loc": {
"start": {
"line": 21,
"column": 5
},
"end": {
"line": 21,
"column": 26
}
}
},
{
"type": "Block",
"value": " trailing comment 2 ",
"start": 399,
"end": 423,
"loc": {
"start": {
"line": 22,
"column": 2
},
"end": {
"line": 22,
"column": 26
}
}
},
{
"type": "Line",
"value": " leading comment 1",
"start": 449,
"end": 469,
"loc": {
"start": {
"line": 26,
"column": 2
},
"end": {
"line": 26,
"column": 22
}
}
},
{
"type": "Line",
"value": " leading comment 2",
"start": 472,
"end": 492,
"loc": {
"start": {
"line": 27,
"column": 2
},
"end": {
"line": 27,
"column": 22
}
}
},
{
"type": "Line",
"value": " trailing comment 1",
"start": 501,
"end": 522,
"loc": {
"start": {
"line": 28,
"column": 8
},
"end": {
"line": 28,
"column": 29
}
}
},
{
"type": "Block",
"value": " trailing comment 2 ",
"start": 525,
"end": 549,
"loc": {
"start": {
"line": 29,
"column": 2
},
"end": {
"line": 29,
"column": 26
}
}
},
{
"type": "Line",
"value": " comment",
"start": 584,
"end": 594,
"loc": {
"start": {
"line": 34,
"column": 11
},
"end": {
"line": 34,
"column": 21
}
}
},
{
"type": "Block",
"value": " another comment ",
"start": 606,
"end": 627,
"loc": {
"start": {
"line": 36,
"column": 2
},
"end": {
"line": 36,
"column": 23
}
}
},
{
"type": "Line",
"value": " a trailing comment",
"start": 636,
"end": 657,
"loc": {
"start": {
"line": 37,
"column": 8
},
"end": {
"line": 37,
"column": 29
}
}
},
{
"type": "Block",
"value": " trailing block comment ",
"start": 660,
"end": 688,
"loc": {
"start": {
"line": 38,
"column": 2
},
"end": {
"line": 38,
"column": 30
}
}
},
{
"type": "Block",
"value": " leading block comment ",
"start": 696,
"end": 723,
"loc": {
"start": {
"line": 41,
"column": 1
},
"end": {
"line": 41,
"column": 28
}
}
},
{
"type": "Line",
"value": " leading line comment",
"start": 739,
"end": 762,
"loc": {
"start": {
"line": 43,
"column": 2
},
"end": {
"line": 43,
"column": 25
}
}
},
{
"type": "Line",
"value": " trailing line comment",
"start": 770,
"end": 794,
"loc": {
"start": {
"line": 44,
"column": 7
},
"end": {
"line": 44,
"column": 31
}
}
},
{
"type": "Block",
"value": " trailing block comment ",
"start": 796,
"end": 824,
"loc": {
"start": {
"line": 45,
"column": 1
},
"end": {
"line": 45,
"column": 29
}
}
}
]
}

20
packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json
Unescape View File

@ -51,5 +51,23 @@
}
]
}
}
},
"_comments": [
{
"type": "Line",
"value": " TODO write some code",
"start": 10,
"end": 33,
"loc": {
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 24
}
}
}
]
}

22
packages/svelte/tests/parser-modern/samples/comment-in-tag/input.svelte
Unescape View File

@ -0,0 +1,22 @@
<div
data-one="1"
// this is a line comment
data-two="2"
/* this is a
m
u
l
t
i
l
i
n
e
comment
*/
// oh look another line comment
// (two, in fact)
data-three="3"
></div>
<span /* inline */ /* another inline */ data-one="1"></span>

286
packages/svelte/tests/parser-modern/samples/comment-in-tag/output.json
Unescape View File

@ -0,0 +1,286 @@
{
"css": null,
"js": [],
"start": 0,
"end": 261,
"type": "Root",
"fragment": {
"type": "Fragment",
"nodes": [
{
"type": "RegularElement",
"start": 0,
"end": 199,
"name": "div",
"name_loc": {
"start": {
"line": 1,
"column": 1,
"character": 1
},
"end": {
"line": 1,
"column": 4,
"character": 4
}
},
"attributes": [
{
"type": "Attribute",
"start": 6,
"end": 18,
"name": "data-one",
"name_loc": {
"start": {
"line": 2,
"column": 1,
"character": 6
},
"end": {
"line": 2,
"column": 9,
"character": 14
}
},
"value": [
{
"start": 16,
"end": 17,
"type": "Text",
"raw": "1",
"data": "1"
}
]
},
{
"type": "Attribute",
"start": 47,
"end": 59,
"name": "data-two",
"name_loc": {
"start": {
"line": 4,
"column": 1,
"character": 47
},
"end": {
"line": 4,
"column": 9,
"character": 55
}
},
"value": [
{
"start": 57,
"end": 58,
"type": "Text",
"raw": "2",
"data": "2"
}
]
},
{
"type": "Attribute",
"start": 177,
"end": 191,
"name": "data-three",
"name_loc": {
"start": {
"line": 19,
"column": 1,
"character": 177
},
"end": {
"line": 19,
"column": 11,
"character": 187
}
},
"value": [
{
"start": 189,
"end": 190,
"type": "Text",
"raw": "3",
"data": "3"
}
]
}
],
"fragment": {
"type": "Fragment",
"nodes": []
}
},
{
"type": "Text",
"start": 199,
"end": 201,
"raw": "\n\n",
"data": "\n\n"
},
{
"type": "RegularElement",
"start": 201,
"end": 261,
"name": "span",
"name_loc": {
"start": {
"line": 22,
"column": 1,
"character": 202
},
"end": {
"line": 22,
"column": 5,
"character": 206
}
},
"attributes": [
{
"type": "Attribute",
"start": 241,
"end": 253,
"name": "data-one",
"name_loc": {
"start": {
"line": 22,
"column": 40,
"character": 241
},
"end": {
"line": 22,
"column": 48,
"character": 249
}
},
"value": [
{
"start": 251,
"end": 252,
"type": "Text",
"raw": "1",
"data": "1"
}
]
}
],
"fragment": {
"type": "Fragment",
"nodes": []
}
}
]
},
"options": null,
"comments": [
{
"type": "Line",
"start": 20,
"end": 45,
"value": " this is a line comment",
"loc": {
"start": {
"line": 3,
"column": 1,
"character": 20
},
"end": {
"line": 3,
"column": 26,
"character": 45
}
}
},
{
"type": "Block",
"start": 61,
"end": 123,
"value": " this is a\n\t\tm\n\t\tu\n\t\tl\n\t\tt\n\t\ti\n\t\tl\n\t\ti\n\t\tn\n\t\te\n\t\tcomment\n\t",
"loc": {
"start": {
"line": 5,
"column": 1,
"character": 61
},
"end": {
"line": 16,
"column": 3,
"character": 123
}
}
},
{
"type": "Line",
"start": 125,
"end": 156,
"value": " oh look another line comment",
"loc": {
"start": {
"line": 17,
"column": 1,
"character": 125
},
"end": {
"line": 17,
"column": 32,
"character": 156
}
}
},
{
"type": "Line",
"start": 158,
"end": 175,
"value": " (two, in fact)",
"loc": {
"start": {
"line": 18,
"column": 1,
"character": 158
},
"end": {
"line": 18,
"column": 18,
"character": 175
}
}
},
{
"type": "Block",
"start": 207,
"end": 219,
"value": " inline ",
"loc": {
"start": {
"line": 22,
"column": 6,
"character": 207
},
"end": {
"line": 22,
"column": 18,
"character": 219
}
}
},
{
"type": "Block",
"start": 220,
"end": 240,
"value": " another inline ",
"loc": {
"start": {
"line": 22,
"column": 19,
"character": 220
},
"end": {
"line": 22,
"column": 39,
"character": 240
}
}
}
]
}

11
packages/svelte/tests/parser-modern/test.ts
Unescape View File

@ -25,8 +25,6 @@ const { test, run } = suite<ParserTest>(async (config, cwd) => {
)
);
delete actual.comments;
// run `UPDATE_SNAPSHOTS=true pnpm test parser` to update parser tests
if (process.env.UPDATE_SNAPSHOTS) {
fs.writeFileSync(`${cwd}/output.json`, JSON.stringify(actual, null, '\t') + '\n');
@ -34,6 +32,11 @@ const { test, run } = suite<ParserTest>(async (config, cwd) => {
fs.writeFileSync(`${cwd}/_actual.json`, JSON.stringify(actual, null, '\t'));
const expected = try_load_json(`${cwd}/output.json`);
if (!expected.comments) {
delete actual.comments;
}
assert.deepEqual(actual, expected);
}
@ -50,7 +53,9 @@ const { test, run } = suite<ParserTest>(async (config, cwd) => {
fs.writeFileSync(`${cwd}/_actual.svelte`, printed.code);
delete reparsed.comments;
if (!actual.comments) {
delete reparsed.comments;
}
assert.deepEqual(clean(actual), clean(reparsed));
}

Write Preview
Loading…
Cancel
Save