fix(compiler): respect preAssignBindingsEnabled state #10469

devversion · 2017-03-07T19:14:59Z

The $mdCompiler now respects the preAssignBindingsEnabled state when using AngularJS 1.6 or higher.

BREAKING CHANGE

Developers that are upgrading from AngularJS 1.5 to AngularJS 1.6 may run into issues with their custom controllers.

All custom controllers that are are being used inside of Material components, like in the $mdDialog or $mdToast, can be affected.

Limited to controllers that are used in combination with the bindToController and locals option.

Angular Material starts respecting the preAssignBindingsEnabled state when using AngularJS 1.6+

This will mean that bindings are no longer initialized at constructor time.

$mdDialog.show({
  locals: {
    myVar: true
  },
  controller: MyController,
  bindToController: true
}

function MyController() {
  // No locals from Angular Material are set yet. e.g myVar is undefined.
}

MyController.prototype.$onInit = function() {
  // Bindings are now set in the $onInit lifecycle hook.
}

To provide consistency, Angular Material components with custom controllers now work with the $onInit lifecycle hook (no other lifecycle hooks)

Fixes #10016

ThomasBurleson · 2017-03-07T22:41:15Z

@devversion - Think this one of my favorite PRs for Q1 2017. 🥇

mgol · 2017-03-07T23:17:48Z

The Karma not logging issue is a regression: karma-runner/karma#2582. How about locking Karma to 1.4 for now as it's not clear for now how they will resolve the issue and removing the workaround?

mgol · 2017-03-07T23:22:27Z

src/core/services/compiler/compiler.js

+  $provide.constant('$$mdPreAssignBindings',
+    // For Angular versions that do not support `preAssignBindingsEnabled`, always pre-assign the
+    // bindings.
+    $compileProvider.preAssignBindingsEnabled ? $compileProvider.preAssignBindingsEnabled() : true


Are we sure the $compileProvider.preAssignBindingsEnabled() output is set in stone at this point?

Also, note that AngularJS 1.7 will remove $compileProvider.preAssignBindingsEnabled and the bindings will stop being pre-assigned so this code is not future-proof.

If the method is not present, it's safer to check the library version and set true only if it's older than 1.6.

Are we sure the $compileProvider.preAssignBindingsEnabled() output is set in stone at this point?

As a matter of fact, it is never set in stone 😁 (you can change it any time). Not that this is a common usecase.

.provider('$$mdPreAssignBindings', function ($compileProvider) { var hardcodedValue = (angular.version.major === 1 && angular.version.minor < 6) ? true : false; var getValue = $compileProvider.preAssignBindingsEnabled ? function () { $compileProvider.preAssignBindingsEnabled(); } : function () { return hardcodedValue; }; this.$get = function () { return getValue; }; })

@gkalpak your implementation is good, I'd just drop the ? true : false part as it's redundant here.

I do this true/false thing too often 😃

gkalpak

🎉 💯

gkalpak · 2017-03-08T00:08:25Z

src/core/services/compiler/compiler.js

+  $provide.constant('$$mdPreAssignBindings',
+    // For Angular versions that do not support `preAssignBindingsEnabled`, always pre-assign the
+    // bindings.
+    $compileProvider.preAssignBindingsEnabled ? $compileProvider.preAssignBindingsEnabled() : true


Are we sure the $compileProvider.preAssignBindingsEnabled() output is set in stone at this point?

As a matter of fact, it is never set in stone 😁 (you can change it any time). Not that this is a common usecase.

.provider('$$mdPreAssignBindings', function ($compileProvider) { var hardcodedValue = (angular.version.major === 1 && angular.version.minor < 6) ? true : false; var getValue = $compileProvider.preAssignBindingsEnabled ? function () { $compileProvider.preAssignBindingsEnabled(); } : function () { return hardcodedValue; }; this.$get = function () { return getValue; }; })

gkalpak · 2017-03-08T00:15:39Z

src/core/services/compiler/compiler.js

-
-      var ctrl = invokeCtrl();
+      // Create the specified controller instance.
+      var ctrl = self._createController(options, injectLocals);


Not directly related to this PR, but it seems that you are accidentally putting $element into locals. Essentially, locals and injectLocals are the same object, but this doesn't seem intentional (and indeed assigning $element to the controller instance seems strange).

Was it intentional?

Good point.

gkalpak · 2017-03-08T00:17:07Z

src/core/services/compiler/compiler.spec.js

+        }
+
+        TestController.prototype.$onInit = function() {
+          expect(this.name).toBe('Bob');


There is no verification that $onInit() is actually called.

What do you mean.

For example, for the constructor you check that called has been set to true, so you know that the constructor has been called (and the expectations it contains verified).
There is nothing similar for $onInit().

You could add expect('foo').toBe('bar') inside $onInit() and the test would not fail if $onInit() is never called. This means that the test may not catch some future regression.

That's why I like QUnit's assert.expect(number) so much but Jasmine has no equivalent.

A proper test here should also probably spy on $onInit (with calledThrough()) to ensure it's not skipped.

mgol

There are some issues to resolve indicated in comments. Otherwise, A good idea to abstract the $onInit logic until support for AngularJS <1.5.10 is dropped.

devversion · 2017-03-08T17:32:36Z

@gkalpak @mgol @ThomasBurleson Updated the PR.

ThomasBurleson · 2017-03-08T17:58:06Z

src/core/services/compiler/compiler.spec.js

+    });
+
+    it('disabled should assigns bindings after constructor', function() {
+      configureCompiler(false);


let's use:

var shouldPreAssign = false; var isIntantiated = false; configureCompiler(shouldPreAssign);

What advantage does this have? I think the spec name and function parameter already explain that this changes the preAssignBindings behavior.

I had to search for how it was disabled. the configureCompiler(false) was not intuitive.

Ah I understand. How is setPreAssignBindings?

ThomasBurleson · 2017-03-09T04:39:30Z

@devversion - please squash your commits.

gkalpak · 2017-03-09T08:52:22Z

src/core/services/compiler/compiler.js

+  var hasPreAssignState = !!$compileProvider.preAssignBindingsEnabled;
+
+  this.$get = function() {
+    return hasPreAssignState ? $compileProvider.preAssignBindingsEnabled() : fallbackValue;


This will not catch changes that will happend at runtime (which is theoretically possible, although highly improbable).

Yeah I intentionally did it that way. If users change it at runtime then I don't think this should be respected. Providers should be set at config phase.

gkalpak · 2017-03-09T08:54:00Z

src/core/services/compiler/compiler.spec.js

+  function setPreAssignBindings(preAssignBindingsEnabled) {
+    module(function($compileProvider) {
+      $compileProvider.preAssignBindingsEnabled(preAssignBindingsEnabled);
+    })


Missing semicolon 😱

gkalpak · 2017-03-09T08:54:58Z

src/core/services/compiler/compiler.js

-
-      var ctrl = invokeCtrl();
+      // Create the specified controller instance.
+      var ctrl = self._createController(options, injectLocals, locals);


Currently, injectLocals and locals are the same object. I assume this a bug, but not directly related to (or introduced by) this PR.

In what way are those similar? injectLocals contains the $element DI token.

In the way that angular.extend(locals, {$element: ...}) extends locals with $element and then returns locals. So, injectLocals === locals. I assume the intended behavior was injectLocals = angular.extend({$element: ...}, locals) or injectLocals = angular.extend({}, locals, {$element: ...}).

gkalpak · 2017-03-09T08:56:06Z

src/core/services/compiler/compiler.spec.js

+      return data;
+    }
+
+    it('enabled should assigns bindings at instantiation', function() {


assigns --> assign

gkalpak · 2017-03-09T08:56:59Z

src/core/services/compiler/compiler.spec.js

+      expect(isInstantiated).toBe(true);
+    });
+
+    it('disabled should assigns bindings after constructor', function() {


assigns --> assign

ThomasBurleson · 2017-03-09T12:34:37Z

@devversion - please squash your commits and rebase from master.

devversion · 2017-03-09T15:54:23Z

@ThomasBurleson Done.

mgol · 2017-03-09T16:24:05Z

@devversion did you see my comment regarding the Karma update? This one: #10469 (comment).

ThomasBurleson · 2017-03-10T22:17:18Z

@mgol, @gkalpak - last review before presubmit ?

devversion · 2017-03-10T22:54:55Z

I just saw that I need to make one adjustment for the tests. Ignore the failing test for review.

mgol · 2017-03-10T23:07:08Z

src/core/services/compiler/compiler.js

+function PreAssignBindingsProvider($compileProvider) {
+  // To avoid breaking changes in Material, AngularJS Material will only respect the
+  // preAssignBindingsEnabled state in AngularJS 1.6.X and higher.
+  var respectPreAssignState = angular.version.major === 1 && angular.version.minor > 5;


This will break with AngularJS ~1.5.10 if the developer sets the preAssignBindingsEnabled flag to false.

What are the breaking changes mentioned in a comment?

Made some changes to it. See updated PR

mgol · 2017-03-10T23:07:37Z

src/core/services/compiler/compiler.js

+  var respectPreAssignState = angular.version.major === 1 && angular.version.minor > 5;
+
+  this.$get = function() {
+    return respectPreAssignState ? $compileProvider.preAssignBindingsEnabled() : true;


This will break in AngularJS 1.7+. We had a thread about it with @gkalpak suggesting an implementation that would work, what did happen with that? :)

The implementation did not cover the changes we discussed in our meeting. Also we don't want to re-check the preAssignBindingsEnabled state for each $mdCompiler call (as it returned a getValue function)

I changed it now to work as we discussed on our meeting

1.5: Bindings will be always pre-assigned as same as with Material before.

1.6: Angular Material will start respecting the preAssignBindingsEnabled state

1.7: Compiler will always sets preAssignBindingsEnabled to false.

* The `$mdCompiler` now respects the `preAssignBindingsEnabled` state when using AngularJS 1.6 or higher. **BREAKING CHANGE** Developers that are upgrading from AngularJS 1.5 to AngularJS 1.6 may run into issues with their custom controllers. All custom controllers that are are being used inside of Material components, like in the `$mdDialog` or `$mdToast`, can be affected. > Limited to controllers that are used in combination with the `bindToController` and `locals` option. Angular Material starts respecting the [`preAssignBindingsEnabled`](angular/angular.js#15352) state when using AngularJS 1.6+ This will mean that bindings are no longer initialized at `constructor` time. ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` To provide consistency, Angular Material components with custom controllers now work with the `$onInit` lifecycle hook (**no** other lifecycle hooks) Fixes angular#10016

gkalpak · 2017-03-11T12:29:09Z

src/core/services/compiler/compiler.js

+  // To avoid that the preAssignBindings state causes issues when upgrading from AngularJS 1.5 to
+  // AngularJS 1.6, AngularJS Material will only respect the preAssignBindingsEnabled state in 1.6
+  var respectPreAssignState = angular.version.major === 1 && angular.version.minor === 6;
+  var fallbackValue = !!$compileProvider.preAssignBindingsEnabled;


As @mgol has pointed out, this will break in the future (most probably 1.7+). The plan is to remove the preAssingBindingsEnabled method and not support pre-assignement neither by default, nor as an opt-in (this is the PR btw: angular/angular.js#15782).

Base on what we have discussed in the last meeting, you need something like this:

var defaultPreAssignValue = angular.version.major === 1 && angular.version.minor < 6; var respectPreAssignState = angular.version.major === 1 && angular.version.minor > 5 && $compileProvider.preAssignBindingsEnabled; this.$get = function() { return respectPreAssingState ? $compileProvider.preAssignBindingsEnabled() : defaultPreAssignState; };

BTW, the current implementation will also break in 1.5.0-1.5.9 (where there is no $compileProvider.preAssignBindingsEnabled method).

@gkalpak

BTW, the current implementation will also break in 1.5.0-1.5.9 (where there is no $compileProvider.preAssignBindingsEnabled method).

How so? fallbackValue will then be false but it won't be used anyway.

fallbackValue will be false and it will be used. respectPreAssignState would be false, so the (incorrect) fallback value will be used.

_{(To be clear, by "current implementation" I mean the implementation currently used in this PR.)}

Well, I thought that preAssignBindingsEnabled has been added in 1.5.X and changed in 1.6 to false by default and will be removed in 1.7.

@gkalpak Ah, you're right.

@devversion The flag has been added in 1.5.9 which was released after a long wait and even after 1.6.0 was released. It's highly likely most of 1.5.x users will not have this flag available.

The fallback value should be defined based on AngularJS version, not the presence of the $compileProvider.preAssignBindingsEnabled flag.

Besides, as far as I understand, Material still supports AngularJS 1.3+, doesn't it?

mgol · 2017-03-11T12:49:02Z

src/core/services/compiler/compiler.js

+function PreAssignBindingsProvider($compileProvider) {
+  // To avoid that the preAssignBindings state causes issues when upgrading from AngularJS 1.5 to
+  // AngularJS 1.6, AngularJS Material will only respect the preAssignBindingsEnabled state in 1.6
+  var respectPreAssignState = angular.version.major === 1 && angular.version.minor === 6;


About not respecting the flag in 1.5: it's about how MdCompilerService#_createController behaves so it will not break 1.5, it will just still preassign bindings in 1.5.10 even if the developer sets preAssignBindingsEnabled to false, I see it now.

We could have respected the flag in 1.5.10 if your PR from a while ago accounting for this flag wasn't merged before this one as it wouldn't be breaking then - no one would be using 1.5.10 with Material & this flag set to false as this configuration was broken.

I assume Material needs to ignore this flag because Material 1.1.3 works fine in 1.5.10 with this flag set to false? (Does it?)

BTW, just in case we'd have to revert the removal and defer it to 1.8, it would be safer to check for angular.version.minor > 5 instead of angular.version.minor === 6

BTW, regarding the argument for not respecting the flag in ~1.5.10 but respecting in ~1.6 - if someone was using Material with AngularJS 1.6 and it worked for them as the components they used worked fine, this change may break them.

Maybe it would be safer to release this as 1.2.0, not 1.1.5 and then not ignoring the flag if it exists?

What do you think? @devversion @gkalpak @ThomasBurleson

if someone was using Material with AngularJS 1.6 and it worked for them as the components they used worked fine, this change may break them.

🤢 True.
It sounds like there is indeed no (reaconable) way to make this non-breaking (without making it opt-in - e.g. with an mdCompiler flag) 😞

mgol · 2017-03-11T12:49:45Z

src/core/services/compiler/compiler.js

+function PreAssignBindingsProvider($compileProvider) {
+  // To avoid that the preAssignBindings state causes issues when upgrading from AngularJS 1.5 to
+  // AngularJS 1.6, AngularJS Material will only respect the preAssignBindingsEnabled state in 1.6
+  var respectPreAssignState = angular.version.major === 1 && angular.version.minor === 6;


BTW, just in case we'd have to revert the removal and defer it to 1.8, it would be safer to check for angular.version.minor > 5 instead of angular.version.minor === 6

jelbourn · 2017-03-16T21:22:49Z

@mmalerba can you presubmit this (but don't merge it if it passes)?

mmalerba · 2017-03-18T01:51:52Z

@jelbourn latest is good, v1_1 has ~10 failures (this is with merging this PR into master at the commit v1_1 is currently synced to)

mgol · 2017-03-27T19:41:57Z

Since there are edge cases that would break when this PR lands in its current state, I think we could do the following to not have to wait for Material 1.2 with landing it:

Add a new public flag (sth like $mdCompilerProvider.respectPreAssignBindingsState(true|false)) that would make Material respect the value of AngularJS' $compilerProvider.preAssignBindingsEnabled().
Make this flag false by default, turn it to true by default in 1.2.0, remove in 1.3.0. Optionally, the flag could be made a noop in Material 1.2.0 (and removed in 1.3.0) if we didn't want to keep this logic that long.
Change the logic in the current PR to:
1. If $mdCompilerProvider.respectPreAssignBindingsState() === false: always preassign bindings.
2. If $mdCompilerProvider.respectPreAssignBindingsState() === true:
  1. If $compilerProvider.preAssignBindingsEnabled exist, preassign bindings if and only if it returns true.
  2. If it doesn't exist then:
    1. With AngularJS <1.6: preassign bindings.
    2. With AngularJS >=1.6: don't preassign bindings.

What do you think?

ThomasBurleson · 2017-05-22T22:20:54Z

@mmalerba - Can you elaborate. I admit that I do not understand your response. Is this PR invalid for a presubmit? If yes, then why ?

ThomasBurleson · 2017-05-22T22:36:32Z

@mgol - let's talk about your ideas offline in a call. Thx

mmalerba · 2017-05-23T02:09:30Z

@ThomasBurleson there were 10 failures when I checked, which is a relatively small number and may include some that were just flakes. It can probably be merged with some shepherding from the caretaker

@devversion

1. The `$mdCompiler` is now able to respect the `preAssignBindingsEnabled` state when using AngularJS 1.5.10 or higher; to enable, invoke: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); 2. `$mdCompiler` now understands the `$onInit` lifecycle hooks in controllers. Note that no other AngularJS 1.5+ lifecycle hooks are supported currently. Invoking: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); will make bindings in Material custom components like `$mdDialog` or `$mdToast` only available in controller constructors if they are available in constructors of all other AngularJS controllers. By default this will happen in AngularJS 1.6 or newer. This can also be achieved with AngularJS >=1.5.10 <1.6.0 if: $compilerProvider.preAssignBindingsEnabled(false); is invoked. Example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` Huge thanks to Paul Gschwendtner (@devversion) for the initial draft version of this feature. Fixes angular#10016 Closes angular#10469

mgol · 2017-06-08T15:53:38Z

This PR evolved into PR #10726; PTAL.

devversion · 2017-06-08T15:54:40Z

@mgol Thanks for continuing this. Closing my PR here.

@devversion

1. The `$mdCompiler` is now able to respect the `preAssignBindingsEnabled` state when using AngularJS 1.5.10 or higher; to enable, invoke: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); 2. `$mdCompiler` now understands the `$onInit` lifecycle hooks in controllers. Note that no other AngularJS 1.5+ lifecycle hooks are supported currently. Invoking: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); will make bindings in Material custom components like `$mdDialog` or `$mdToast` only available in controller constructors if they are available in constructors of all other AngularJS controllers. By default this will happen in AngularJS 1.6 or newer. This can also be achieved with AngularJS >=1.5.10 <1.6.0 if: $compilerProvider.preAssignBindingsEnabled(false); is invoked. Example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` Huge thanks to Paul Gschwendtner (@devversion) for the initial draft version of this feature. Fixes angular#10016 Ref angular#10469 Closes angular#10726

@devversion

1. The `$mdCompiler` is now able to respect the `preAssignBindingsEnabled` state when using AngularJS 1.5.10 or higher; to enable, invoke: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); 2. `$mdCompiler` now understands the `$onInit` lifecycle hooks in controllers. Note that no other AngularJS 1.5+ lifecycle hooks are supported currently. Invoking: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); will make bindings in Material custom components like `$mdDialog` or `$mdToast` only available in controller constructors if they are available in constructors of all other AngularJS controllers. By default this will happen in AngularJS 1.6 or newer. This can also be achieved with AngularJS >=1.5.10 <1.6.0 if: $compilerProvider.preAssignBindingsEnabled(false); is invoked. Example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` Huge thanks to Paul Gschwendtner (@devversion) for the initial draft version of this feature. Fixes angular#10016 Ref angular#10469 Closes angular#10726

@devversion

1. The `$mdCompiler` is now able to respect the `preAssignBindingsEnabled` state when using AngularJS 1.5.10 or higher; to enable, invoke: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); 2. `$mdCompiler` now understands the `$onInit` lifecycle hooks in controllers. Note that no other AngularJS 1.5+ lifecycle hooks are supported currently. Invoking: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); will make bindings in Material custom components like `$mdDialog` or `$mdToast` only available in controller constructors if they are available in constructors of all other AngularJS controllers. By default this will happen in AngularJS 1.6 or newer. This can also be achieved with AngularJS >=1.5.10 <1.6.0 if: $compilerProvider.preAssignBindingsEnabled(false); is invoked. Example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` Huge thanks to Paul Gschwendtner (@devversion) for the initial draft version of this feature. Fixes angular#10016 Ref angular#10469 Closes angular#10726

@devversion

1. The `$mdCompiler` is now able to respect the `preAssignBindingsEnabled` state when using AngularJS 1.5.10 or higher; to enable, invoke: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); 2. `$mdCompiler` now understands the `$onInit` lifecycle hooks in controllers. Note that no other AngularJS 1.5+ lifecycle hooks are supported currently. Invoking: $mdCompilerProvider.respectPreAssignBindingsEnabled(true); will make bindings in Material custom components like `$mdDialog` or `$mdToast` only available in controller constructors if they are available in constructors of all other AngularJS controllers. By default this will happen in AngularJS 1.6 or newer. This can also be achieved with AngularJS >=1.5.10 <1.6.0 if: $compilerProvider.preAssignBindingsEnabled(false); is invoked. Example: ```js $mdDialog.show({ locals: { myVar: true }, controller: MyController, bindToController: true } function MyController() { // No locals from Angular Material are set yet. e.g myVar is undefined. } MyController.prototype.$onInit = function() { // Bindings are now set in the $onInit lifecycle hook. } ``` Huge thanks to Paul Gschwendtner (@devversion) for the initial draft version of this feature. Fixes #10016 Ref #10469 Closes #10726

devversion added the needs: review This PR is waiting on review from the team label Mar 7, 2017

devversion assigned ThomasBurleson Mar 7, 2017

devversion requested a review from ThomasBurleson March 7, 2017 19:14

googlebot added cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ labels Mar 7, 2017

ThomasBurleson added this to the 1.1.4 milestone Mar 7, 2017

ThomasBurleson added P0 - Presubmit First in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: review This PR is waiting on review from the team and removed needs: review This PR is waiting on review from the team P0 - Presubmit First labels Mar 7, 2017

mgol suggested changes Mar 7, 2017

View reviewed changes

gkalpak reviewed Mar 8, 2017

View reviewed changes

mgol suggested changes Mar 8, 2017

View reviewed changes

devversion removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Mar 8, 2017

ThomasBurleson reviewed Mar 8, 2017

View reviewed changes

ThomasBurleson added needs: presubmit and removed needs: review This PR is waiting on review from the team labels Mar 9, 2017

ThomasBurleson added the P0 - Presubmit First label Mar 9, 2017

gkalpak reviewed Mar 9, 2017

View reviewed changes

devversion force-pushed the fix/compiler-pre-assign branch from 295141e to d85d4d2 Compare March 9, 2017 15:37

ThomasBurleson removed needs: presubmit labels Mar 9, 2017

mgol suggested changes Mar 10, 2017

View reviewed changes

devversion force-pushed the fix/compiler-pre-assign branch from 203c380 to 3b6f4a2 Compare March 11, 2017 11:05

gkalpak suggested changes Mar 11, 2017

View reviewed changes

mgol reviewed Mar 11, 2017

View reviewed changes

mgol suggested changes Mar 11, 2017

View reviewed changes

devversion added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Mar 17, 2017

ThomasBurleson added needs: presubmit and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels May 8, 2017

ThomasBurleson added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed needs: presubmit labels May 25, 2017

ThomasBurleson assigned mgol and unassigned ThomasBurleson May 25, 2017

mgol mentioned this pull request Jun 8, 2017

feat(compiler): respect preAssignBindingsEnabled state #10726

Merged

devversion closed this Jun 8, 2017

fix(compiler): respect preAssignBindingsEnabled state #10469

fix(compiler): respect preAssignBindingsEnabled state #10469

Conversation

devversion commented Mar 7, 2017 • edited

ThomasBurleson commented Mar 7, 2017

mgol commented Mar 7, 2017

mgol Mar 7, 2017 • edited

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

gkalpak left a 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

mgol left a comment

Choose a reason for hiding this comment

devversion commented Mar 8, 2017

Choose a reason for hiding this comment

devversion Mar 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomasBurleson commented Mar 9, 2017

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

gkalpak Mar 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomasBurleson commented Mar 9, 2017 • edited

devversion commented Mar 9, 2017

mgol commented Mar 9, 2017 • edited

ThomasBurleson commented Mar 10, 2017

devversion commented Mar 10, 2017

Choose a reason for hiding this comment

devversion Mar 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devversion Mar 11, 2017 • edited

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

mgol Mar 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelbourn commented Mar 16, 2017

mmalerba commented Mar 18, 2017

mgol commented Mar 27, 2017 • edited

ThomasBurleson commented May 22, 2017

ThomasBurleson commented May 22, 2017

mmalerba commented May 23, 2017

mgol commented Jun 8, 2017

devversion commented Jun 8, 2017

devversion commented Mar 7, 2017 •

edited

mgol Mar 7, 2017 •

edited

devversion Mar 8, 2017 •

edited

gkalpak Mar 9, 2017 •

edited

ThomasBurleson commented Mar 9, 2017 •

edited

mgol commented Mar 9, 2017 •

edited

devversion Mar 11, 2017 •

edited

devversion Mar 11, 2017 •

edited

mgol Mar 11, 2017 •

edited

mgol commented Mar 27, 2017 •

edited