From eb1faa8f87edc6ddd9dd855752a3b2b8a626b09b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 17 Aug 2023 09:31:48 +0200 Subject: [PATCH] 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 --- packages/compiler/src/render3/r3_control_flow.ts | 14 ++++++++------ .../compiler/test/render3/r3_ast_spans_spec.ts | 9 +++------ .../test/render3/r3_template_transform_spec.ts | 10 +++++----- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/compiler/src/render3/r3_control_flow.ts b/packages/compiler/src/render3/r3_control_flow.ts index 72ca4cf15de..c4a05644065 100644 --- a/packages/compiler/src/render3/r3_control_flow.ts +++ b/packages/compiler/src/render3/r3_control_flow.ts @@ -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')); diff --git a/packages/compiler/test/render3/r3_ast_spans_spec.ts b/packages/compiler/test/render3/r3_ast_spans_spec.ts index 9990e580c68..e6a79a02608 100644 --- a/packages/compiler/test/render3/r3_ast_spans_spec.ts +++ b/packages/compiler/test/render3/r3_ast_spans_spec.ts @@ -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!'], diff --git a/packages/compiler/test/render3/r3_template_transform_spec.ts b/packages/compiler/test/render3/r3_template_transform_spec.ts index 35c287bef5b..4826e607afb 100644 --- a/packages/compiler/test/render3/r3_template_transform_spec.ts +++ b/packages/compiler/test/render3/r3_template_transform_spec.ts @@ -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', () => {