johnpapa / angular-styleguide

Angular Style Guide: A starting point for Angular development teams to provide consistency through good practices.
http://johnpapa.net
MIT License
23.88k stars 4.16k forks source link

ng-include #444

Open alsfurlan opened 9 years ago

alsfurlan commented 9 years ago

@johnpapa Why you don't recommend use of the directives instead ng-include in your guide? I think a best pratice use directives.

ghost commented 9 years ago

@alsfurlan I completely agree with you regarding the use of custom directives over ng-include. When using ng-include, you are forced to spread and duplicate urls to templates throughout your code. This is solved by using a custom directive where the url can be described as a templateUrl in a single location.

/* avoid */

// index.html
<div ng-include="/src/feature/feature.html"></div>
/* recommended */

// index.html
<div feature></div>

// src/feature/feature.directive.js
(function() {

    angular
        .module("feature")
        .directive("abcFeature", abcFeature);

    function abcFeature() {
        var directive = {
            restrict: "A",
            templateUrl: "/src/feature/feature.html"
        }
        return directive;
    }

})();
alsfurlan commented 9 years ago

@jteppinette Yeah! This is why I ask to @johnpapa. I think with your code is very relevant and could be included in him angular style guide. Ng-include remeber languages like PHP e Java and it seems, to me, a bad practice.

dietergeerts commented 9 years ago

And what do you do when you want to switch out the view for a controller/directive? View and ViewModel should be separated and there are situations where I want another view for the same ViewModel. For this, ng-include is really handy, as you can define the viewmodel (controller) and view that has to be combined. A directive should be used for small components that doesn't influence the actual view, but work on it.

Also, using directives messes up Angular's form behaviours and you aren't being able to have correct validation anymore...

ghost commented 9 years ago

@dietergeerts I believe the tool you are looking for in this case is a router. In your particular situation, you can define a parent route with a controller and then you can create children of that route with or without their own individual templates, controllers, urls, etc...

Thank you for joining the conversation. We need as much input as possible to create a great styleguide.

alsfurlan commented 9 years ago

I agree with @jteppinette.

ng-include seems only one not to let the html at all on the same page, which makes no sense at all. It will be a reusable HTML, uses a directive, if you have other logical use a directive.

Not least because it does not seem coherent keep doing this with many other features in angular.

If you change the path of your HTML file will break the view. Millions of places I can have my ng-include, millions of places to refactor. A directive only in one spot, on a route only in one location.

Do not you want the big HTML? Think of componentization and division of responsibilities between controller, do not use ng-include.

dietergeerts commented 9 years ago

@jteppinette , no, for many situations the router can't be used. If you have a list of items for example, I can now use ng-include to tell how they must be represented, as it can be different depending on where this list is. The controller to set the item behaviour can also be set in that ng-include. This has nothing to do with router. If you think it does, than please provide such an example.

ghost commented 9 years ago

@dietergeerts In the situation you have depicted, the best course of action would be utilizing a directive with a template that used 'ng-if' to switch between different sets of inner html based on an isolated scope variable that is passed into the directive.

I am currently on my phone, but when I reach a computer, I will provide proper examples.

alsfurlan commented 9 years ago

@dietergeerts In your case, for items a directive should be created. It is the best way to do. You can see in several angularjs books including, in your example, a list of items, most of them create directive for item individually. The controller for setting behavior, in this case, must be set in the directive. "ng-include" is an amateur way to do it, I think. And then again, as you would treat a case where moves your html file location? Refactor everywhere?

alsfurlan commented 9 years ago

@dietergeerts And as well remembered by @jteppinette, has the scope that can treat it.

johnpapa commented 9 years ago

i dont personally use ng-include anymore. i use a directive for even basic footers and headers. but its not a bad practice to use ng-include.

might be worth adding to the guide, but i would be interested in hearing about more use cases for and against

alsfurlan commented 9 years ago

@johnpapa Of course not, I think just an elegant way to do what ng-include do: directive. Ok, let the people discuss. :+1:

johnpapa commented 9 years ago

just a few stray thoughts on both sides, nd in between...

ng-include is dirt simple. thats a big plus. also very obvious what it is doing.

harder to test , yeah, i think directive is easier.

if you use ng-include you could add controller in the view directly, which is different than the style of putting the routes and controllers together. not good or bad, just different.

directives are a pain .... ugly with their DDO. but using them for a simple template is pretty easy.

ghost commented 9 years ago

Statement


I am in support of adding a directive over ng-include rule to the angular style guide.

Positive & Negatives


To demonstrate my thoughts clearly, I have created the following list of positives and negatives:

Positives

Negatives

References


Discussion Contributors


wardbell commented 9 years ago

I will speak in favor of ng-include as a valuable alternative to creating a custom directive.

Pros

Cons

The Ideal ng-include Scenario:

ng-include is perfect if you're referencing the included view template exactly once. I do that a lot.

I break up a gigantic, incomprehensible view into a master view and sub-views. Sub-views may have their own controllers but they often just inherit the master view controller.

These sub-views won't be seen anywhere else except in the master view. I don't want you to navigate to them or to any particular combination of sub-views. The master view and sub-view combinations are determined entirely by the master view's controller and whatever state is passed in or preserved from a prior visit.

When not to use ng-include

When the view will appear in many places (e.g., headers and footers). Such intentionally re-usable visual components deserve the effort necessary to make them directives.

My opinion

I think creating a one-off directive is a waste of effort, complicates understanding, is a burden to maintain, and is an excess of architectural zealotry.

In general, I do not worrying about DRY nor go through the gyrations of abstracting into multiple moving parts until I've used the darn thing more than once.

The @jteppinette example (early in this thread) of what to avoid and what to do illustrates my point perfectly.

I would always prefer the one line, <div ng-include="/src/feature/feature.html"></div>, to the 16 lines of madness if I am only ever going to see feature.html in one place.

OTOH, the @jteppinette example makes perfect sense if the view is re-usable.

ghost commented 9 years ago

@wardbell Thank you joining the conversation! Your expertise will definitely come in handy.

I completely agree with the points you have made, and I believe that we could point out to readers that both approaches have merit when used in the proper context.

Ng-Include should be used when the template is only being injected a single time.

Custom Directives should be used when the component is being used in multiple places.

reflexdemon commented 9 years ago

If we actually consider from a style guide recommendation perspective we cannot recommend Custom Directives over ng-include.

The answer to the question mainly remains on what is put on the Custom Directives or ng-include.

As a developer I would put static content like footer(provided it is static) of the page or some static disclaimer types of things on ng-include. If I have the content of directive to have any actionable items like navbar or some content that might change due to user action or the content on other sections then a directive is very useful.

This is how I would choose what to be used on ng-include vs Custom Directives.

johnpapa commented 9 years ago

Great discussion.

One thing I want to highlight here is that I 200% agree that directives should not be created for one time use, without some other factors (such as heavy required DOM manipulation) leaning to a directive. There is a trend for everything to be a reusable component .... even if it will never be reused. If it will not be reused, then there must be other valid factors to influence it, otherwise, no directive for me.

Reusable ---> directive Heavy DOM ---> directive 1 timer ---> ng-include is fine

alsfurlan commented 9 years ago

But I think the should not even exist because if your HTML page is big enough to need to use the ng-include, probably you should separate your logic into smaller pieces.

An example set would be a screen with tabs, you can use the ng-include to use tabs, each tab or to be a controller and have a parent controller. Each region of the screen being controlled by a controller into small pieces, as it should be. Something that would not be used if it were simply used the ng-include.

I personally use it and I think it makes more sense. Do not you think?

Like the picture:

sem titulo

dietergeerts commented 9 years ago

One thing that didn't work a few versions back, is that when you use a directive to split up a form, your form validation will get messed up. I think it's still that way, so for splitting up a form, ng-include should be used. Like for example, when you have an address part that you use in multiple forms, I include it with:

<ng-include data-src="address.html" 
          data-controller="AddressController as vm" 
          data-ng-model="formController.address">
</ng-include>

This also shows nicely how you can give a model object to a controller to work with. There is no need for that controller to know its parent.

dietergeerts commented 9 years ago

@alsfurlan, When I first saw those pictures, I was: That's wrong!. Then I realized that I always look at these things differently then most people do. Yes you have a view and a viewmodel, which are html and controller in this case. And yes they will always be coupled together in some way. But it's where and how they get coupled that I see differently. Just switch the controller and html pairs, so that the html comes first and in that the controller. You will then notice that ng-include is not bad and actually usefull, especially in the case you are describing: a tab control! If you design your controllers well, there is even no need to emit or broadcast events as the controller for an item doesn't have to know it's parent controller, and otherwise. I always build controllers to be on their own, no need for any controller to know others... (unless you have very special cases). The wiring up of the controllers and views will happen at your html with ng-include, which can be given the view, viewmodel(controller) and a model, which can be used by the controller. You could even use some viewresolver to get a view based on the actual model....

Ow, picture 2 is not how I ever saw ng-includes! That is just splitting up your html in smaller pieces, but if you do that, then there should ring a bell you have to split up your controller too. So I always see that a controller will go together with a view(thml).

dietergeerts commented 9 years ago

@jteppinette: Some thoughts I have on your positives:

• Dry: When working on large code bases that make use of ng-include simple code-reorganization requires changing urls throughout the code base.

This ins't true when using kind of a ViewResolver. Plus a simple find and replace isn't that hard if not using a ViewResolver...

• Modularization, Cohesion, & Coupling: When I have seen developers make use of ng-include it is typically accompanied by very high coupling and low cohesion of their controllers. The simple act of using a directive to supply a template encourages the developer to also place the templates business logic inside a separate template or service that is accompanied by that specific directive.

Just never couple controllers! Always create a controller that does his thing and don't mind about others, this is just applying good practice... Business logic in a template? There should only be view logic in a template, nothing more...

• Testing: The practice of testing directives has been repeatedly tried and documented. The use of directives over ng-include is clearly the winner in this regard.

Nope, it's not your html that you are testing, but the controller behind it. If you clearly split up your controllers and have them isolated from any other controller, they are very easy to test! You just have to see controllers as small components that are doing one or two things.

• Naming: Directives provide readers of software a clear way to identify the functionality of an element or attribute directive through its element or attribute name. This is in comparison to the use of ng-include where the reader must parse the url to identify its functionality.

Also nope, this is just using good naming. You can put a controller attribute in ng-include to specify which controller to choose, that name should be clear on what it will be doing. Hence, it will be even more clear when you have multiple templates that you can use and 'remove/hide' some functionality of the controller because it's not included. Just use good naming...

jansepke commented 9 years ago

@dietergeerts I just added my thoughts

Always create a controller that does his thing and don't mind about others, this is just applying good practice...

I'm with you. But if you split up your view into multiple parts with ngInclude using its own controller you will end up with fragments like this:

<ng-include data-src="templates/list-item.html" 
          data-controller="ListItemController as vm">
</ng-include>

if you would use a directive even if you only need this fragment in one single place you get something much more easier to read:

<xx-list-item></xx-list-item>

and the directive definition would just contain a controller name and the template url, thats not a big deal to me.

One thing that didn't work a few versions back, is that when you use a directive to split up a form, your form validation will get messed up. I think it's still that way, so for splitting up a form, ng-include should be used

I see a valid point for ngInclude for partial form parts as a workaround for the problem you described if this problem could be solved in a future version of angular.

another advantage of a directive is that you could define an interface with the bindToController property so you can see what properties are needed by this fragment:

<xx-list-item item="loopVar" color="red"></xx-list-item>
johnpapa commented 9 years ago

I'll consider adding this guidance. Need to think more on it and see how I want to write it up. My thoughts are unchanged and are congruent with my last comment.

Thanks for the feedback.

hrajchert commented 9 years ago

I agree with @alsfurlan, if you are having a huge HTML it may be a code smell to separate it in smaller components. Even if they are not reusable they are easier to read.

The drawback is the amount of files being generated, because for a ng-include you only need one file (the template) and for the directive you need two, the definition (and optional logic) and the template.

I tend to have a lot of components (element directives + template) that don't even have logic, but its easier to read and reason about and as a really cool bonus I can declare a parent component in the directive requirements to avoid just trusting the data is in the scope.

alsfurlan commented 9 years ago

@hrajchert Yeah! That's a way to think too. Easier to read. You can keep your scope in your directive. So why not use directives all the time? A more semantic html how the HTML5 proposal

hrajchert commented 9 years ago

@alsfurlan You made me remember another point, It also makes your end to end easier as you can rely on semantic elements instead of just divs with clases

tedvanderveen commented 9 years ago

I think the 'one time use' scenario clouded this discussion and moved it away from the original question in this issue. I cannot see a very good reason for ever creating a seperate child view/template that is used only once, just leave the html in the corresponding parent.

When it comes to multi-use scenarios, I always use directives. I believe componentization is key for DRY and Single responsibility principle reasons when keeping in mind the upcoming Angular2 shadow DOM stratigies for directives. Injecting html around using ng-include to me smells like the spaghetti code of the old MS ASP scripting days...

johnpapa commented 9 years ago

@tedvanderveen If we isolate your last point and only look here ...

When it comes to multi-use scenarios, I always use directives.

... then yes.

johnpapa commented 9 years ago

Interesting thoughts ... anyone want to try to summarize and make a proposal? Ive got my own ideas, but Id' like to hear how ya'll would propose it to look in the guide

NicholasBoll commented 9 years ago

I prefer component directives over ng-include and ng-controller. With component directives the API is explicit. Component directives can be tested in isolation (I actually use protractor to do this). A template with ng-include cannot be tested in isolation since it requires some matching ng-controller - more likely a series of ng-controllers.

ng-include encourages a mess of scope inheritance and is not suitable for larger teams. Looking at <div ng-include="'/src/feature/feature.html'"></div> can you tell me what $scope values it requires? Can you tell me which controller it requires for methods?

The trap is that ng-include is easier for one-off templates, but as applications grow those snippets of HTML need to exist somewhere else. Now you have to untangle the complicated mess of $scope inheritance and $scope events to use it somewhere else.

ng-include requires an extra $apply ($rootScope.$digest) for an included template (even if it is already in $templateCache). Proof. With a component, you can use the template property to avoid extra $scope.$applys which are costly when building the page. Templates can be included through a build process or a module bundler like webpack.

Angular 2, React and Ember are all components and component hierarchies. The new Angular router is even a 'component router'. The goal of this style-guide is to promote best practices that avoid pitfalls.

dietergeerts commented 9 years ago

@NicholasBoll , then please give an example of such directive that has some input fields and will be used in a form, and where all validation Angular provides still works....

NicholasBoll commented 9 years ago

@dietergeerts, is this what you're looking for?

http://plnkr.co/edit/5JmO4qyTG2lc6642bYFo?p=preview

Version 1 shows passing down the form through an attribute Version 2 shows the form being required by the text-field directive

dietergeerts commented 9 years ago

@NicholasBoll , yes thx! (For some reason, that didn't work in very old version of Angular. Never tested it in newer version, Glad to know it does work now.)

NicholasBoll commented 9 years ago

@dietergeerts You must be talking about versions before 1.2, where attributes were evaluated inside the isolate scope. This meant you had to access $parent to get to the data you wanted. In my provided example, it would have looked like this:

<text-field label="First name" value="$parent.user.first"></text-field>

Luckily they fixed that - isolate scopes were pretty useless back then.

NikitaGp commented 8 years ago

hello ,

I am facing a problem when using ng-include for include a view in another view which have a different controller id render that only when i refresh the page then its hide no error display on console could you please what i am doing wrong here is the code :-

Enable Friends ``` ```

angular.module('starter.controllers', []) .controller('ChatListCtrl', function($scope) { $scope.settings = { enableFriends: true };

.controller('ChatsCtrl', function($scope, Chats) {

$scope.chats = Chats.all(); $scope.remove = function(chat) { Chats.remove(chat); }; })

NikitaGp commented 8 years ago

Thanq @darioTecchia

johnpapa commented 8 years ago

there is some great stuff in here. anyone want to make a PR?