feat(loader): add convenience method for creating components

[shahata]

The proposed syntax:

angular.module('myApp').component('myComponent', {
  template: '<div></div>', //string | Function, default: ''
  templateUrl: 'a.html', //string | Function, default: undefined
  controller: MyCtrl, //string | Function, default: function(){}
  controllerAs: 'vm', //string, default: component name
  transclude: false, //boolean | object, default: true
  isolate: false, //boolean, default: true (scope: {} .vs. scope: true)
  bindings: {abc: '@'}, //object, default: {} (passed to bindToController)
  $canActivate: MyCtrl.canActivate, //Function, default: undefined (passed to factory.$canActivate)
  $routeConfig: MyCtrl.routeConfig, //RouteConfig, default: undefined (passed to factory.$routeConfig)
});

This means that for the most common use-case of simple component ppl will only need to pass template, controller and bindings.

I've discussed this with @btford and we've decided to change a couple of things from #12907:

1. Controller lifecycle hooks (onActivate, onDeactivate, etc.) are not going to be part of the component options since it makes more sense that the developer will just put those as methods of his controller (this way those methods have access to controller's state)
2. In case developer passes a function for template/templateUrl, this function is injectable with locals of $element and $attrs. This is important because today if you pass a function for template/templateUrl in DDO, you get element as attrs, but for DI you rely on the injectable DDO factory. In the component helper you don't have a factory, therefore we make all functions that can be passed in options injectable.

Docs are still missing obviously, but I would like to get some feedback before moving on.

Closes #10007

[petebacondarwin]

For completeness we ought to mention that this PR is related to the discussion at #10007.

@shahata can you clarify how this version compares with the suggestion at #10007 (comment)?

Also have we considered the idea of ditching the magic symbols: @, = and & and using more ng2 terminology: properties, events and ... bindings, such as described at #10007 (comment)?

[petebacondarwin]

should be using invokeLaterAndSetModuleName?

[shahata]

this is just a wrapper around module.directive. module.directive will do the invokeLaterAndSetModuleName

[pkozlowski-opensource]

Once comment: IMO we should avoid the bindings name as it is really overloaded in ng2 and adding yet another meaning in ng1 won't help. Maybe "properties"?

[shahata]

@petebacondarwin this proposal is consistent with #10007 (comment) with the exceptions that module.component accepts an options obj instead of factory method and that scope isolation is optional

I'll look into the properties/events proposal

[shahata]

@petebacondarwin I thought a bit about ditching the magic symbols today and I think that I'm leaning against it. Main reasons are that terminology is not the same since ng1 doesn't really have one way binding (would you use properties for @ or =? what will you call the other?) and that as we previously discussed in #9137, introducing new way to do the same thing is something we should avoid. wdyt?

[shahata]

@petebacondarwin @btford @pkozlowski-opensource @caitp
Any thoughts regarding accepting properties and events instead of magic symbols in module.component? (see previous comment)

[petebacondarwin]

@shahata - I kind of like ditching the magical =, @ &, but I also appreciate that there is no direct mapping two properties - if we can't come up with a set of names that we are happy with then let's just stick with what we have. Also note that we now also have the additional "modifiers" on these magical characters: ?-optional and *-collection.

[aciccarello]

I don't think the functionality needs to line up 100% between ng1 and ng2 properties, events, etc.. But I do think that the mapping needs to be intuitive and well documented.

[pkozlowski-opensource]

@shahata I'm with @petebacondarwin here - the pb is that both @ and = mean "binding to a property" (using the ng2 terminology) - those just denote 2 different ways of binding. The same could be argued about & which can be also used to read expression value from an attribute....

So no, we shouldn't map existing symbols to props / events.

[shahata]

@petebacondarwin @pkozlowski-opensource if we decide to go forward with adding one way binding maybe we can use properties for one way binds, events for callbacks and optionally also bindings for the existing magic symbols syntax. wdyt?

[pkozlowski-opensource]

@shahata just fyi properties were just renamed to inputs and events to outputs in ng2. This sparked quite a discussion which IMO steamed from the fact that different people got different mental model of those things. Add on top the fact that concepts don't match between ng1 and ng2 and you are looking for trouble :-)

I must say that I'm not huge fan of the whole "make ng1 look like ng2 even if concepts don't match" story

[shahata]

This is why I suggested to do this only if we decide to match the concept of one way binds. Regarding naming I'm not going to touch it, we'll just do whatever ng2 does. :)

[petebacondarwin]

Let's stick with =, & and @ (and possibly > if it lands) for this PR.
That way we are not going to get caught in trying to keep parity with potentially changing ng2 API naming :-)
If ngForward wants to sugar this to look more like ng2 then that can sit on top of this easily enough.

[pkozlowski-opensource]

@shahata what is the one way binding in ng1 for you? @? If so the problem is that semantics are not the same... The pb with @ is that it can only accept strings (or at least it is going to convert bound value to string).

In ng2 you can bind any type so [foo]="expr" is more like = binding in ng1 if no one does assignment :-)

[petebacondarwin]

@pkozlowski-opensource the one-way binding we are talking about is this PR - #12835 - but using a different symbol rather than #, such as >.

Here is an interesting idea, which @MikeRyan52 has:

• all components must have either a template or a templateUrl
• all components must transclude their contents

This seems reasonable. I can't think of a situation where you would want a "component" that doesn't provide a template. And if that is the case then we would always be clearing out any child nodes of the element where the component is instantiated, so we might as well stick it in a transclude function. Previously this would have incurred a performance penalty but now that we have lazy compilation of transcluded contents, it would be negligible.

---

Side note - if the multi-slot transclusion lands, then this proposal would change to:

• component must have transclude: true (the default) or transclude: { ... } to indicate multi slot transclusion

[pkozlowski-opensource]

@petebacondarwin thnx for the info. I like the > symbol somehow, I must say!

And I completely agree that each component must have template / templateUrl. I would also add that each component need to use isolated scope (consequence of having a template and most probably state that goes with it)

all components must transclude their contents

I don't think I understand reasoning behind it. There are many examples of components that don't need to transclude anything (ex.: pagination, rating etc.)

[petebacondarwin]

Yes, I agree that components should also always have scope: {}

Regarding transclusion, it means that the configuration object that gets passed to module.component is even simpler in the majority of cases. I.E. that we would never need to put transclude: true in the options for this call.

If there was no content below the element then we could optmize so that nothing would be transcluded anyway. In any case, now that we have lazy compilation the cost will barely be noticed, and if it was a problem then we could allow people to explicitly override it with transclude: false.

So I guess my suggestion is that for components the default would be transclude: true.

[pkozlowski-opensource]

So I guess my suggestion is that for components the default would be transclude: true.

@petebacondarwin makes sense if there are no side effect / perf hit

[petebacondarwin]

So the API that I would be happy with is:

angular.module('myApp').component('myComponent', {

  template: '<div></div>', //string | Function, default: undefined
  templateUrl: 'a.html', //string | Function, default: undefined

  controller: MyCtrl, //string | Function | Array, default: function(){}
  controllerAs: 'vm', //string, default: component name
  bindings: {abc: '@'}, //object, default: {} (passed to bindToController)

  $canActivate: MyCtrl.canActivate, //Function, default: undefined (passed to factory.$canActivate)
  $routeConfig: MyCtrl.routeConfig, //RouteConfig, default: undefined (passed to factory.$routeConfig)

  isolate: false, //boolean, default: true (scope: {} .vs. scope: true)
  transclude: false //boolean | object, default: true
});

where

• one of template or templateUrl must be provided
• controller can be a string (name of a registered controller), function/class (explicit controller) or an array (inline annotated explicit controller)
• isolate defaults to true
• tranclude defaults to true

[pkozlowski-opensource]

controller: MyCtrl, //string | Function | Array, default: function(){}
controllerAs: 'vm', //string, default: component name

To be honest I've never was fan of 2 properties here. How about MyCtrl as myName with default being vm if no as is provided?

[shahata]

np, I'll add transclude: true by default. regarding controllerAs, it is allowed even today (though poorly documented) to pass controller: 'MyCtrl as vm' in DDO and therefore it is also supported by module.component. The reason we still need controllerAs though is in the case I pass a class/function in controller, which I think will be the more common case.

[petebacondarwin]

I prefer the case as stated where it defaults to the name of the component

[shahata]

+1 for @petebacondarwin

[0x-r4bbit]

Okay sounds good! 👍

[gkalpak]

Why are we deviating from the "normal" DDO's template/templateUrl (which don't get injected) ?
There's probably a good reason; I just want to know it :)

Maybe we should stress it more, because more experienced users might get biten by this.

[petebacondarwin]

The reason is that in module.component there is no factory function. In classic DDO, the template and templateUrl properties are in the closure of the factory function and so have access to injectables from there.

[gkalpak]

I left a coule of minor comments.

One thing that is missing imo, is the restrict property. Right now it falls back to the current default for directives (and is not configurable using .component()).
When using .component(), I would expect it to default to 'E' and also be configurable.

It might also be useful to make all defaults globally configurable, but I am not sure it is a good idea to give too much power to the user 😛

[petebacondarwin]

The defaults need to be consistent globally, since people may be loading in 3rd party libraries that also register using module.component. If you could configure them per app then these 3rd party components might break.

[gkalpak]

As a user (of the framework) I would prefer to be able to configure the defaults (espacially if I have a large number of components) and 3rd-party libraries better define explicit values to avoid being broken.

But I agree it might be too much for now - it's probably better to keep things as straightforward as possible.

[mgol]

IMO defaulting to restrict: 'E' for components would be clearer, that's what people use for components when they drop IE8 support. Is there any reason why it needs to stay EA?

[shahata]

Updated docs & didn't squash on purpose to make it easy to see diff. I'm staying with capital letters for now until we decide otherwise. Regarding restrict: 'E' I tend to agree that it should be the default, but I can go either way.

[petebacondarwin]

I agree, let's default to E but allow it to be overridden in the options object.

[shahata]

Cool. Done.

[petebacondarwin]

Any more feedback before we try to get this merged?

[gkalpak]

LGTM 😸

[shairez]

We should probably change "bindings" to "providers" right?

[petebacondarwin]

@shairez - nope. Bindings in this context are template bindings. Nothing to do with the DI system.

[shairez]

@petebacondarwin sorry, skipped that part in the discussion, I see now it's like what used to be the scope object in the DDO.
I agree with you that it might be confusing, but since the name changed in ng2 from "bindings" to "providers", I hope it will not be confusing moving forward.
Thanks for clarifying! I'll start and test it to give more feedback

[gkalpak]

Let there be...:sparkles: component :sparkles:

Woohoo ! 🎉

[petebacondarwin]

Well done everyone who had a hand in this, especially @shahata for the implementation

[sarod]

Sorry for the late feedback but I find that using transclude:true by default is probably not very user friendly for beginners as it causes more problems for simple usecases.

Thinking a bit more about this I created a separate issue to track this #13566