fix(animations): generate aot code for animation trigger output events #12291

matsko · 2016-10-14T03:00:55Z

No description provided.

matsko · 2016-10-14T03:01:49Z

@tbosch non jit code fails for animation integration tests.

Also, we now explose player within animations (which I think is a useful feature to have since it allows for users to get access to the player to pause and cancel the animation programmatically).

Closes angular#11707 Closes angular#12291

matsko · 2016-10-14T03:48:41Z

Aot Sample:

    _View_App0.prototype.detectChangesInternal = function(throwOnChange) {
        var self = this;
        var transitionAnimation = null ;
        var currVal_4 = self.context.openClose;
        if (jit_checkBinding6(throwOnChange, self._expr_4, currVal_4)) {
            var oldRenderVar = self._expr_4;
            if ((oldRenderVar == jit_UNINITIALIZED5) ) {
                (oldRenderVar = 'void');
            }
            var newRenderVar = currVal_4;
            if ((newRenderVar == jit_UNINITIALIZED5) ) {
                (newRenderVar = 'void');
            }
            transitionAnimation = self.componentType.animations['myTrigger'](self, self._el_8, oldRenderVar, newRenderVar);
            self._expr_4 = currVal_4;
        }
        if (transitionAnimation) {
            transitionAnimation.player.onDone(self.eventHandler(self._handle_myTrigger_8_0.bind(self, transitionAnimation)));
        }
        if (transitionAnimation) {
            transitionAnimation.player.onStart(self.eventHandler(self._handle_myTrigger_8_1.bind(self, transitionAnimation)));
        }
        self.detectContentChildrenChanges(throwOnChange);
        self.detectViewChildrenChanges(throwOnChange);
    }

matsko · 2016-10-14T16:36:13Z

main refactorings:

generate events within onDone and onStart
return only player from animation factory
register inside of the if block

Closes angular#11707 Closes angular#12291

matsko · 2016-10-17T22:20:43Z

Codegen looks like this now:

_View_App0.prototype.detectChangesInternal = function(throwOnChange) {
    var self = this;
    var currVal_4 = self.context.openClose;
    if (jit_checkBinding6(throwOnChange, self._expr_4, currVal_4)) {
        var oldRenderVar = self._expr_4;
        if ((oldRenderVar == jit_UNINITIALIZED5) ) {
            (oldRenderVar = 'void');
        }
        var newRenderVar = currVal_4;
        if ((newRenderVar == jit_UNINITIALIZED5) ) {
            (newRenderVar = 'void');
        }
        var animationTransition_myTrigger = self.componentType.animations['myTrigger'](self, self._el_8, oldRenderVar, newRenderVar);
        animationTransition_myTrigger.onDone(self._handle_myTrigger_8_0.bind(self));
        animationTransition_myTrigger.onStart(self._handle_myTrigger_8_1.bind(self));
        self._expr_4 = currVal_4;
    }
    self.detectContentChildrenChanges(throwOnChange);
    self.detectViewChildrenChanges(throwOnChange);
};

Closes angular#11707 Closes angular#12291

tbosch · 2016-10-18T15:34:18Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

@@ -172,12 +176,32 @@ function bindAndWriteToRenderer(
            [newRenderVar.set(emptyStateValue).toStmt()]));


Remove the variables oldRenderVar and newRenderVar, and use a ternary expression instead.

...animationFnExpr.callFn([o.THIS_EXPR, oldRenderValue.equals(o.importExpr(resolveIdentifier(Identifiers.UNINITIALIZED)) .conditional(oldRenderVar, emptyStateValue)), newRenderValue.equals(o.importExpr(resolveIdentifier(Identifiers.UNINITIALIZED)) .conditional(newRenderVar, emptyStateValue)), ...

tbosch · 2016-10-18T15:37:13Z

modules/@angular/compiler/src/animation/animation_compiler.ts

@@ -290,7 +292,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
          new o.FnParam(_ANIMATION_CURRENT_STATE_VAR.name, o.DYNAMIC_TYPE),
          new o.FnParam(_ANIMATION_NEXT_STATE_VAR.name, o.DYNAMIC_TYPE)
        ],
-        statements);
+        statements, o.DYNAMIC_TYPE);


This should be o.importType(resolveIdentifier(Identifiers.AnimationTransition)), as the factory returns an AnimationTransition, right?

tbosch · 2016-10-18T15:38:04Z

modules/@angular/compiler/src/view_compiler/event_binder.ts

@@ -10,6 +10,7 @@ import {CompileDirectiveMetadata} from '../compile_metadata';
 import {isPresent} from '../facade/lang';
 import {identifierToken} from '../identifiers';
 import * as o from '../output/output_ast';
+import {ViewType} from '../private_import_core';


tbosch · 2016-10-18T15:38:32Z

modules/@angular/compiler/src/view_compiler/event_binder.ts

-      listener.listenToAnimation();
-    } else {
+    // the animation listeners are handled within property_binder.ts to
+    // allow then to be placed next to the animation factory statements


typo: allow them...

tbosch · 2016-10-18T15:39:55Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

        compileMethod = view.animationBindingsMethod;

+        const _ANIMATION_TRANSITION_VAR = o.variable('animationTransition_' + animationName);


remove _ prefix, and don't upper case everything. This is just a variable here, not a constant defined at the top leve.

tbosch · 2016-10-18T15:40:42Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

        compileMethod = view.animationBindingsMethod;

+        const _ANIMATION_TRANSITION_VAR = o.variable('animationTransition_' + animationName);


Move this further down, right before you declare the variable (as it is not used before that).

tbosch · 2016-10-18T15:44:30Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

-        view.detachMethod.addStmt(
-            animationFnExpr.callFn([o.THIS_EXPR, renderNode, oldRenderValue, emptyStateValue])
-                .toStmt());
+        detachStmts.push(_ANIMATION_TRANSITION_VAR


Why are we always creating the animation again on view detach?

tbosch · 2016-10-18T15:45:44Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

+        eventListeners.forEach(listener => {
+          if (listener.isAnimation && listener.eventName === animationName) {
+            let callbackMethod = listener.eventPhase == 'start' ? 'onStart' : 'onDone';
+            let transitionEvent = o.variable('e');


this variable is not used any more.

tbosch · 2016-10-18T15:48:08Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

+                _ANIMATION_TRANSITION_VAR
+                    .callMethod(
+                        callbackMethod,
+                        [o.THIS_EXPR.prop(listener.methodName).callMethod('bind', [o.THIS_EXPR])])


Use .callMethod(o.BuiltinMethod.Bind, ..., don't pass in bind as a string.

tbosch · 2016-10-18T15:48:36Z

modules/@angular/compiler/src/view_compiler/property_binder.ts

+
+        eventListeners.forEach(listener => {
+          if (listener.isAnimation && listener.eventName === animationName) {
+            let callbackMethod = listener.eventPhase == 'start' ? 'onStart' : 'onDone';


Move the logic here into CompileEventListener.listenToAnimation.

tbosch · 2016-10-18T15:49:31Z

modules/@angular/core/src/animation/animation_transition.ts

+  private _totalTime: number;
+
+  constructor(
+      public player: AnimationPlayer,


Why does the player need to be public?

tbosch · 2016-10-18T15:50:14Z

modules/@angular/core/src/animation/animation_transition.ts

+
+  constructor(
+      public player: AnimationPlayer,
+      {fromState, toState, totalTime}: {fromState: string, toState: string, totalTime: number}) {


Don't use named arguments here, just positional ones and declare the fields as well. E.g.

constructor(private _player: AnimationPlayer, private _fromState: string, ...) {}

tbosch · 2016-10-18T15:54:58Z

tools/public_api_guard/core/index.d.ts

        fromState: string;
        toState: string;
        totalTime: number;
+        phaseName: string;


Please ping @IgorMinar whether this is a breaking change, and we need to make this field optional. I don't expect users to new up their own AnimationTransitionEvent.

Closes angular#11707 Closes angular#12291

tbosch · 2016-10-18T22:00:12Z

modules/@angular/core/src/animation/animation_transition.ts

+      private _totalTime: number) {}
+
+  /** @internal */
+  _createEvent(phaseName: string): AnimationTransitionEvent {


why not private

Closes angular#11707 Closes angular#12291

angular#12291) Closes angular#11707 Closes angular#12291

angular-automatic-lock-bot · 2019-09-10T01:21:32Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Oct 14, 2016

matsko assigned tbosch Oct 14, 2016

matsko force-pushed the codegen_animation_outputs branch from e78a4bc to f655e78 Compare October 14, 2016 03:25

matsko added a commit to matsko/angular that referenced this pull request Oct 14, 2016

fix(animations): generate aot code for animation trigger output events

f655e78

Closes angular#11707 Closes angular#12291

matsko force-pushed the codegen_animation_outputs branch from f655e78 to 374d991 Compare October 17, 2016 22:18

matsko added a commit to matsko/angular that referenced this pull request Oct 17, 2016

fix(animations): generate aot code for animation trigger output events

374d991

Closes angular#11707 Closes angular#12291

matsko force-pushed the codegen_animation_outputs branch from 374d991 to ed328c5 Compare October 17, 2016 22:26

matsko added a commit to matsko/angular that referenced this pull request Oct 17, 2016

fix(animations): generate aot code for animation trigger output events

ed328c5

Closes angular#11707 Closes angular#12291

matsko force-pushed the codegen_animation_outputs branch from ed328c5 to af0e84f Compare October 17, 2016 23:41

matsko added a commit to matsko/angular that referenced this pull request Oct 17, 2016

fix(animations): generate aot code for animation trigger output events

af0e84f

Closes angular#11707 Closes angular#12291

tbosch reviewed Oct 18, 2016

View reviewed changes

tbosch added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews pr_state: LGTM labels Oct 18, 2016

matsko force-pushed the codegen_animation_outputs branch from af0e84f to b5702b1 Compare October 18, 2016 21:55

matsko added a commit to matsko/angular that referenced this pull request Oct 18, 2016

fix(animations): generate aot code for animation trigger output events

b5702b1

Closes angular#11707 Closes angular#12291

tbosch reviewed Oct 18, 2016

View reviewed changes

matsko force-pushed the codegen_animation_outputs branch from b5702b1 to 6466f4b Compare October 18, 2016 22:04

matsko added a commit to matsko/angular that referenced this pull request Oct 18, 2016

fix(animations): generate aot code for animation trigger output events

6466f4b

Closes angular#11707 Closes angular#12291

fix(animations): generate aot code for animation trigger output events

7d0a494

Closes angular#11707 Closes angular#12291

matsko force-pushed the codegen_animation_outputs branch from 6466f4b to 7d0a494 Compare October 18, 2016 23:23

matsko added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 18, 2016

alxhub merged commit 6e5f8b5 into angular:master Oct 19, 2016

matsko deleted the codegen_animation_outputs branch October 19, 2016 00:52

btrigueiro pushed a commit to btrigueiro/angular that referenced this pull request Oct 21, 2016

fix(animations): generate aot code for animation trigger output events (

48206d8

angular#12291) Closes angular#11707 Closes angular#12291

angular-automatic-lock-bot bot unassigned tbosch Sep 10, 2019

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(animations): generate aot code for animation trigger output events #12291

fix(animations): generate aot code for animation trigger output events #12291

matsko commented Oct 14, 2016

matsko commented Oct 14, 2016

matsko commented Oct 14, 2016 •

edited

Loading

matsko commented Oct 14, 2016

matsko commented Oct 17, 2016 •

edited

Loading

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

tbosch Oct 18, 2016

angular-automatic-lock-bot bot commented Sep 10, 2019

		@@ -172,12 +176,32 @@ function bindAndWriteToRenderer(
		[newRenderVar.set(emptyStateValue).toStmt()]));

		compileMethod = view.animationBindingsMethod;

		const _ANIMATION_TRANSITION_VAR = o.variable('animationTransition_' + animationName);

fix(animations): generate aot code for animation trigger output events #12291

fix(animations): generate aot code for animation trigger output events #12291

Conversation

matsko commented Oct 14, 2016

matsko commented Oct 14, 2016

matsko commented Oct 14, 2016 • edited Loading

matsko commented Oct 14, 2016

matsko commented Oct 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 10, 2019

matsko commented Oct 14, 2016 •

edited

Loading

matsko commented Oct 17, 2016 •

edited

Loading