refactor(compiler): don't allow as expressions in else if blocks (#51398)

Based on some discussions, these changes remove the ability to have an `as` expression on an `else if` block.

PR Close #51398
This commit is contained in:
Kristiyan Kostadinov
2023-08-17 09:31:48 +02:00
committed by Andrew Kushnir
parent 362e069364
commit eb1faa8f87
3 changed files with 16 additions and 17 deletions
@@ -47,9 +47,7 @@ export function createIfBlock(
continue;
}
// Expressions for `{:else if}` blocks start at 2 to skip the `if` from the expression.
const expressionStart = block.name === 'if' ? 0 : 2;
const params = parseConditionalBlockParameters(block, errors, bindingParser, expressionStart);
const params = parseConditionalBlockParameters(block, errors, bindingParser);
if (params !== null) {
branches.push(new t.IfBlockBranch(
@@ -284,15 +282,16 @@ function parseBlockParameterToBinding(
/** Parses the parameter of a conditional block (`if` or `else if`). */
function parseConditionalBlockParameters(
block: html.Block, errors: ParseError[], bindingParser: BindingParser,
primaryExpressionStart: number) {
block: html.Block, errors: ParseError[], bindingParser: BindingParser) {
if (block.parameters.length === 0) {
errors.push(new ParseError(block.sourceSpan, 'Conditional block does not have an expression'));
return null;
}
const isPrimaryIfBlock = block.name === 'if';
const expression =
parseBlockParameterToBinding(block.parameters[0], bindingParser, primaryExpressionStart);
// Expressions for `{:else if}` blocks start at 2 to skip the `if` from the expression.
parseBlockParameterToBinding(block.parameters[0], bindingParser, isPrimaryIfBlock ? 0 : 2);
let expressionAlias: string|null = null;
// Start from 1 since we processed the first parameter already.
@@ -305,6 +304,9 @@ function parseConditionalBlockParameters(
if (aliasMatch === null) {
errors.push(new ParseError(
param.sourceSpan, `Unrecognized conditional paramater "${param.expression}"`));
} else if (!isPrimaryIfBlock) {
errors.push(new ParseError(
param.sourceSpan, '"as" expression is only allowed on the primary "if" block'));
} else if (expressionAlias !== null) {
errors.push(
new ParseError(param.sourceSpan, 'Conditional can only have one "as" expression'));
@@ -718,7 +718,7 @@ describe('R3 AST source spans', () => {
it('is correct for if blocks', () => {
const html = `{#if cond.expr; as foo}` +
`Main case was true!` +
`{:else if other.expr; as bar}` +
`{:else if other.expr}` +
`Extra case was true!` +
`{:else}` +
`False case!` +
@@ -727,15 +727,12 @@ describe('R3 AST source spans', () => {
expectFromHtml(html, ['if']).toEqual([
[
'IfBlock',
'{#if cond.expr; as foo}Main case was true!{:else if other.expr; as bar}Extra case was true!{:else}False case!{/if}',
'{#if cond.expr; as foo}Main case was true!{:else if other.expr}Extra case was true!{:else}False case!{/if}',
'{#if cond.expr; as foo}', '{/if}'
],
['IfBlockBranch', '{#if cond.expr; as foo}Main case was true!', '{#if cond.expr; as foo}'],
['Text', 'Main case was true!'],
[
'IfBlockBranch', '{:else if other.expr; as bar}Extra case was true!',
'{:else if other.expr; as bar}'
],
['IfBlockBranch', '{:else if other.expr}Extra case was true!', '{:else if other.expr}'],
['Text', 'Extra case was true!'],
['IfBlockBranch', '{:else}False case!', '{:else}'],
['Text', 'False case!'],
@@ -1521,7 +1521,7 @@ describe('R3 template transform', () => {
expectIf(`
{#if cond.expr; as foo}
Main case was true!
{:else if other.expr; as bar}
{:else if other.expr}
Extra case was true!
{:else}
False case!
@@ -1530,7 +1530,7 @@ describe('R3 template transform', () => {
['IfBlock'],
['IfBlockBranch', 'cond.expr', 'foo'],
['Text', ' Main case was true! '],
['IfBlockBranch', 'other.expr', 'bar'],
['IfBlockBranch', 'other.expr'],
['Text', ' Extra case was true! '],
['IfBlockBranch', null],
['Text', ' False case! '],
@@ -1622,10 +1622,10 @@ describe('R3 template transform', () => {
`).toThrowError(/Conditional can only have one "as" expression/);
});
it('should report an else if block that has multiple `as` expressions', () => {
it('should report an else if block that has an `as` expression', () => {
expectIfError(`
{#if foo}hello{:else if bar; as one; as two}goodbye{/if}
`).toThrowError(/Conditional can only have one "as" expression/);
{#if foo}hello{:else if bar; as alias}goodbye{/if}
`).toThrowError(/"as" expression is only allowed on the primary "if" block/);
});
it('should report an unknown block inside an if block', () => {