Skip to content

Commit 18a6750

Browse files
crisbetoAndrewKushnir
authored andcommitted
fix(compiler): more permissive parsing of @ characters (#62644)
When we introduced blocks, we made a deliberate decision to treat the `@` character as a reserved character in case we need to use it for other syntax in the future. This meant that some common cases, like writing out an email address in the template, can be broken. After some recent discussions we decided to relax the requirement and only treat `@` as a reserve character if it's followed by a character sequence that matches a known block. PR Close #62644
1 parent 56fbb32 commit 18a6750

File tree

5 files changed

+169
-154
lines changed

5 files changed

+169
-154
lines changed

packages/compiler/src/ml_parser/lexer.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,20 @@ enum CharacterReferenceType {
144144
DEC = 'decimal',
145145
}
146146

147+
const SUPPORTED_BLOCKS = [
148+
'@if',
149+
'@else', // Covers `@else if` as well
150+
'@for',
151+
'@switch',
152+
'@case',
153+
'@default',
154+
'@empty',
155+
'@defer',
156+
'@placeholder',
157+
'@loading',
158+
'@error',
159+
];
160+
147161
// See https://www.w3.org/TR/html51/syntax.html#writing-html-documents
148162
class _Tokenizer {
149163
private _cursor: CharacterCursor;
@@ -234,10 +248,10 @@ class _Tokenizer {
234248
// don't want to advance in case it's not `@let`.
235249
this._cursor.peek() === chars.$AT &&
236250
!this._inInterpolation &&
237-
this._attemptStr('@let')
251+
this._isLetStart()
238252
) {
239253
this._consumeLetDeclaration(start);
240-
} else if (this._tokenizeBlocks && this._attemptCharCode(chars.$AT)) {
254+
} else if (this._tokenizeBlocks && this._isBlockStart()) {
241255
this._consumeBlockStart(start);
242256
} else if (
243257
this._tokenizeBlocks &&
@@ -284,6 +298,7 @@ class _Tokenizer {
284298
}
285299

286300
private _consumeBlockStart(start: CharacterCursor) {
301+
this._requireCharCode(chars.$AT);
287302
this._beginToken(TokenType.BLOCK_OPEN_START, start);
288303
const startToken = this._endToken([this._getBlockName()]);
289304

@@ -363,6 +378,7 @@ class _Tokenizer {
363378
}
364379

365380
private _consumeLetDeclaration(start: CharacterCursor) {
381+
this._requireStr('@let');
366382
this._beginToken(TokenType.LET_START, start);
367383

368384
// Require at least one white space after the `@let`.
@@ -628,6 +644,32 @@ class _Tokenizer {
628644
return char;
629645
}
630646

647+
private _peekStr(chars: string): boolean {
648+
const len = chars.length;
649+
if (this._cursor.charsLeft() < len) {
650+
return false;
651+
}
652+
const cursor = this._cursor.clone();
653+
for (let i = 0; i < len; i++) {
654+
if (cursor.peek() !== chars.charCodeAt(i)) {
655+
return false;
656+
}
657+
cursor.advance();
658+
}
659+
return true;
660+
}
661+
662+
private _isBlockStart(): boolean {
663+
return (
664+
this._cursor.peek() === chars.$AT &&
665+
SUPPORTED_BLOCKS.some((blockName) => this._peekStr(blockName))
666+
);
667+
}
668+
669+
private _isLetStart(): boolean {
670+
return this._cursor.peek() === chars.$AT && this._peekStr('@let');
671+
}
672+
631673
private _consumeEntity(textTokenType: TokenType): void {
632674
this._beginToken(TokenType.ENCODED_ENTITY);
633675
const start = this._cursor.clone();
@@ -1261,7 +1303,7 @@ class _Tokenizer {
12611303
this._tokenizeBlocks &&
12621304
!this._inInterpolation &&
12631305
!this._isInExpansion() &&
1264-
(this._cursor.peek() === chars.$AT || this._cursor.peek() === chars.$RBRACE)
1306+
(this._isBlockStart() || this._isLetStart() || this._cursor.peek() === chars.$RBRACE)
12651307
) {
12661308
return true;
12671309
}

packages/compiler/test/ml_parser/html_parser_spec.ts

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -798,8 +798,8 @@ describe('HtmlParser', () => {
798798

799799
describe('blocks', () => {
800800
it('should parse a block', () => {
801-
expect(humanizeDom(parser.parse('@foo (a b; c d){hello}', 'TestComp'))).toEqual([
802-
[html.Block, 'foo', 0],
801+
expect(humanizeDom(parser.parse('@defer (a b; c d){hello}', 'TestComp'))).toEqual([
802+
[html.Block, 'defer', 0],
803803
[html.BlockParameter, 'a b'],
804804
[html.BlockParameter, 'c d'],
805805
[html.Text, 'hello', 1, ['hello']],
@@ -852,49 +852,54 @@ describe('HtmlParser', () => {
852852
it('should parse nested blocks', () => {
853853
const markup =
854854
`<root-sibling-one/>` +
855-
`@root {` +
855+
`@if (root) {` +
856856
`<outer-child-one/>` +
857857
`<outer-child-two>` +
858-
`@child (childParam === 1) {` +
859-
`@innerChild (innerChild1 === foo) {` +
858+
`@if (childParam === 1) {` +
859+
`@if (innerChild1 === foo) {` +
860860
`<inner-child-one/>` +
861-
`@grandChild {` +
862-
`@innerGrandChild {` +
861+
`@switch (grandChild) {` +
862+
`@case (innerGrandChild) {` +
863863
`<inner-grand-child-one/>` +
864864
`}` +
865-
`@innerGrandChild {` +
865+
`@case (innerGrandChild) {` +
866866
`<inner-grand-child-two/>` +
867867
`}` +
868868
`}` +
869869
`}` +
870-
`@innerChild {` +
870+
`@if (innerChild) {` +
871871
`<inner-child-two/>` +
872872
`}` +
873873
`}` +
874874
`</outer-child-two>` +
875-
`@outerChild (outerChild1; outerChild2) {` +
875+
`@for (outerChild1; outerChild2) {` +
876876
`<outer-child-three/>` +
877877
`}` +
878878
`} <root-sibling-two/>`;
879879

880880
expect(humanizeDom(parser.parse(markup, 'TestComp'))).toEqual([
881881
[html.Element, 'root-sibling-one', 0, '#selfClosing'],
882-
[html.Block, 'root', 0],
882+
[html.Block, 'if', 0],
883+
[html.BlockParameter, 'root'],
883884
[html.Element, 'outer-child-one', 1, '#selfClosing'],
884885
[html.Element, 'outer-child-two', 1],
885-
[html.Block, 'child', 2],
886+
[html.Block, 'if', 2],
886887
[html.BlockParameter, 'childParam === 1'],
887-
[html.Block, 'innerChild', 3],
888+
[html.Block, 'if', 3],
888889
[html.BlockParameter, 'innerChild1 === foo'],
889890
[html.Element, 'inner-child-one', 4, '#selfClosing'],
890-
[html.Block, 'grandChild', 4],
891-
[html.Block, 'innerGrandChild', 5],
891+
[html.Block, 'switch', 4],
892+
[html.BlockParameter, 'grandChild'],
893+
[html.Block, 'case', 5],
894+
[html.BlockParameter, 'innerGrandChild'],
892895
[html.Element, 'inner-grand-child-one', 6, '#selfClosing'],
893-
[html.Block, 'innerGrandChild', 5],
896+
[html.Block, 'case', 5],
897+
[html.BlockParameter, 'innerGrandChild'],
894898
[html.Element, 'inner-grand-child-two', 6, '#selfClosing'],
895-
[html.Block, 'innerChild', 3],
899+
[html.Block, 'if', 3],
900+
[html.BlockParameter, 'innerChild'],
896901
[html.Element, 'inner-child-two', 4, '#selfClosing'],
897-
[html.Block, 'outerChild', 1],
902+
[html.Block, 'for', 1],
898903
[html.BlockParameter, 'outerChild1'],
899904
[html.BlockParameter, 'outerChild2'],
900905
[html.Element, 'outer-child-three', 2, '#selfClosing'],
@@ -913,28 +918,30 @@ describe('HtmlParser', () => {
913918
});
914919

915920
it('should parse an empty block', () => {
916-
expect(humanizeDom(parser.parse('@foo{}', 'TestComp'))).toEqual([[html.Block, 'foo', 0]]);
921+
expect(humanizeDom(parser.parse('@defer{}', 'TestComp'))).toEqual([
922+
[html.Block, 'defer', 0],
923+
]);
917924
});
918925

919926
it('should parse a block with void elements', () => {
920-
expect(humanizeDom(parser.parse('@foo {<br>}', 'TestComp'))).toEqual([
921-
[html.Block, 'foo', 0],
927+
expect(humanizeDom(parser.parse('@defer {<br>}', 'TestComp'))).toEqual([
928+
[html.Block, 'defer', 0],
922929
[html.Element, 'br', 1],
923930
]);
924931
});
925932

926933
it('should close void elements used right before a block', () => {
927-
expect(humanizeDom(parser.parse('<img>@foo {hello}', 'TestComp'))).toEqual([
934+
expect(humanizeDom(parser.parse('<img>@defer {hello}', 'TestComp'))).toEqual([
928935
[html.Element, 'img', 0],
929-
[html.Block, 'foo', 0],
936+
[html.Block, 'defer', 0],
930937
[html.Text, 'hello', 1, ['hello']],
931938
]);
932939
});
933940

934941
it('should report an unclosed block', () => {
935-
const errors = parser.parse('@foo {hello', 'TestComp').errors;
942+
const errors = parser.parse('@defer {hello', 'TestComp').errors;
936943
expect(errors.length).toEqual(1);
937-
expect(humanizeErrors(errors)).toEqual([['foo', 'Unclosed block "foo"', '0:0']]);
944+
expect(humanizeErrors(errors)).toEqual([['defer', 'Unclosed block "defer"', '0:0']]);
938945
});
939946

940947
it('should report an unexpected block close', () => {
@@ -950,13 +957,13 @@ describe('HtmlParser', () => {
950957
});
951958

952959
it('should report unclosed tags inside of a block', () => {
953-
const errors = parser.parse('@foo {<strong>hello}', 'TestComp').errors;
960+
const errors = parser.parse('@defer {<strong>hello}', 'TestComp').errors;
954961
expect(errors.length).toEqual(1);
955962
expect(humanizeErrors(errors)).toEqual([
956963
[
957964
null,
958965
'Unexpected closing block. The block may have been closed earlier. If you meant to write the } character, you should use the "&#125;" HTML entity instead.',
959-
'0:19',
966+
'0:21',
960967
],
961968
]);
962969
});
@@ -1020,38 +1027,41 @@ describe('HtmlParser', () => {
10201027
});
10211028

10221029
it('should parse an incomplete block with no parameters', () => {
1023-
const result = parser.parse('Use the @Input() decorator', 'TestComp');
1030+
const result = parser.parse('This is the @if() block', 'TestComp');
10241031

10251032
expect(humanizeNodes(result.rootNodes, true)).toEqual([
1026-
[html.Text, 'Use the ', 0, ['Use the '], 'Use the '],
1027-
[html.Block, 'Input', 0, '@Input() ', '@Input() ', null],
1028-
[html.Text, 'decorator', 0, ['decorator'], 'decorator'],
1033+
[html.Text, 'This is the ', 0, ['This is the '], 'This is the '],
1034+
[html.Block, 'if', 0, '@if() ', '@if() ', null],
1035+
[html.Text, 'block', 0, ['block'], 'block'],
10291036
]);
10301037

10311038
expect(humanizeErrors(result.errors)).toEqual([
10321039
[
1033-
'Input',
1034-
'Incomplete block "Input". If you meant to write the @ character, you should use the "&#64;" HTML entity instead.',
1035-
'0:8',
1040+
'if',
1041+
'Incomplete block "if". If you meant to write the @ character, you should use the "&#64;" HTML entity instead.',
1042+
'0:12',
10361043
],
10371044
]);
10381045
});
10391046

10401047
it('should parse an incomplete block with no parameters', () => {
1041-
const result = parser.parse('Use @Input({alias: "foo"}) to alias your input', 'TestComp');
1048+
const result = parser.parse(
1049+
'This is the @if({alias: "foo"}) block with params',
1050+
'TestComp',
1051+
);
10421052

10431053
expect(humanizeNodes(result.rootNodes, true)).toEqual([
1044-
[html.Text, 'Use ', 0, ['Use '], 'Use '],
1045-
[html.Block, 'Input', 0, '@Input({alias: "foo"}) ', '@Input({alias: "foo"}) ', null],
1054+
[html.Text, 'This is the ', 0, ['This is the '], 'This is the '],
1055+
[html.Block, 'if', 0, '@if({alias: "foo"}) ', '@if({alias: "foo"}) ', null],
10461056
[html.BlockParameter, '{alias: "foo"}', '{alias: "foo"}'],
1047-
[html.Text, 'to alias your input', 0, ['to alias your input'], 'to alias your input'],
1057+
[html.Text, 'block with params', 0, ['block with params'], 'block with params'],
10481058
]);
10491059

10501060
expect(humanizeErrors(result.errors)).toEqual([
10511061
[
1052-
'Input',
1053-
'Incomplete block "Input". If you meant to write the @ character, you should use the "&#64;" HTML entity instead.',
1054-
'0:4',
1062+
'if',
1063+
'Incomplete block "if". If you meant to write the @ character, you should use the "&#64;" HTML entity instead.',
1064+
'0:12',
10551065
],
10561066
]);
10571067
});
@@ -1066,10 +1076,11 @@ describe('HtmlParser', () => {
10661076

10671077
it('should parse a let declaration that is nested in a parent', () => {
10681078
expect(
1069-
humanizeDom(parser.parse('@grandparent {@parent {@let foo = 123;}}', 'TestCmp')),
1079+
humanizeDom(parser.parse('@defer {@if (true) {@let foo = 123;}}', 'TestCmp')),
10701080
).toEqual([
1071-
[html.Block, 'grandparent', 0],
1072-
[html.Block, 'parent', 1],
1081+
[html.Block, 'defer', 0],
1082+
[html.Block, 'if', 1],
1083+
[html.BlockParameter, 'true'],
10731084
[html.LetDeclaration, 'foo', '123'],
10741085
]);
10751086
});

0 commit comments

Comments
 (0)