fix($compile): remove the preAssignBindingsEnabled flag #15782
Conversation
|
Can you please add to the commit message a short explanation of what preAssignBindingsEnabled did? |
mgol
changed the title
|
@Narretz PR updated; how does it look now? (I only changed the commit message) |
|
@mgol As I was reading through this I was wondering if this a typo:
Doesn't this have to be: |
|
Nice catch, I'll fix it later.
|
|
Can you add |
|
Will do. |
|
Commit message updated, PTAL. |
mgol
force-pushed
the
pre-assign-removal
branch
from
0ff1310 to
2249b30
Compare
There was a problem hiding this comment.
The code LGTM.
I believe we could simplify things a little more in $compile (e.g. by not using the "lazy" path for controller instantiation). And potentially drop that path in $controller altogether.
But this doesn't have to be part of this PR.
WRT the commit message:
-
It helps (when creating the changelog) to wrap code in backticks (e.g.
$compile.preAssignBindingsEnabledand$onInitin the first paragraph). -
In (2)
$compileProvider.preAssignBindingsEnabled(true)should usefalse, nottrue.
BREAKING CHANGE: Previously, the `$compileProvider.preAssignBindingsEnabled` flag was supported. The flag controlled whether bindings were available inside the controller constructor or only in the `$onInit` hook. The bindings are now no longer available in the constructor. To migrate your code: 1. If you haven't invoked `$compileProvider.preAssignBindingsEnabled()` you don't have to do anything to migrate. 2. If you specified `$compileProvider.preAssignBindingsEnabled(false)`, you can remove that statement - since AngularJS 1.6.0 this is the default so your app should still work even in AngularJS 1.6 after such removal. Afterwards, migrating to AngularJS 1.7.0 shouldn't require any further action. 3. If you specified `$compileProvider.preAssignBindingsEnabled(true)` you need to first migrate your code so that the flag can be flipped to `false`. The instructions on how to do that are available in the "Migrating from 1.5 to 1.6" guide: https://docs.angularjs.org/guide/migration#migrating-from-1-5-to-1-6 Afterwards, remove the `$compileProvider.preAssignBindingsEnabled(true)` statement.
mgol
force-pushed
the
pre-assign-removal
branch
from
2249b30 to
cc788d5
Compare
|
@gkalpak Yeah, this PR is already big due to all the indentation changes so I didn't want to introduce logic changes at this point. I'll happily look into it further once this lands (any regressions in such updates will be easier to revert). Commit message updated. |
|
LGTM |
1 similar comment
|
LGTM |
|
There is a typo in the PR description - bullet 1's |
|
Good catch! Thx for pointing it out. |
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed.
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed.
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed.
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed.
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed.
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed. One difference with the previous implementation is that all non-ES2015-class controller instances were available on the element before calling their constructors. Now it depends on the relative order of controllers. Controller constructors shouldn't be used to access other controllers (e.g. via `$element.controller(directiveName)`). The recommended way is to use the `require` property of the directive definition object and the life cycle hooks `$onChanges` or `$onInit`. See https://docs.angularjs.org/api/ng/service/$compile#-require- https://docs.angularjs.org/api/ng/service/$compile#life-cycle-hooks
Now that we don't need to support `preAssignBindingsEnabled` (removed in angular#15782), complexity introduced in `$controller` by angular#7645 can be removed. One difference with the previous implementation is that all non-ES2015-class controller instances were available on the element before calling their constructors. Now it depends on the relative order of controllers. Controller constructors shouldn't be used to access other controllers (e.g. via `$element.controller(directiveName)`). The recommended way is to use the `require` property of the directive definition object and the life cycle hooks `$onChanges` or `$onInit`. See https://docs.angularjs.org/api/ng/service/$compile#-require- https://docs.angularjs.org/api/ng/service/$compile#life-cycle-hooks
Now that we don't need to support `preAssignBindingsEnabled` (removed in #15782), complexity introduced in `$controller` by #7645 can be removed. One difference with the previous implementation is that all non-ES2015-class controller instances were available on the element before calling their constructors. Now it depends on the relative order of controllers. Controller constructors shouldn't be used to access other controllers (e.g. via `$element.controller(directiveName)`). The recommended way is to use the `require` property of the directive definition object and the life cycle hooks `$onChanges` or `$onInit`. See https://docs.angularjs.org/api/ng/service/$compile#-require- https://docs.angularjs.org/api/ng/service/$compile#life-cycle-hooks Closes #16580
Now that we don't need to support `preAssignBindingsEnabled` (removed in #15782), complexity introduced in `$controller` by #7645 can be removed. One difference with the previous implementation is that all non-ES2015-class controller instances were available on the element before calling their constructors. Now it depends on the relative order of controllers. Controller constructors shouldn't be used to access other controllers (e.g. via `$element.controller(directiveName)`). The recommended way is to use the `require` property of the directive definition object and the life cycle hooks `$onChanges` or `$onInit`. See https://docs.angularjs.org/api/ng/service/$compile#-require- https://docs.angularjs.org/api/ng/service/$compile#life-cycle-hooks Closes #16580
BREAKING CHANGE:
Previously, the $compileProvider.preAssignBindingsEnabled flag was supported.
To migrate your code:
If you specified
$compileProvider.preAssignBindingsEnabled(false), youcan remove that statement - since AngularJS 1.6.0 this is the default so your
app should still work even in AngularJS 1.6 after such removal. Afterwards,
migrating to AngularJS 1.7.0 shouldn't require any further action.
If you specified
$compileProvider.preAssignBindingsEnabled(true), firstmigrate your code so that the flag can be flipped to
false. The instructionson how to do that are available in the "Migrating from 1.5 to 1.6" guide:
https://docs.angularjs.org/guide/migration#migrating-from-1-5-to-1-6
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature removal.
What is the current behavior? (You can also link to an open issue here)
The
$compileProvider.preAssignBindingsEnabledflag is supported.What is the new behavior (if this is a feature change)?
The
$compileProvider.preAssignBindingsEnabledflag doesn't exist.Does this PR introduce a breaking change?
Yes.
Please check if the PR fulfills these requirements
Tests for the changes have been added (for bug fixes / features)Docs have been added / updated (for bug fixes / features)Other information:
It is strongly advised to view this pull request with
?w=1in the query string i.e. at https://github.com/angular/angular.js/pull/15782/files?w=1. Unfortunately, it's impossible to insert comments in this view.