iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): don’t publish animation bindings as attributes (#27805) 

Some of the animation tests have been failing because animation gets
triggered multiple times. The reason for this is that the compiler was
generating static attribute bindings in addition to dynamic bindings.
This created multiple writes to the animation render which failed the
tests.

PR Close #27805
This commit is contained in:
Misko Hevery 2018-12-21 11:53:18 -08:00 committed by Ben Lesh
parent 880b4aabdb
commit 1f1e77b641
9 changed files with 1151 additions and 1080 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts
View File

@ -120,4 +120,70 @@ describe('r3_view_compiler', () => {
expectEmit(result.source, bV_call, 'Incorrect bV call');
});
});
describe('animations', () => {
it('should keep @attr but suppress [@attr]', () => {
const files: MockDirectory = {
app: {
'example.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'my-app',
template: '<div @attrOnly [@myAnimation]="exp"></div>'
})
export class MyApp {
}
@NgModule({declarations: [MyApp]})
export class MyModule {}`
}
};
const template = `
const _c0 = ["@attrOnly", ""];
// ...
template: function MyApp_Template(rf, ctx) {
if (rf & 1) {
$i0$.ɵelement(0, "div", _c0);
// ...
}
// ...
}`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect initialization attributes');
});
it('should dedup multiple [@event] listeners', () => {
const files: MockDirectory = {
app: {
'example.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'my-app',
template: '<div (@mySelector.start)="false" (@mySelector.done)="false" [@mySelector]="0"></div>'
})
export class MyApp {
}
@NgModule({declarations: [MyApp]})
export class MyModule {}`
}
};
const template = `
const _c0 = [1, "mySelector"];
// ...
template: function MyApp_Template(rf, ctx) {
if (rf & 1) {
$i0$.ɵelementStart(0, "div", _c0);
// ...
}
// ...
}`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect initialization attributes');
});
});
});

3
packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
View File

@ -214,7 +214,6 @@ describe('compiler compliance: styling', () => {
};
const template = `
const $e0_attrs$ = ["@foo", ""];
const $e1_attrs$ = ["@bar", ""];
const $e2_attrs$ = ["@baz", ""];
…
@ -224,7 +223,7 @@ describe('compiler compliance: styling', () => {
vars: 1,
template: function MyComponent_Template(rf, $ctx$) {
if (rf & 1) {
$r3$.ɵelement(0, "div", $e0_attrs$);
$r3$.ɵelement(0, "div");
$r3$.ɵelement(1, "div", $e1_attrs$);
$r3$.ɵelement(2, "div", $e2_attrs$);
}

39
packages/compiler/src/render3/view/template.ts
View File

@ -10,7 +10,7 @@ import {flatten, sanitizeIdentifier} from '../../compile_metadata';
import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter';
import {ConstantPool} from '../../constant_pool';
import * as core from '../../core';
import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast';
import {AST, ASTWithSource, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast';
import {Lexer} from '../../expression_parser/lexer';
import {Parser} from '../../expression_parser/parser';
import * as i18n from '../../i18n/i18n_ast';
@ -967,6 +967,29 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
styles?: StylingBuilder): o.Expression[] {
const attrExprs: o.Expression[] = [];
const nonSyntheticInputs: t.BoundAttribute[] = [];
const alreadySeen = new Set<string>();
function isASTWithSource(ast: AST): ast is ASTWithSource {
return ast instanceof ASTWithSource;
}
function isLiteralPrimitive(ast: AST): ast is LiteralPrimitive {
return ast instanceof LiteralPrimitive;
}
function addAttrExpr(key: string | number, value?: o.Expression): void {
if (typeof key === 'string') {
if (!alreadySeen.has(key)) {
attrExprs.push(o.literal(key));
if (value !== undefined) {
attrExprs.push(value);
}
alreadySeen.add(key);
}
} else {
attrExprs.push(o.literal(key));
}
}
if (inputs.length) {
const EMPTY_STRING_EXPR = asLiteral('');
@ -976,7 +999,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// may be supported differently in future versions of angular. However,
// @triggers should always just be treated as regular attributes (it's up
// to the renderer to detect and use them in a special way).
attrExprs.push(asLiteral(prepareSyntheticAttributeName(input.name)), EMPTY_STRING_EXPR);
const valueExp = input.value;
if (isASTWithSource(valueExp)) {
const literal = valueExp.ast;
if (isLiteralPrimitive(literal) && literal.value === undefined) {
addAttrExpr(prepareSyntheticAttributeName(input.name), EMPTY_STRING_EXPR);
}
}
} else {
nonSyntheticInputs.push(input);
}
@ -991,9 +1020,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
}
if (nonSyntheticInputs.length || outputs.length) {
attrExprs.push(o.literal(core.AttributeMarker.SelectOnly));
nonSyntheticInputs.forEach((i: t.BoundAttribute) => attrExprs.push(asLiteral(i.name)));
outputs.forEach((o: t.BoundEvent) => attrExprs.push(asLiteral(o.name)));
addAttrExpr(core.AttributeMarker.SelectOnly);
nonSyntheticInputs.forEach((i: t.BoundAttribute) => addAttrExpr(i.name));
outputs.forEach((o: t.BoundEvent) => addAttrExpr(o.name));
}
return attrExprs;

5
packages/core/src/render3/instructions.ts
View File

@ -394,8 +394,9 @@ function renderComponentOrTemplate<T>(
hostView: LView, context: T, templateFn?: ComponentTemplate<T>) {
const rendererFactory = hostView[RENDERER_FACTORY];
const oldView = enterView(hostView, hostView[HOST_NODE]);
const normalExecutionPath = !getCheckNoChangesMode();
try {
if (rendererFactory.begin) {
if (normalExecutionPath && rendererFactory.begin) {
rendererFactory.begin();
}
@ -414,7 +415,7 @@ function renderComponentOrTemplate<T>(
templateFn && templateFn(RenderFlags.Update, context !);
refreshDescendantViews(hostView);
} finally {
if (rendererFactory.end) {
if (normalExecutionPath && rendererFactory.end) {
rendererFactory.end();
}
leaveView(oldView);

56
packages/core/test/animation/animation_integration_spec.ts
View File

@ -109,8 +109,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.status).toEqual('done');
}));
fixmeIvy('unknown').it(
'should always run .start callbacks before .done callbacks even for noop animations',
it('should always run .start callbacks before .done callbacks even for noop animations',
fakeAsync(() => {
@Component({
selector: 'cmp',
@ -142,8 +141,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.log).toEqual(['start', 'done']);
}));
fixmeIvy('unknown').it(
'should emit the correct totalTime value for a noop-animation', fakeAsync(() => {
it('should emit the correct totalTime value for a noop-animation', fakeAsync(() => {
@Component({
selector: 'cmp',
template: `
@ -299,7 +297,7 @@ const DEFAULT_COMPONENT_ID = '1';
});
describe('animation triggers', () => {
fixmeIvy('unknown').it('should trigger a state change animation from void => state', () => {
it('should trigger a state change animation from void => state', () => {
@Component({
selector: 'if-cmp',
template: `
@ -329,8 +327,7 @@ const DEFAULT_COMPONENT_ID = '1';
]);
});
fixmeIvy('unknown').it(
'should allow a transition to use a function to determine what method to run', () => {
it('should allow a transition to use a function to determine what method to run', () => {
let valueToMatch = '';
let capturedElement: any;
const transitionFn = (fromState: string, toState: string, element: any) => {
@ -342,10 +339,9 @@ const DEFAULT_COMPONENT_ID = '1';
selector: 'if-cmp',
template: '<div #element [@myAnimation]="exp"></div>',
animations: [
trigger(
'myAnimation',
[transition(
transitionFn, [style({opacity: 0}), animate(1234, style({opacity: 1}))])]),
trigger('myAnimation', [transition(
transitionFn,
[style({opacity: 0}), animate(1234, style({opacity: 1}))])]),
]
})
class Cmp {
@ -573,8 +569,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.log).toEqual(['myAnimation-start', 'myAnimation-done']);
}));
fixmeIvy('unknown').it(
'should only turn a view removal as into `void` state transition', () => {
it('should only turn a view removal as into `void` state transition', () => {
@Component({
selector: 'if-cmp',
template: `
@ -584,11 +579,9 @@ const DEFAULT_COMPONENT_ID = '1';
'myAnimation',
[
transition(
'void <=> *',
[style({width: '0px'}), animate(1000, style({width: '100px'}))]),
'void <=> *', [style({width: '0px'}), animate(1000, style({width: '100px'}))]),
transition(
'* => *',
[style({height: '0px'}), animate(1000, style({height: '100px'}))]),
'* => *', [style({height: '0px'}), animate(1000, style({height: '100px'}))]),
])]
})
class Cmp {
@ -671,7 +664,7 @@ const DEFAULT_COMPONENT_ID = '1';
]);
});
fixmeIvy('unknown').it('should stringify boolean triggers to `1` and `0`', () => {
it('should stringify boolean triggers to `1` and `0`', () => {
@Component({
selector: 'if-cmp',
template: `
@ -1577,7 +1570,8 @@ const DEFAULT_COMPONENT_ID = '1';
}
});
it('should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles',
fixmeIvy('unknown').it(
'should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles',
() => {
@Component({
selector: 'ani-cmp',
@ -1977,7 +1971,8 @@ const DEFAULT_COMPONENT_ID = '1';
expect(players[0].duration).toEqual(5678);
});
it('should not render animations when the object expression value is the same as it was previously',
fixmeIvy('unknown').it(
'should not render animations when the object expression value is the same as it was previously',
() => {
@Component({
selector: 'ani-cmp',
@ -2459,8 +2454,7 @@ const DEFAULT_COMPONENT_ID = '1';
});
describe('animation listeners', () => {
fixmeIvy('unknown').it(
'should trigger a `start` state change listener for when the animation changes state from void => state',
it('should trigger a `start` state change listener for when the animation changes state from void => state',
fakeAsync(() => {
@Component({
selector: 'if-cmp',
@ -2497,8 +2491,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.event.toState).toEqual('true');
}));
fixmeIvy('unknown').it(
'should trigger a `done` state change listener for when the animation changes state from a => b',
it('should trigger a `done` state change listener for when the animation changes state from a => b',
fakeAsync(() => {
@Component({
selector: 'if-cmp',
@ -2508,8 +2501,7 @@ const DEFAULT_COMPONENT_ID = '1';
animations: [trigger(
'myAnimation123',
[transition(
'* => b',
[style({'opacity': '0'}), animate(999, style({'opacity': '1'}))])])],
'* => b', [style({'opacity': '0'}), animate(999, style({'opacity': '1'}))])])],
})
class Cmp {
exp: any = false;
@ -2767,8 +2759,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.event.toState).toEqual('TRUE');
}));
fixmeIvy('unknown').it(
'should always fire callbacks even when a transition is not detected', fakeAsync(() => {
it('should always fire callbacks even when a transition is not detected', fakeAsync(() => {
@Component({
selector: 'my-cmp',
template: `
@ -2844,8 +2835,7 @@ const DEFAULT_COMPONENT_ID = '1';
expect(cmp.log).toEqual(['start => void', 'done => void']);
}));
fixmeIvy('unknown').it(
'should fire callbacks on a sub animation once it starts and finishes', fakeAsync(() => {
it('should fire callbacks on a sub animation once it starts and finishes', fakeAsync(() => {
@Component({
selector: 'my-cmp',
template: `
@ -3597,8 +3587,7 @@ const DEFAULT_COMPONENT_ID = '1';
/only state\(\) and transition\(\) definitions can sit inside of a trigger\(\)/);
});
fixmeIvy('unknown').it(
'should combine multiple errors together into one exception when an animation fails to be built',
it('should combine multiple errors together into one exception when an animation fails to be built',
() => {
@Component({
selector: 'if-cmp',
@ -3741,8 +3730,7 @@ const DEFAULT_COMPONENT_ID = '1';
});
});
fixmeIvy('unknown').it(
'should throw when using an @prop binding without the animation module', () => {
it('should throw when using an @prop binding without the animation module', () => {
@Component({template: `<div [@myAnimation]="true"></div>`})
class Cmp {
}

6
packages/core/test/animation/animation_query_integration_spec.ts
View File

@ -853,8 +853,7 @@ import {HostListener} from '../../src/metadata/directives';
});
});
fixmeIvy('unknown').it(
'should cleanup :enter and :leave artifacts from nodes when any animation sequences fail to be built',
it('should cleanup :enter and :leave artifacts from nodes when any animation sequences fail to be built',
() => {
@Component({
selector: 'ani-cmp',
@ -2520,8 +2519,7 @@ import {HostListener} from '../../src/metadata/directives';
expect(p4.element.classList.contains('d'));
});
fixmeIvy('unknown').it(
'should collect multiple root levels of :enter and :leave nodes', () => {
it('should collect multiple root levels of :enter and :leave nodes', () => {
@Component({
selector: 'ani-cmp',
animations: [

24
packages/core/test/animation/animation_router_integration_spec.ts
View File

@ -34,8 +34,7 @@ import {RouterTestingModule} from '@angular/router/testing';
});
});
fixmeIvy('unknown').it(
'should query the old and new routes via :leave and :enter', fakeAsync(() => {
it('should query the old and new routes via :leave and :enter', fakeAsync(() => {
@Component({
animations: [
trigger(
@ -152,8 +151,7 @@ import {RouterTestingModule} from '@angular/router/testing';
]);
}));
fixmeIvy('unknown').it(
'should allow inner enter animations to be emulated within a routed item', fakeAsync(() => {
it('should allow inner enter animations to be emulated within a routed item', fakeAsync(() => {
@Component({
animations: [
trigger(
@ -258,8 +256,7 @@ import {RouterTestingModule} from '@angular/router/testing';
]);
}));
fixmeIvy('unknown').it(
'should allow inner leave animations to be emulated within a routed item', fakeAsync(() => {
it('should allow inner leave animations to be emulated within a routed item', fakeAsync(() => {
@Component({
animations: [
trigger(
@ -305,8 +302,7 @@ import {RouterTestingModule} from '@angular/router/testing';
]),
trigger(
'ifAnimation',
[transition(
':leave', [style({opacity: 1}), animate(1000, style({opacity: 0}))])]),
[transition(':leave', [style({opacity: 1}), animate(1000, style({opacity: 0}))])]),
]
})
class Page1Cmp {
@ -364,8 +360,7 @@ import {RouterTestingModule} from '@angular/router/testing';
]);
}));
fixmeIvy('unknown').it(
'should properly collect :enter / :leave router nodes even when another non-router *template component is within the trigger boundaries',
it('should properly collect :enter / :leave router nodes even when another non-router *template component is within the trigger boundaries',
fakeAsync(() => {
@Component({
selector: 'ani-cmp',
@ -451,11 +446,9 @@ import {RouterTestingModule} from '@angular/router/testing';
expect(ip2.element.innerText).toEqual('page2');
}));
fixmeIvy('unknown').it(
'should allow a recursive set of :leave animations to occur for nested routes',
it('should allow a recursive set of :leave animations to occur for nested routes',
fakeAsync(() => {
@Component(
{selector: 'ani-cmp', template: '<router-outlet name="recur"></router-outlet>'})
@Component({selector: 'ani-cmp', template: '<router-outlet name="recur"></router-outlet>'})
class ContainerCmp {
constructor(private _router: Router) {}
log: string[] = [];
@ -472,8 +465,7 @@ import {RouterTestingModule} from '@angular/router/testing';
trigger(
'pageAnimations',
[
transition(
':leave', [group([
transition(':leave', [group([
sequence([style({opacity: 1}), animate('1s', style({opacity: 0}))]),
query('@*', animateChild(), {optional: true})
])]),

3
packages/core/test/animation/animations_with_web_animations_integration_spec.ts
View File

@ -242,8 +242,7 @@ import {fixmeIvy} from '@angular/private/testing';
]);
});
fixmeIvy('unknown').it(
'should treat * styles as ! for queried items that are collected in a container that is being removed',
it('should treat * styles as ! for queried items that are collected in a container that is being removed',
() => {
@Component({
selector: 'my-app',

3
packages/platform-browser/animations/test/animation_renderer_spec.ts
View File

@ -279,8 +279,7 @@ import {el} from '../../testing/src/browser_util';
});
});
fixmeIvy(`FW-802: Animation 'start' and 'end' hooks are invoked twice`)
.it('should provide hooks at the start and end of change detection', () => {
it('should provide hooks at the start and end of change detection', () => {
@Component({
selector: 'my-cmp',
template: `