docs($compile): add double compilation known issue #15392

Narretz · 2016-11-15T09:03:38Z

No description provided.

gkalpak

We could also put that in the compiler guide (with a short note here in the API docs that double-compilation is not supported).

gkalpak · 2016-11-15T10:27:41Z

src/ng/compile.js

+ * elements.
+ *
+ * This can cause unpredictable behavior, e.g. `ngModel` $formatters and $parsers will be
+ * attached again to the ngModelController. It can also degrade performance, as


ngModel $formatters and $parsers will be attached again to the ngModelController

Why? ngModel seems to only be compiled once.

True, I was actually thinking of an issue with replace. I will change the example.

gkalpak · 2016-11-15T10:27:57Z

src/ng/compile.js

+ * attached again to the ngModelController. It can also degrade performance, as
+ * watchers for text interpolation are added twice to the scope.
+ *
+ * Double compilation should therefore avoided. In the above example, the better way is to only


avoided --> be avoided

the better way --> a better way (?)

gkalpak · 2016-11-15T10:31:58Z

src/ng/compile.js

+      link: function(scope, element, attrs) {
+        var newEl = angular.element('<input ng-model="$ctrl.value">');
+        $compile(newEl)(scope); // Only compile the new element
+        element.append(newEl);


I think usually it is preferrable to first append the element and then compile it, in order to have proper context (e.g. requiring parent directive controllers).

gkalpak · 2016-11-15T10:33:30Z

src/ng/compile.js

+  angular.module('app').directive('addOptions', function($compile) {
+    return {
+      link: function(scope, element, attrs) {
+        attrs.$set('addInput', null) // To stop infinite compile loop


addInput --> addOptions

gkalpak · 2016-11-15T10:44:13Z

src/ng/compile.js

+ *
+ * In that case, it is necessary to intercept the *initial* compilation of the element:
+ *
+ * 1. give your directive the `terminal` property and a higher priority than directives


Capitalize these sentences.

gkalpak · 2016-11-15T10:45:57Z

src/ng/compile.js

+ * that should not be compiled twice. In the example, the compiler will only compile directives
+ * which have a priority of 100 or higher.
+ * 2. inside this directive's compile function, remove the original directive attribute from the element,
+ * and add any other directive attributes. Removing the attribute is necessary, because otherwise the


You don't need to remove the attribute, since you pass the priority to $compile.

Narretz · 2016-11-15T17:36:04Z

I've updated the example and moved it to the compiler guide, and added a short paragraph with a link to the compile api docs

gkalpak · 2016-11-15T17:55:39Z

docs/content/guide/compiler.ngdoc

+  return {
+    link: function(scope, element, attrs) {
+      var newEl = angular.element('<span ng-show="showHint"> My Hint</span>');
+      element.on('mouseenter mouseout', function() {


The opposite of mouseenter is ~~mouseout~~mouseleave 😁

Sorry, I don't understand. Do you mean it should be mouseleave?

Yeah, sorry, that's what I meant 😁

This is still mouseout 😞

gkalpak · 2016-11-15T17:56:58Z

docs/content/guide/compiler.ngdoc

+to a button with `ngClick` on it:
+
+```
+angular.module('app').directive('addMouseover', function($compile) {


We should have a big, fat comment or a danger alert that this is what you should NOT do. People tend to get confused when the docs show bad examples to be avoided.

I agree, but we should have a general solution to this.

I tried putting a div around it, but then the code blocks aren't rendered correctly. Seems to be an issue with the marked library: markedjs/marked#596

gkalpak · 2016-11-15T17:58:01Z

docs/content/guide/compiler.ngdoc

+        scope.$apply('showHint = !showHint');
+      });
+
+      attrs.$set('addMouseover', null);


No need to remove this any more.

gkalpak · 2016-11-15T17:58:43Z

docs/content/guide/compiler.ngdoc

+```
+
+Another scenario is adding a directive programmatically to a compiled element and then executing
+compile again.


Again, lets make it clear this is a BAD thing.

gkalpak · 2016-11-15T18:00:03Z

docs/content/guide/compiler.ngdoc

+  which have a priority of 100 or higher.
+  2. Inside this directive's compile function, remove the original directive attribute from the element,
+  and add any other directive attributes. Removing the attribute is necessary, because otherwise the
+  compilation would result in an infinite loop.


You don't need to remove the attribute, since you pass the priority to $compile.

gkalpak · 2016-11-15T18:01:10Z

docs/content/guide/compiler.ngdoc

+  return {
+    priority: 100, // ngModel has priority 1
+    terminal: true,
+    template: '<input ng-model="$ctrl.value">',


You don't want to have a template.

Related angular#15278

Narretz · 2016-11-22T21:10:57Z

I've updated the PR @gkalpak PTAL

gkalpak

A couple of typos.
LGTM otherwise 👍

gkalpak · 2016-11-22T21:33:12Z

docs/content/guide/compiler.ngdoc

+  return {
+    link: function(scope, element, attrs) {
+      var newEl = angular.element('<span ng-show="showHint"> My Hint</span>');
+      element.on('mouseenter mouseout', function() {


This is still mouseout 😞

gkalpak · 2016-11-22T21:34:51Z

docs/content/guide/compiler.ngdoc

+  2. Inside this directive's compile function, add any other directive attributes to the template.
+  3. Compile the element, but restrict the maximum priority, so that any already compiled directives
+  (including the `addOptions` directive) are not compiled again.
+  4. In the link function, link the compiled element with the element's scope


Missing period at the end.

gkalpak · 2016-11-22T21:37:15Z

docs/content/guide/compiler.ngdoc

+function on the directive element. In the following **faulty example**, a directive adds a mouseover behavior
+to a button with `ngClick` on it:
+
+```


Should this be ```js (here and below) or does marked autodetect JS?

I always thought it auto-detects

gkalpak · 2016-11-22T21:40:00Z

docs/content/guide/compiler.ngdoc

+to a button with `ngClick` on it:
+
+```
+angular.module('app').directive('addMouseover', function($compile) {


I would still prefer a comment above this line; something like:

// BAD - Don't do this

And an equivalent comment in the recommended alternatives; e.g.:

// OK - You can do this

But I can leave without if you think it's too much. Up to you.

I'll leave it as is, and we'll see how soon someone stumbles over this

Narretz · 2016-11-22T22:22:48Z

I think the part about mouseout/leave is broken on Github's UI. You can see that it's mouseleave in the file at the latest fixup commit. It just shows wrongly in the "Files Changed" view. I can also not comment on it from the comment view.

gkalpak · 2016-11-23T07:50:17Z

There were two occurrences of mouseout. You have replaced the second one with mouseleave, but there is still one more (on line 397).

Narretz · 2016-11-23T12:05:48Z

Omg ...

gkalpak

LGTM (as soon as travis is happy)

Related #15278 Closes #15392

Related angular#15278 Closes angular#15392

Narretz added component: $compile type: docs labels Nov 15, 2016

Narretz added this to the Backlog milestone Nov 15, 2016

googlebot added the cla: yes label Nov 15, 2016

gkalpak suggested changes Nov 15, 2016

View reviewed changes

Narretz force-pushed the docs-compile-double branch from a622fa7 to af8f311 Compare November 15, 2016 17:36

gkalpak suggested changes Nov 15, 2016

View reviewed changes

petebacondarwin assigned Narretz Nov 21, 2016

Narretz added 3 commits November 22, 2016 17:35

docs($compile): add double compilation known issue

ea7b680

Related angular#15278

fixup: better examples

ae6a126

move to the compiler guide

275b4e3

Narretz force-pushed the docs-compile-double branch 2 times, most recently from 9bce926 to f97ed1f Compare November 22, 2016 16:49

fixup

53bef98

Narretz force-pushed the docs-compile-double branch from f97ed1f to 53bef98 Compare November 22, 2016 16:49

gkalpak approved these changes Nov 22, 2016

View reviewed changes

Add period

6b11591

user378230 mentioned this pull request Nov 23, 2016

Error when doing $compile within the <ui-select> Tag angular-ui/ui-select#1856

Closed

remove second mouseout

9114144

gkalpak approved these changes Nov 23, 2016

View reviewed changes

Narretz merged commit 69f59f2 into angular:master Nov 23, 2016

Narretz added a commit that referenced this pull request Nov 23, 2016

docs($compile, guide/compiler): add "double compilation" known issue

3c407fd

Related #15278 Closes #15392

petebacondarwin pushed a commit that referenced this pull request Nov 23, 2016

docs($compile, guide/compiler): add "double compilation" known issue

a2d7691

Related #15278 Closes #15392

petebacondarwin pushed a commit that referenced this pull request Nov 24, 2016

docs($compile, guide/compiler): add "double compilation" known issue

1f61f30

Related #15278 Closes #15392

ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017

docs($compile, guide/compiler): add "double compilation" known issue

8ec3198

Related angular#15278 Closes angular#15392

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs($compile): add double compilation known issue #15392

docs($compile): add double compilation known issue #15392

Narretz commented Nov 15, 2016

gkalpak left a comment

gkalpak Nov 15, 2016

Narretz Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

Narretz commented Nov 15, 2016

gkalpak Nov 15, 2016 •

edited

Loading

Narretz Nov 17, 2016

gkalpak Nov 18, 2016

gkalpak Nov 22, 2016

gkalpak Nov 15, 2016

Narretz Nov 17, 2016

Narretz Nov 17, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

gkalpak Nov 15, 2016

Narretz commented Nov 22, 2016

gkalpak left a comment

gkalpak Nov 22, 2016

gkalpak Nov 22, 2016

gkalpak Nov 22, 2016

Narretz Nov 22, 2016

gkalpak Nov 22, 2016

Narretz Nov 22, 2016

Narretz commented Nov 22, 2016

gkalpak commented Nov 23, 2016 •

edited

Loading

Narretz commented Nov 23, 2016

gkalpak left a comment

docs($compile): add double compilation known issue #15392

docs($compile): add double compilation known issue #15392

Conversation

Narretz commented Nov 15, 2016

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

Choose a reason for hiding this comment

Narretz commented Nov 15, 2016

gkalpak Nov 15, 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

Narretz commented Nov 22, 2016

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

Narretz commented Nov 22, 2016

gkalpak commented Nov 23, 2016 • edited Loading

Narretz commented Nov 23, 2016

gkalpak left a comment

Choose a reason for hiding this comment

gkalpak Nov 15, 2016 •

edited

Loading

gkalpak commented Nov 23, 2016 •

edited

Loading