`$onInit` and friends #13763

petebacondarwin · 2016-01-13T17:43:20Z

No description provided.

gkalpak · 2016-01-13T19:04:06Z

src/ng/compile.js

+ * * a **string** containing the name of the directive to pass to the linking function
+ * * an **array** containing the names of directives to pass to the linking function. The argument passed to the
+ * linking function will be an array of controllers in the same order as the names in the `require` property
+ * * an **object** whose property values are the names of the directibes to pass to the linking function. The argument


directibes --> directives

gkalpak · 2016-01-13T19:35:53Z

With the exception of #13763 (comment) (and a couple of nitpicks), this L(pretty)GTM 👍

lgalfaso · 2016-01-14T11:09:29Z

test/ng/compileSpec.js

@@ -4244,6 +4244,22 @@ describe('$compile', function() {
      if (!/chrome/i.test(navigator.userAgent)) return;


unrelated to this issue, but can we change this to if (!support.classes) return; ?

On it [sic]!

Actually, it is not so easy @lgalfaso since the latest Safari (9.0.2) is even worse for toString on a a class instance.

it looks like there are at least 2 issues reported on JavascriptCore (Safari JS engine) regarding this
https://bugs.webkit.org/show_bug.cgi?id=149743
https://bugs.webkit.org/show_bug.cgi?id=144285

I think that some other strategy. Meanwhile, just keep the old code

…oller construction This enables option three of angular#13510 (comment) by allowing the creator of directive controllers using ES6 classes to have a hook that is called when the bindings are definitely available. Moreover this will help solve the problem of accessing `require`d controllers from controller instances without resorting to wiring up in a `link` function. See angular#5893

This provides an elegant alternative to the array form of the `require` property but also helps to support binding of `require`d controllers to directive controllers.

…ve controller If directives are required through an object hash, rather than a string or array, the required directives' controllers are bound to the current directive's controller in much the same way as the properties are bound to using `bindToController`. The binding is done after the controller has been constructed and all the bindings are guaranteed to be complete by the time the controller's `$onInit` method is called. This change makes it much simpler to access require controllers without the need for manually wiring them up in link functions. In particular this enables support for `require` in directives defined using `mod.component()`

This provides an elegant alternative to the array form of the `require` property but also helps to support binding of `require`d controllers to directive controllers.

…inding require

…ve controller If directives are required through an object hash, rather than a string or array, the required directives' controllers are bound to the current directive's controller in much the same way as the properties are bound to using `bindToController`. The binding is done after the controller has been constructed and all the bindings are guaranteed to be complete by the time the controller's `$onInit` method is called. This change makes it much simpler to access require controllers without the need for manually wiring them up in link functions. In particular this enables support for `require` in directives defined using `mod.component()`

…oller construction This enables option three of angular#13510 (comment) by allowing the creator of directive controllers using ES6 classes to have a hook that is called when the bindings are definitely available. Moreover this will help solve the problem of accessing `require`d controllers from controller instances without resorting to wiring up in a `link` function. See angular#5893

…ve controller If directives are required through an object hash, rather than a string or array, the required directives' controllers are bound to the current directive's controller in much the same way as the properties are bound to using `bindToController`. The binding is done after the controller has been constructed and all the bindings are guaranteed to be complete by the time the controller's `$onInit` method is called. This change makes it much simpler to access require controllers without the need for manually wiring them up in link functions. In particular this enables support for `require` in directives defined using `mod.component()`

Narretz · 2016-01-14T15:04:33Z

src/ng/compile.js

+ *
+ * If the `require` property is an object and the directive provides a controller, then the required controllers are
+ * bound to the controller using the keys of the `require` property. See the {@link $compileProvider#component} helper
+ * for an example of how this can be used.


Should note that the controllers are only available once $onInit has been called, no?

Narretz · 2016-01-14T15:11:29Z

Now that require can be an object, should we pass the ctrls as an object to the link function like described here: #8401? (Yes from me)

Narretz · 2016-01-14T15:19:14Z

Don't forget that these issues are fixed by the PR:
#6040
#5893

petebacondarwin · 2016-01-14T21:36:30Z

Now that require can be an object, should we pass the ctrls as an object to the link function like described here: #8401? (Yes from me)

@Narretz One of the commits in this PR does actually do that - but only when the require property is an object itself.

…required correctly See angular#13763 (comment)

gkalpak · 2016-01-15T11:40:18Z

src/ng/compile.js

- * is instantiated, the initial values of the isolate scope bindings will be available if the controller is not an ES6 class.
+ * allow a component to have its properties bound to the controller, rather than to scope.
+ *
+ * After the controller is instantiated, the initial values of the isolate scope bindings will bound to the controller


will bound --> will be? bound

…s truthy

gkalpak · 2016-01-15T11:56:15Z

test/ng/compileSpec.js

@@ -4280,6 +4283,7 @@ describe('$compile', function() {
        element = $compile('<div foo-dir dir-data="remoteData" ' +
                                 'dir-str="Hello, {{whom}}!" ' +
                                 'dir-fn="fn()"></div>')($rootScope);
+        expect(Controller.prototype.$onInit).toHaveBeenCalled();
        element.data('$fooDirController').check();


Isn't this redundant now ?

yep! removed

Narretz · 2016-01-15T11:57:27Z

I just noticed the component method doesn't accept the templateNamespace / multiElement property, is this intentional?

gkalpak · 2016-01-15T11:59:54Z

test/ng/compileSpec.js

+            require: { container: '^parent', friend: 'sibling' },
+            bindToController: true,
+            controller: MeController,
+            controllerAs: '$ctrl'


Two controllerAs occurrences (where 1 would be sufficient).

…s truthy

…required correctly See angular#13763 (comment)

…s truthy

petebacondarwin · 2016-01-15T12:32:28Z

I just noticed the component method doesn't accept the templateNamespace / multiElement property, is this intentional?

@Narretz - I think we should try to keep the component API as simple as possible. These two are fairly uncommon, right? If people really need them they can complain and we can add them later.

Narretz · 2016-01-15T13:19:49Z

@petebacondarwin Sounds good

Narretz · 2016-01-15T13:25:38Z

There's a real travis failure btw:

Chrome 45.0.2454 (Linux 0.0.0) $compile isolated locals should eventually expose isolate scope variables on ES6 class controller with controllerAs when bindToController is true FAILED

Although the test wasn't introduced by this PR.

petebacondarwin · 2016-01-15T16:00:02Z

@Narretz I have fixed that failing test.

ev45ive · 2016-01-17T21:49:24Z

This looks very nice and clean. Should be like this from beggining. Very nice work. Do you think $onInit with bound required controllers and component() syntax will get on board with 1.5 release? I'd love to use it in production - it would clean up a lot of messy directive code. :-)

…required correctly See angular#13763 (comment)

This provides an elegant alternative to the array form of the `require` property but also helps to support binding of `require`d controllers to directive controllers. Closes #8401 Closes #13763

…ve controller If directives are required through an object hash, rather than a string or array, the required directives' controllers are bound to the current directive's controller in much the same way as the properties are bound to using `bindToController`. This only happens if `bindToController` is truthy. The binding is done after the controller has been constructed and all the bindings are guaranteed to be complete by the time the controller's `$onInit` method is called. This change makes it much simpler to access require controllers without the need for manually wiring them up in link functions. In particular this enables support for `require` in directives defined using `mod.component()` Closes #6040 Closes #5893 Closes #13763

usernamealreadyis · 2016-01-24T10:10:18Z

@Narretz I have fixed that failing test.

that's the point.... fixes all fail...

petebacondarwin added type: feature component: $compile needs: review branch: 1.5.x labels Jan 13, 2016

petebacondarwin added this to the 1.5.x - migration-facilitation milestone Jan 13, 2016

googlebot added the cla: yes label Jan 13, 2016

gkalpak reviewed Jan 13, 2016
View reviewed changes

lgalfaso reviewed Jan 14, 2016
View reviewed changes

petebacondarwin added 10 commits January 14, 2016 11:56

test($compile): check that $onInit is called correctly for ES6 classes

cf4e7be

docs($compile): document the new $onInit controller hook

452dd39

feat($compile): allow require to be an object

f72ecbd

This provides an elegant alternative to the array form of the `require` property but also helps to support binding of `require`d controllers to directive controllers.

feat($compile): allow require to be an object

37793ce

This provides an elegant alternative to the array form of the `require` property but also helps to support binding of `require`d controllers to directive controllers.

test($compile): check explicit return controllers are not broken by b…

a12ddc6

…inding require

petebacondarwin force-pushed the onInit branch from 620baa7 to 9e6db1a Compare January 14, 2016 11:59

Narretz reviewed Jan 14, 2016
View reviewed changes

fix($compile): ensure controllers with return value constructors are …

1d18df2

…required correctly See angular#13763 (comment)

gkalpak reviewed Jan 15, 2016
View reviewed changes

fix($compile): only bind required controllers if bindToController i…

cae40ca

…s truthy

petebacondarwin force-pushed the onInit branch from 98dc205 to cae40ca Compare January 15, 2016 11:54

gkalpak reviewed Jan 15, 2016
View reviewed changes

petebacondarwin added 6 commits January 15, 2016 12:25

docs($compile): fix typo

276b9ee

fix($compile): only bind required controllers if bindToController i…

6d66a75

…s truthy

test($compile): check that $onInit is called correctly for ES6 classes

32b7da3

fix($compile): only bind required controllers if bindToController i…

cb495a5

…s truthy

fix($compile): ensure controllers with return value constructors are …

270e230

…required correctly See angular#13763 (comment)

fix($compile): only bind required controllers if bindToController i…

281d987

…s truthy

test($compile): check that $onInit is called correctly for ES6 classes

9f76a11

Narretz force-pushed the master branch 2 times, most recently from 70049ae to b77e14b Compare January 16, 2016 21:59

Narretz pushed a commit to Narretz/angular.js that referenced this pull request Jan 19, 2016

fix($compile): ensure controllers with return value constructors are …

02f1a83

…required correctly See angular#13763 (comment)

petebacondarwin closed this in 3ffdf38 Jan 19, 2016

petebacondarwin deleted the onInit branch November 24, 2016 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`$onInit` and friends #13763

`$onInit` and friends #13763

petebacondarwin commented Jan 13, 2016

gkalpak Jan 13, 2016

gkalpak commented Jan 13, 2016

lgalfaso Jan 14, 2016

petebacondarwin Jan 14, 2016

petebacondarwin Jan 14, 2016

lgalfaso Jan 16, 2016

Narretz Jan 14, 2016

petebacondarwin Jan 15, 2016

Narretz commented Jan 14, 2016

Narretz commented Jan 14, 2016

petebacondarwin commented Jan 14, 2016

gkalpak Jan 15, 2016

petebacondarwin Jan 15, 2016

gkalpak Jan 15, 2016

petebacondarwin Jan 15, 2016

Narretz commented Jan 15, 2016

gkalpak Jan 15, 2016

petebacondarwin commented Jan 15, 2016

Narretz commented Jan 15, 2016

Narretz commented Jan 15, 2016

petebacondarwin commented Jan 15, 2016

ev45ive commented Jan 17, 2016

usernamealreadyis commented Jan 24, 2016

		@@ -4244,6 +4244,22 @@ describe('$compile', function() {
		if (!/chrome/i.test(navigator.userAgent)) return;

$onInit and friends #13763

$onInit and friends #13763

Conversation

petebacondarwin commented Jan 13, 2016

Choose a reason for hiding this comment

gkalpak commented Jan 13, 2016

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

Narretz commented Jan 14, 2016

Narretz commented Jan 14, 2016

petebacondarwin commented Jan 14, 2016

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

Narretz commented Jan 15, 2016

Choose a reason for hiding this comment

petebacondarwin commented Jan 15, 2016

Narretz commented Jan 15, 2016

Narretz commented Jan 15, 2016

petebacondarwin commented Jan 15, 2016

ev45ive commented Jan 17, 2016

usernamealreadyis commented Jan 24, 2016

`$onInit` and friends #13763

`$onInit` and friends #13763