Skip to content

Commit ea66c09

Browse files
refactor(compiler): Throw an error when old and new animations are used together
This adds a new compilation error if someone attempts to put legacy animations and `animate.enter` or `animate.leave` in the same component.
1 parent 70332b0 commit ea66c09

File tree

5 files changed

+61
-18
lines changed

5 files changed

+61
-18
lines changed

goldens/public-api/compiler-cli/error_code.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
// @public (undocumented)
88
export enum ErrorCode {
9+
COMPONENT_ANIMATIONS_CONFLICT = 2027,
910
COMPONENT_IMPORT_NOT_STANDALONE = 2011,
1011
COMPONENT_INVALID_SHADOW_DOM_SELECTOR = 2009,
1112
COMPONENT_INVALID_STYLE_URLS = 2021,

packages/compiler-cli/src/ngtsc/annotations/component/src/animations.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
TmplAstElement,
1414
} from '@angular/compiler';
1515

16+
const ANIMATE_ENTER = 'animate.enter';
1617
const ANIMATE_LEAVE = `animate.leave`;
1718

1819
/**
@@ -25,26 +26,24 @@ export function analyzeTemplateForAnimations(template: TmplAstNode[]): {
2526
const analyzer = new AnimationsAnalyzer();
2627
tmplAstVisitAll(analyzer, template);
2728

28-
// The template is considered selectorless only if there
29-
// are direct references to directives or pipes.
3029
return {hasAnimations: analyzer.hasAnimations};
3130
}
3231

3332
/**
3433
* Visitor that traverses all the template nodes and
35-
* expressions to look for selectorless references.
34+
* expressions to look for animation references.
3635
*/
3736
class AnimationsAnalyzer extends CombinedRecursiveAstVisitor {
3837
hasAnimations: boolean = false;
3938

4039
override visitElement(element: TmplAstElement): void {
4140
for (const attr of element.attributes) {
42-
if (attr.name === ANIMATE_LEAVE) {
41+
if (attr.name === ANIMATE_LEAVE || attr.name === ANIMATE_ENTER) {
4342
this.hasAnimations = true;
4443
}
4544
}
4645
for (const input of element.inputs) {
47-
if (input.name === ANIMATE_LEAVE) {
46+
if (input.name === ANIMATE_LEAVE || input.name === ANIMATE_ENTER) {
4847
this.hasAnimations = true;
4948
}
5049
}

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ import {extractHmrMetatadata, getHmrUpdateDeclaration} from '../../../hmr';
198198
import {getProjectRelativePath} from '../../../util/src/path';
199199
import {ComponentScope} from '../../../scope/src/api';
200200
import {analyzeTemplateForSelectorless} from './selectorless';
201+
import {analyzeTemplateForAnimations} from './animations';
201202

202203
const EMPTY_ARRAY: any[] = [];
203204

@@ -767,6 +768,24 @@ export class ComponentDecoratorHandler
767768
}
768769
}
769770
}
771+
772+
if (component.has('animations')) {
773+
const {hasAnimations} = analyzeTemplateForAnimations(template.nodes);
774+
if (hasAnimations) {
775+
if (diagnostics === undefined) {
776+
diagnostics = [];
777+
}
778+
diagnostics.push(
779+
makeDiagnostic(
780+
ErrorCode.COMPONENT_ANIMATIONS_CONFLICT,
781+
component.get('animations')!,
782+
`A component cannot have both the 'animations' property and use 'animate.enter' or 'animate.leave' in the template.`,
783+
),
784+
);
785+
isPoisoned = true;
786+
}
787+
}
788+
770789
const templateResource: Resource = template.declaration.isInline
771790
? {path: null, node: component.get('template')!}
772791
: {

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,12 @@ export enum ErrorCode {
178178
*/
179179
UNSUPPORTED_SELECTORLESS_COMPONENT_FIELD = 2026,
180180

181+
/**
182+
* A component is using both the `animations` property and `animate.enter` or `animate.leave`
183+
* in the template.
184+
*/
185+
COMPONENT_ANIMATIONS_CONFLICT = 2027,
186+
181187
SYMBOL_NOT_EXPORTED = 3001,
182188
/**
183189
* Raised when a relationship between directives and/or pipes would cause a cyclic import to be

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -299,30 +299,48 @@ runInEachFileSystem((os: string) => {
299299
expect(updateInstances?.length).toBe(1);
300300
});
301301

302-
it('should compile animate.enter with a host binding string', () => {
302+
it('should throw an error when legacy animations are used with animate.enter', () => {
303303
env.write(
304304
'test.ts',
305305
`
306-
import {Component, signal, ViewChild, ElementRef} from '@angular/core';
306+
import {Component} from '@angular/core';
307307
308308
@Component({
309309
selector: 'test-cmp',
310-
host: {'animate.enter': 'fade'},
311-
template:
312-
'<div><p>I should slide in</p></div>',
310+
template: '<div animate.enter="some-class"></div>',
311+
animations: [],
313312
})
314-
class TestComponent {
315-
show = signal(false);
316-
}
313+
class TestComponent {}
317314
`,
318315
);
319316

320-
env.driveMain();
317+
const diags = env.driveDiagnostics();
318+
expect(diags.length).toBe(1);
319+
expect(diags[0].messageText).toContain(
320+
`A component cannot have both the 'animations' property and use 'animate.enter' or 'animate.leave' in the template.`,
321+
);
322+
});
321323

322-
const jsContents = env.getContents('test.js');
323-
expect(jsContents).toContain('i0.ɵɵanimateEnter("fade");');
324-
const updateInstances = jsContents.match(/ɵɵanimateEnter\(/g);
325-
expect(updateInstances?.length).toBe(1);
324+
it('should throw an error when legacy animations are used with animate.leave', () => {
325+
env.write(
326+
'test.ts',
327+
`
328+
import {Component} from '@angular/core';
329+
330+
@Component({
331+
selector: 'test-cmp',
332+
template: '<div animate.leave="some-class"></div>',
333+
animations: [],
334+
})
335+
class TestComponent {}
336+
`,
337+
);
338+
339+
const diags = env.driveDiagnostics();
340+
expect(diags.length).toBe(1);
341+
expect(diags[0].messageText).toContain(
342+
`A component cannot have both the 'animations' property and use 'animate.enter' or 'animate.leave' in the template.`,
343+
);
326344
});
327345
});
328346

0 commit comments

Comments
 (0)