Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

WIP angular.component implementation [ci skip] #12166

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ function setupModuleLoader(window) {
*/
controller: invokeLaterAndSetModuleName('$controllerProvider', 'register'),

/**
* @ngdoc method
* @name angular.Module#component
* @module ng
* @param {string} name Component name, used as a directive name
* @param {object=} bindings
* @param {string|function} controller used for component
*/
component: invokeLaterAndSetModuleName('$compileProvider', 'component'),

/**
* @ngdoc method
* @name angular.Module#directive
Expand Down
54 changes: 52 additions & 2 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,52 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
}

/**
* @ngdoc method
* @name $compileProvider#component
* @kind function
*
* @description
* Register a new component with the compiler.
*
* A component is a shorthand for a directive with a controller, template, and a
* new scope.
*
* @returns {ng.$compileProvider} Self for chaining.
*/
this.component = registerComponent;
function registerComponent(name, bindings, controller) {
if (isString(name)) {
assertValidDirectiveName(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going to be called by registerDirective() anyway ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see the error thrown in the registration method I call, rather than the underlying implementation.

if (arguments.length < 3) {
controller = bindings;
bindings = controller.bindings || null;
}
return registerDirective(name, valueFn({
controller: controller,
controllerAs: controller.controllerAs || '$' + name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ was an unexpected default for me (fwiw).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be $ctrl or something standard across all directives so that it's easy to do a global find/replace when switching to Angular 2. As-is, you'd have to find/replace for each directive template individually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - when I created a similar approach, we'd just done a bunch of work with React. We called it state.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ctrl or vm?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use vm which I like because it is short. I'd only be concerned that it may be confusing for someone who isn't familiar with the "view model" terminology. That term would have to be explained well in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Horrible terrible no good but very flexible idea: let users specify the default controllerAs in a provider, because no one is ever going to like the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we need to really figure out what we want to do here

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see ngUpgraders/ng-forward#6 for discussion on configuration in that decorators implementation

scope: controller.isolate ? {} : true,
bindToController: bindings,
template: controller.template || false,
templateUrl: controller.templateUrl || false
}, true));
} else {
forEach(name, function(value, name) {
var controller = null;
var bindings = null;
if (isArray(value) || isFunction(value)) {
controller = value;
bindings = value.bindings || null;
} else if (isObject(value)) {
controller = value.controller || null;
bindings = value.bindings || null;
}
return registerComponent(name, bindings, controller);
});
}
return this;
}

/**
* @ngdoc method
* @name $compileProvider#directive
Expand All @@ -827,7 +873,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
* {@link guide/directive} for more info.
* @returns {ng.$compileProvider} Self for chaining.
*/
this.directive = function registerDirective(name, directiveFactory) {
this.directive = function callRegisterDirective(name, directiveFactory) {
return registerDirective(name, directiveFactory, false);
};
function registerDirective(name, directiveFactory, isComponent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not enough space for this.directive?

assertNotHasOwnProperty(name, 'directive');
if (isString(name)) {
assertValidDirectiveName(name);
Expand All @@ -850,6 +899,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
directive.name = directive.name || name;
directive.require = directive.require || (directive.controller && directive.name);
directive.restrict = directive.restrict || 'EA';
directive.component = !!isComponent;
var bindings = directive.$$bindings =
parseDirectiveBindings(directive, directive.name);
if (isObject(bindings.isolateScope)) {
Expand All @@ -869,7 +919,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
forEach(name, reverseParams(registerDirective));
}
return this;
};
}


/**
Expand Down