Skip to content
This repository was archived by the owner on Sep 7, 2018. It is now read-only.

Annotation Improvement #10

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Annotation Improvement #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0f78561
Add auto finding CtrlName, ServiceName. Change directive declaration.
Romakita Mar 7, 2016
bb4c7b1
Add decorator description, improve readme
Romakita Mar 7, 2016
a821021
Fix some bugs
Romakita Mar 7, 2016
a3a5ca4
Fix angular version
Romakita Mar 7, 2016
90135ad
Fix karma version (see
Romakita Mar 7, 2016
7bb90b9
Fix directive, Fix decorator instantiation
Romakita Mar 9, 2016
c19d74f
Add unit test
Romakita Mar 9, 2016
34433c0
Fix at.decorator, add unit test
Romakita Mar 9, 2016
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
106 changes: 98 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ What ?
**angular-typescript** provides annotation like decorators:

```
@at.service(moduleName: string, serviceName: string)
@at.service(moduleName: string, serviceName?: string)
@at.inject(dependencyOne: string, ...dependencies?: string[])
@at.controller(moduleName: string, controllerName: string)
@at.controller(moduleName: string, controllerName?: string)
@at.directive(moduleName: string, directiveName: string)
@at.classFactory(moduleName: string, className: string)
@at.resource(moduleName: string, resourceClassName: string)
@at.directive(moduleName: string, directiveProperties: at.IDirectiveProperties)
@at.classFactory(moduleName: string, className?: string)
@at.resource(moduleName: string, resourceClassName?: string)
@at.decorator(moduleName: string, serviceBaseName: string)
```

Why ?
Expand Down Expand Up @@ -62,7 +64,7 @@ angular.module('ngModuleName').service('someService', SomeService);
Using **angular-typescript** it will look like:

```typescript
@service('ngModuleName', 'someService')
@service('ngModuleName')
class SomeService {

constructor() {
Expand All @@ -74,6 +76,22 @@ class SomeService {
}

}

// or

@service('ngModuleName','AliasSomeService')
class SomeService {

constructor() {
// do stuff
}

public someMethod(anArg: number): boolean {
// do some stuff
}

}

```

***
Expand Down Expand Up @@ -123,9 +141,27 @@ class SomeService {

### Controller


```typescript
@controller('ngModuleName', 'SomeController')
@controller('ngModuleName')
class SomeController {

constructor(
@inject('$scope') $scope: angular.IScope,
@inject('$parse') private $$parse: angular.IParseService
) {
// do stuff with $scope and $$parse;
}

public someMethod(anArg: number): boolean {
// do some stuff with this.$$parse();
}

}


//or

@controller('ngModuleName', 'AliasSomeCtrl')
class SomeController {

constructor(
Expand All @@ -146,7 +182,35 @@ class SomeController {

### Directive

Static class members of directive controller are used as config directive config.
You can configure your directive with annotation :

```typescript
@directive('ngModuleName', {
selector: 'atSomeDirective', //required
controllerAs: 'someDirectiveCtrl', //optional
templateUrl: '/partials/some-directive.html'
})
class SomeDirectiveController {

public static link: angular.IDirectiveLinkFn = (scope, element, attrs, ctrl: SomeDirectiveController) => {
Copy link
Owner

Choose a reason for hiding this comment

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

I think that static link member should be actually moved o@directive decorator in this example.

Copy link
Author

Choose a reason for hiding this comment

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

I think isn't a best practice to declare a function into an annotation. I never see that in Java.

But I think, effectivily, the description isn't clear...

Copy link
Owner

Choose a reason for hiding this comment

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

I think isn't a best practice to declare a function into an annotation.

Totally agree. That is why I decided to move the configuration to static members, not to annotation.

Problem is that this linking function must be in this example in annotation, or it will not work at all…

Copy link
Author

Choose a reason for hiding this comment

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

I think work fine because have these instructions (line 116 - 125) :

config = <IDirectiveProperties> angular.copy(directiveSettings);

// Store FuncName as ControllerAs
config.controllerAs = getFuncName(target);
config.controller = target;

angular.forEach(directiveProperties, function(property: string): any {
    if (angular.isDefined(target[property])) {
       config[property] = target[property];
    }
})

Link method will be stored in config object.

Copy link
Owner

Choose a reason for hiding this comment

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

You are totally right. I focused on a if (typeof directiveSettings === 'string') { guard and didn't notice the part in lines 122 - 126.

So my observation is invalid.

Now I see - there is another issue. Look down to those lines…

ctrl.init(attrs.atSomeDirective);
};

constructor(
@inject('$scope') private $$scope: angular.IScope,
@inject('$parse') private $$parse: angular.IParseService
) {
// do stuff with $$scope and $$parse;
}

public init(anArg: string): boolean {
// do some stuff with this.$$parse and this.$$scope
}

}
```

or with static class members of directive controller are used as config directive config :

```typescript
@directive('ngModuleName', 'atSomeDirective')
Expand Down Expand Up @@ -242,3 +306,29 @@ class UserResource extends at.Resource {

***

### Decorator

Angular provide a decorator feature to extends a service/factory. Use @decorator to use them and adding some others methods :

```typescript
@decorator('ngModuleName', 'SomeService')
class SomeServiceDecorator {

constructor(
@at.inject('SomeService') private parent:SomeService;
) {
Copy link
Owner

Choose a reason for hiding this comment

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

Please, drop constructor from this example as current implementation of @decorator will never inject anything into it.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok. I understand, just remove @at.inject('SomeService') part

// do stuff
}

public someMethod2(anArg: number): boolean {
// do some stuff

console.log(this.parent.someMethod(anArg));
Copy link
Owner

Choose a reason for hiding this comment

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

what is parent here ? I do not see any reference in code…

Copy link
Author

Choose a reason for hiding this comment

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

Look line 318 @at.inject('SomeService') private parent:SomeService;

"private" keyword on constructor create a private attribute on your instance. Here, parent is the private attribute create on construtor and it values by SomeService.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, but @at.inject('SomeService') will do nothing there. It's ok but you will have to pass the $delegate into constructor then :)

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe let's use some convention here - parent is a common word and it may cause collision. Maybe $host or $delegate would be better?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it's a bad exemple for parent.

Copy link
Author

Choose a reason for hiding this comment

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

$delegate it's better, according to angular doc.


// do some stuff
return true;
}

}

```
11 changes: 8 additions & 3 deletions at-angular-resource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ module at {
angular.extend(instance, new instance.$_Resource(model));
}

function getFuncName(target: any): string {
/* istanbul ignore next */
return target.name || target.toString().match(/^function\s*([^\s(]+)/)[1];
}

/* istanbul ignore next */
export class Resource implements angular.resource.IResource<Resource> {
public static get: (params?: Object) => Resource;
Expand Down Expand Up @@ -43,10 +48,10 @@ module at {
}

export interface IResourceAnnotation {
(moduleName: string, className: string): IClassAnnotationDecorator;
(moduleName: string, className?: string): IClassAnnotationDecorator;
}

export function resource(moduleName: string, className: string): IClassAnnotationDecorator {
export function resource(moduleName: string, className?: string): IClassAnnotationDecorator {
return (target: any): void => {
function resourceClassFactory($resource: ResourceService, ...args: any[]): any {
const newResource: ResourceClass = $resource(target.url, target.params, target.actions, target.options);
Expand All @@ -59,7 +64,7 @@ module at {
})), ...args);
}
resourceClassFactory.$inject = (['$resource']).concat(target.$inject /* istanbul ignore next */ || []);
angular.module(moduleName).factory(className, resourceClassFactory);
angular.module(moduleName).factory(className || getFuncName(target), resourceClassFactory);
};
}
/* tslint:enable:no-any */
Expand Down
122 changes: 97 additions & 25 deletions at-angular.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ module at {
(t: any, key: string, index: number): void;
}

function instantiate(moduleName: string, name: string, mode: string): IClassAnnotationDecorator {
function getFuncName(target: any): string {
/* istanbul ignore next */
return target.name || target.toString().match(/^function\s*([^\s(]+)/)[1];
}

function instantiate(moduleName: string, mode: string, name?: string): IClassAnnotationDecorator {
return (target: any): void => {
angular.module(moduleName)[mode](name, target);
let fnName: string = getFuncName(target);
angular.module(moduleName)[mode](name || fnName, target);
};
}

Expand All @@ -55,50 +61,81 @@ module at {
}

export interface IServiceAnnotation {
(moduleName: string, serviceName: string): IClassAnnotationDecorator;
(moduleName: string, serviceName?: string): IClassAnnotationDecorator;
}

export function service(moduleName: string, serviceName: string): at.IClassAnnotationDecorator {
return instantiate(moduleName, serviceName, 'service');
export function service(moduleName: string, serviceName?: string): at.IClassAnnotationDecorator {
return instantiate(moduleName, 'service', serviceName);
}

export interface IControllerAnnotation {
(moduleName: string, ctrlName: string): IClassAnnotationDecorator;
(moduleName: string, ctrlName?: string): IClassAnnotationDecorator;
}

export function controller(moduleName: string, ctrlName: string): at.IClassAnnotationDecorator {
return instantiate(moduleName, ctrlName, 'controller');
export function controller(moduleName: string, ctrlName?: string): at.IClassAnnotationDecorator {
return instantiate(moduleName, 'controller', ctrlName);
}

export interface IDirectiveAnnotation {
(moduleName: string, directiveName: string): IClassAnnotationDecorator;
(moduleName: string, directiveName: string | IDirectiveProperties): IClassAnnotationDecorator;
}

export function directive(moduleName: string, directiveName: string): at.IClassAnnotationDecorator {
export interface IDirectiveProperties extends angular.IDirective {
selector: string;
}

export function directive(
moduleName: string,
directiveSettings: string | IDirectiveProperties
): at.IClassAnnotationDecorator {

return (target: any): void => {
let config: angular.IDirective;
const ctrlName: string = angular.isString(target.controller) ? target.controller.split(' ').shift() : null;
/* istanbul ignore else */

let config: IDirectiveProperties;

const ctrlName: string = angular.isString(target.controller)
? target.controller.split(' ').shift()
: null;

let controllerAs: string;

if (ctrlName) {
controller(moduleName, ctrlName)(target);
}
config = directiveProperties.reduce((
config: angular.IDirective,
property: string
) => {
return angular.isDefined(target[property]) ? angular.extend(config, {[property]: target[property]}) :
config; /* istanbul ignore next */
}, {controller: target, scope: Boolean(target.templateUrl)});

angular.module(moduleName).directive(directiveName, () => (config));
// Retrocompatibilty

if (typeof directiveSettings === 'string') {
directiveSettings = <IDirectiveProperties> {
selector: <string> directiveSettings
};
} else {
controllerAs = (<IDirectiveProperties> directiveSettings).controllerAs || getFuncName(target);
}

config = directiveProperties.reduce((config: angular.IDirective, property: string) => {
return angular.isDefined(target[property])
? angular.extend(config, {[property]: target[property]})
: config; /* istanbul ignore next */

}, angular.extend({}, directiveSettings, {
controllerAs: controllerAs,
controller: target
}));

/* istanbul ignore else */

angular
.module(moduleName)
.directive(config.selector, () => (config));
};
}

export interface IClassFactoryAnnotation {
(moduleName: string, className: string): IClassAnnotationDecorator;
(moduleName: string, className?: string): IClassAnnotationDecorator;
}

export function classFactory(moduleName: string, className: string): at.IClassAnnotationDecorator {
export function classFactory(moduleName: string, className?: string): at.IClassAnnotationDecorator {

return (target: any): void => {
function factory(...args: any[]): any {
return at.attachInjects(target, ...args);
Expand All @@ -107,8 +144,43 @@ module at {
if (target.$inject && target.$inject.length > 0) {
factory.$inject = target.$inject.slice(0);
}
angular.module(moduleName).factory(className, factory);
angular.module(moduleName).factory(className || getFuncName(target), factory);
};
}

export interface IDecoratorAnnotation {
(moduleName: string, targetProvider: string): IClassAnnotationDecorator;
}

export function decorator(moduleName: string, targetProvider: string): at.IClassAnnotationDecorator {
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe it should be generic (parametrized) function:

I think it should finally look like:

    export function decorator<A, B>(moduleName: string, targetProvider: string): at.IClassAnnotationDecorator {

        interface IDecoratorStatic<D> extends Function {
            new (...args: any[]): D;
        }

        return (targetClass: IDecoratorStatic<B>): void => {

            angular
                .module(moduleName)
                .config([
                    '$provide',
                    function($provide: angular.auto.IProvideService): void {

                        delegation.$inject = ['$delegate', '$injector'];

                        function delegation(
                            $delegate: A,
                            $injector: angular.auto.IInjectorService
                        ): A & B {

                            const instance: B = $injector.instantiate<B>(targetClass, {
                                $delegate: $delegate
                            });

                            return angular.extend($delegate, targetClass.prototype, instance);
                        }

                        $provide.decorator(targetProvider, delegation);
                    }]);

        };

    }

Copy link
Owner

Choose a reason for hiding this comment

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

And it would be nice if you followed such direction in code style:

    interface IDecoratorStatic<D> extends Function {
        new (...args: any[]): D;
    }

    export function decorator<A, B>(moduleName: string, targetProvider: string): at.IClassAnnotationDecorator {
        return (targetClass: IDecoratorStatic<B>): void => {
            angular
                .module(moduleName)
                .config(['$provide', ($provide: angular.auto.IProvideService): void => {
                    $provide.decorator(targetProvider, ['$delegate', '$injector', (
                        $delegate: A,
                        $injector: angular.auto.IInjectorService
                    ): A & B => angular.extend(
                        $delegate,
                        targetClass.prototype,
                        $injector.instantiate<B>(targetClass, {$delegate: $delegate})
                    )]);
                }]);
        };

    }

:D


return (targetClass: any): void => {

angular
.module(moduleName)
.config([
'$provide',
function($provide: angular.auto.IProvideService): void {

delegation.$inject = ['$delegate', '$injector'];

function delegation(
$delegate: angular.ISCEDelegateProvider,
Copy link
Owner

Choose a reason for hiding this comment

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

it is not a angular.ISCEDelegateProvider

$injector: angular.auto.IInjectorService
): any {

let instance: any = $injector.instantiate(targetClass, {
Copy link
Owner

Choose a reason for hiding this comment

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

This variable is used only once, so it can be removed - just place right hand part of expresion as 3rd param of extend below.

$delegate: $delegate
});

return angular.extend($delegate, targetClass.prototype, instance);
}

$provide.decorator(targetProvider, delegation);
}]);

};

Copy link
Owner

Choose a reason for hiding this comment

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

Functionality here is now OK :)

}
/* tslint:enable:no-any */

Expand Down
6 changes: 3 additions & 3 deletions bower.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
"name": "angular-typescript",
"version": "0.0.8",
"dependencies": {
"angular": "^1.2.0",
"angular-resource": "^1.2.0"
"angular": "1.4.0",
Copy link
Owner

Choose a reason for hiding this comment

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

1.5.2 maybe ?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe. 1.5 give me an error.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok leave it on 1.4 I'll fix errors and upgrade deps later.

"angular-resource": "1.4.0"
},
"devDependencies": {
"angular-mocks": "^1.2.0"
"angular-mocks": "1.4.0"
},
"authors": ["Jakub Strojewski <[email protected]>"],
"ignore": [
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"gulp-tslint": "4.2.2",
"gulp-typescript": "2.10.0",
"jasmine-core": "2.4.1",
"karma": "0.13.16",
"karma": "0.13.19",
"karma-chrome-launcher": "0.2.2",
"karma-coffee-preprocessor": "0.3.0",
"karma-coverage": "0.5.3",
Expand Down
13 changes: 13 additions & 0 deletions test/class-factory-spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,17 @@ describe 'annotations:', ->
expect testInstanceOne.accept
.toBe 'application/json, text/plain, */*'

describe '@classFactory (without Factory name)', ->
TestClassSecond = null

beforeEach inject (_TestClassSecond_) ->
TestClassSecond = _TestClassSecond_

it 'should create class as a service', ->

expect TestClassSecond
.toBeDefined()

expect TestClassSecond
.toBe test.TestClassSecond

3 changes: 3 additions & 0 deletions test/class-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ module test {

}

@classFactory('test')
export class TestClassSecond {}

}
Loading