From 1045abd3c873f2da90ecde31fff47b6dbd486ee0 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 3 Aug 2023 14:30:39 +0200 Subject: [PATCH] refactor(compiler): add more deferred validations (#51262) Adds validations for the following invalid deferred block structures: * Duplicated triggers. * Multiple `minimum` parameters on `placeholder` and `loading` blocks. * Multiple `after` parameters on `loading` blocks. PR Close #51262 --- .../src/render3/r3_deferred_blocks.ts | 12 ++++++ .../src/render3/r3_deferred_triggers.ts | 12 ++++-- .../render3/r3_template_transform_spec.ts | 38 +++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/render3/r3_deferred_blocks.ts b/packages/compiler/src/render3/r3_deferred_blocks.ts index ab591bd24c9..bceed138b18 100644 --- a/packages/compiler/src/render3/r3_deferred_blocks.ts +++ b/packages/compiler/src/render3/r3_deferred_blocks.ts @@ -112,6 +112,10 @@ function parsePlaceholderBlock(ast: html.Block, visitor: html.Visitor): t.Deferr for (const param of ast.parameters) { if (MINIMUM_PARAMETER_PATTERN.test(param.expression)) { + if (minimumTime != null) { + throw new Error(`Placeholder block can only have one "minimum" parameter`); + } + const parsedTime = parseDeferredTime(param.expression.slice(getTriggerParametersStart(param.expression))); @@ -137,6 +141,10 @@ function parseLoadingBlock(ast: html.Block, visitor: html.Visitor): t.DeferredBl for (const param of ast.parameters) { if (AFTER_PARAMETER_PATTERN.test(param.expression)) { + if (afterTime != null) { + throw new Error(`Loading block can only have one "after" parameter`); + } + const parsedTime = parseDeferredTime(param.expression.slice(getTriggerParametersStart(param.expression))); @@ -146,6 +154,10 @@ function parseLoadingBlock(ast: html.Block, visitor: html.Visitor): t.DeferredBl afterTime = parsedTime; } else if (MINIMUM_PARAMETER_PATTERN.test(param.expression)) { + if (minimumTime != null) { + throw new Error(`Loading block can only have one "minimum" parameter`); + } + const parsedTime = parseDeferredTime(param.expression.slice(getTriggerParametersStart(param.expression))); diff --git a/packages/compiler/src/render3/r3_deferred_triggers.ts b/packages/compiler/src/render3/r3_deferred_triggers.ts index 24c067e9b60..13e46824ec4 100644 --- a/packages/compiler/src/render3/r3_deferred_triggers.ts +++ b/packages/compiler/src/render3/r3_deferred_triggers.ts @@ -51,7 +51,7 @@ export function parseWhenTrigger( const start = getTriggerParametersStart(expression, whenIndex + 1); const parsed = bindingParser.parseBinding( expression.slice(start), false, sourceSpan, sourceSpan.start.offset + start); - trackTrigger('when', triggers, new t.BoundDeferredTrigger(parsed, sourceSpan)); + trackTrigger('when', triggers, errors, new t.BoundDeferredTrigger(parsed, sourceSpan)); } } @@ -243,7 +243,7 @@ class OnTriggerParser { } private trackTrigger(name: keyof t.DeferredBlockTriggers, trigger: t.DeferredTrigger): void { - trackTrigger(name, this.triggers, trigger); + trackTrigger(name, this.triggers, this.errors, trigger); } private error(token: Token, message: string): void { @@ -259,9 +259,13 @@ class OnTriggerParser { /** Adds a trigger to a map of triggers. */ function trackTrigger( - name: keyof t.DeferredBlockTriggers, allTriggers: t.DeferredBlockTriggers, + name: keyof t.DeferredBlockTriggers, allTriggers: t.DeferredBlockTriggers, errors: ParseError[], trigger: t.DeferredTrigger) { - allTriggers[name] = trigger as any; + if (allTriggers[name]) { + errors.push(new ParseError(trigger.sourceSpan, `Duplicate "${name}" trigger is not allowed`)); + } else { + allTriggers[name] = trigger as any; + } } function createIdleTrigger( diff --git a/packages/compiler/test/render3/r3_template_transform_spec.ts b/packages/compiler/test/render3/r3_template_transform_spec.ts index 8bbb5679693..1e9f06bbed5 100644 --- a/packages/compiler/test/render3/r3_template_transform_spec.ts +++ b/packages/compiler/test/render3/r3_template_transform_spec.ts @@ -1096,6 +1096,44 @@ describe('R3 template transform', () => { expectDeferredError('{#defer on viewport(a, b)}hello{/defer}') .toThrowError(/"viewport" trigger can only have zero or one parameters/); }); + + it('should report duplicate when triggers', () => { + expectDeferredError('{#defer when isVisible(); when somethingElse()}hello{/defer}') + .toThrowError(/Duplicate "when" trigger is not allowed/); + }); + + it('should report duplicate on triggers', () => { + expectDeferredError('{#defer on idle; when isVisible(); on timer(10), idle}hello{/defer}') + .toThrowError(/Duplicate "idle" trigger is not allowed/); + }); + + it('should report duplicate prefetch when triggers', () => { + expectDeferredError( + '{#defer prefetch when isVisible(); prefetch when somethingElse()}hello{/defer}') + .toThrowError(/Duplicate "when" trigger is not allowed/); + }); + + it('should report duplicate prefetch on triggers', () => { + expectDeferredError( + '{#defer prefetch on idle; prefetch when isVisible(); prefetch on timer(10), idle}hello{/defer}') + .toThrowError(/Duplicate "idle" trigger is not allowed/); + }); + + it('should report multiple minimum parameters on a placeholder block', () => { + expectDeferredError( + '{#defer}hello{:placeholder minimum 1s; minimum 500ms}placeholder{/defer}') + .toThrowError(/Placeholder block can only have one "minimum" parameter/); + }); + + it('should report multiple minimum parameters on a loading block', () => { + expectDeferredError('{#defer}hello{:loading minimum 1s; minimum 500ms}loading{/defer}') + .toThrowError(/Loading block can only have one "minimum" parameter/); + }); + + it('should report multiple after parameters on a loading block', () => { + expectDeferredError('{#defer}hello{:loading after 1s; after 500ms}loading{/defer}') + .toThrowError(/Loading block can only have one "after" parameter/); + }); }); }); });