Skip to content

Improve annotations #1343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 37 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
7b062e2
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett Aug 24, 2019
2cf47bf
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett Apr 21, 2021
36310b7
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett May 1, 2021
57ad651
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett May 3, 2021
9ec5940
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett May 7, 2021
ce54f28
Merge branch 'master' of https://github.com/inversify/InversifyJS
tonyhallett May 8, 2021
2073070
refactor to helper createTaggedDecorator which supports all decorator…
tonyhallett May 8, 2021
035d68c
make createTaggedDecorator permit multiple metadata
tonyhallett May 8, 2021
2e1c863
refactor tagParameter and tagProperty
tonyhallett May 8, 2021
dd92dc7
extract _ensureNoMetadataKeyDuplicates
tonyhallett May 8, 2021
08e46e6
change parameter name to indexOrPropertyDescriptor
tonyhallett May 8, 2021
ff16c9a
grammar correct historic test names
tonyhallett May 8, 2021
17a0fa7
use sinon sandbox for spying
tonyhallett May 8, 2021
3fcb065
review
tonyhallett May 9, 2021
2e5a011
wiki
tonyhallett May 9, 2021
6da7ac6
improve typing
tonyhallett May 9, 2021
a0eede0
improve decorator typing
tonyhallett May 9, 2021
c9487b8
throw error for decorated property missing inject / multiInject decor…
tonyhallett May 9, 2021
44c43f0
refactor code climate
tonyhallett May 9, 2021
2e4119f
permit injection for symbol property key
tonyhallett May 9, 2021
ee81df3
move symbol tests to container
tonyhallett May 9, 2021
5703cdf
remove callback from createTaggedDecorator. minor type changes.
tonyhallett May 10, 2021
9c75702
permit lazyserviceidentifier for multiinject
tonyhallett May 10, 2021
7124f67
correct decorator typing
tonyhallett May 10, 2021
0f565da
type target to Object
tonyhallett May 10, 2021
3137069
replace if condition
tonyhallett May 10, 2021
0fc9b09
DecoratorTarget type and throw for annotating static properties
tonyhallett May 11, 2021
b7b7741
DecoratorTarget for js decorate
tonyhallett May 11, 2021
9c39f67
add test for js inject for property
tonyhallett May 11, 2021
2756407
extract getSymbolDescription
tonyhallett May 11, 2021
b325f59
refactor: simplify types
notaphplover May 12, 2021
39f3913
_tagParameterOrProperty on constructor function
tonyhallett May 12, 2021
5a4462e
DecoratorTarget type
tonyhallett May 13, 2021
b266fe1
type predicate cast to type testing for
tonyhallett May 13, 2021
bf99ac1
Prototype type - map properties to include undefined
tonyhallett May 13, 2021
03e7bf9
Merge branch 'master' into improve-annotations
tonyhallett May 14, 2021
a2fac9b
update changelog
tonyhallett May 14, 2021
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
79 changes: 49 additions & 30 deletions src/annotation/decorator_utils.ts
Original file line number Diff line number Diff line change
@@ -1,68 +1,87 @@
import * as ERROR_MSGS from "../constants/error_msgs";
import * as METADATA_KEY from "../constants/metadata_keys";
import { interfaces } from "../interfaces/interfaces";
import { getFirstArrayDuplicate } from "../utils/js";

function tagParameter(
annotationTarget: any,
propertyName: string,
annotationTarget: Object,
propertyName: string | symbol | undefined,
parameterIndex: number,
metadata: interfaces.Metadata
metadata: interfaces.MetadataOrMetadataArray
) {
const metadataKey = METADATA_KEY.TAGGED;
_tagParameterOrProperty(metadataKey, annotationTarget, propertyName, metadata, parameterIndex);
if(propertyName !== undefined) {
throw new Error(ERROR_MSGS.INVALID_DECORATOR_OPERATION);
}
_tagParameterOrProperty(METADATA_KEY.TAGGED, annotationTarget, parameterIndex.toString(), metadata);
}

function tagProperty(
annotationTarget: any,
propertyName: string,
metadata: interfaces.Metadata
annotationTarget: Object,
propertyName: string | symbol,
metadata: interfaces.MetadataOrMetadataArray
) {
const metadataKey = METADATA_KEY.TAGGED_PROP;
_tagParameterOrProperty(metadataKey, annotationTarget.constructor, propertyName, metadata);
_tagParameterOrProperty(METADATA_KEY.TAGGED_PROP, annotationTarget.constructor, propertyName, metadata);
}

function _ensureNoMetadataKeyDuplicates(metadata: interfaces.MetadataOrMetadataArray):interfaces.Metadata[]{
let metadatas: interfaces.Metadata[] = [];
if(Array.isArray(metadata)){
metadatas = metadata;
const duplicate = getFirstArrayDuplicate(metadatas.map(md => md.key));
if(duplicate !== undefined) {
throw new Error(`${ERROR_MSGS.DUPLICATED_METADATA} ${duplicate.toString()}`);
}
}else{
metadatas = [metadata];
}
return metadatas;
}

function _tagParameterOrProperty(
metadataKey: string,
annotationTarget: any,
propertyName: string,
metadata: interfaces.Metadata,
parameterIndex?: number
annotationTarget: Object,
key: string | symbol,
metadata: interfaces.MetadataOrMetadataArray,
) {
const metadatas: interfaces.Metadata[] = _ensureNoMetadataKeyDuplicates(metadata);

let paramsOrPropertiesMetadata: interfaces.ReflectResult = {};
const isParameterDecorator = (typeof parameterIndex === "number");
const key: string = (parameterIndex !== undefined && isParameterDecorator) ? parameterIndex.toString() : propertyName;

// if the decorator is used as a parameter decorator, the property name must be provided
if (isParameterDecorator && propertyName !== undefined) {
throw new Error(ERROR_MSGS.INVALID_DECORATOR_OPERATION);
}

let paramsOrPropertiesMetadata:Record<string | symbol, interfaces.Metadata[] | undefined> = {};
// read metadata if available
if (Reflect.hasOwnMetadata(metadataKey, annotationTarget)) {
paramsOrPropertiesMetadata = Reflect.getMetadata(metadataKey, annotationTarget);
}

// get metadata for the decorated parameter by its index
let paramOrPropertyMetadata: interfaces.Metadata[] = paramsOrPropertiesMetadata[key];
let paramOrPropertyMetadata: interfaces.Metadata[] | undefined = paramsOrPropertiesMetadata[key as any];

if (!Array.isArray(paramOrPropertyMetadata)) {
if (paramOrPropertyMetadata === undefined) {
paramOrPropertyMetadata = [];
} else {
for (const m of paramOrPropertyMetadata) {
if (m.key === metadata.key) {
if (metadatas.some(md => md.key === m.key)) {
throw new Error(`${ERROR_MSGS.DUPLICATED_METADATA} ${m.key.toString()}`);
}
}
}

// set metadata
paramOrPropertyMetadata.push(metadata);
paramsOrPropertiesMetadata[key] = paramOrPropertyMetadata;
paramOrPropertyMetadata.push(...metadatas);
paramsOrPropertiesMetadata[key as any] = paramOrPropertyMetadata;
Reflect.defineMetadata(metadataKey, paramsOrPropertiesMetadata, annotationTarget);

}

function createTaggedDecorator(
metadata:interfaces.MetadataOrMetadataArray,
) {
return (target:Object, targetKey:string | symbol | undefined, indexOrPropertyDescriptor?:number | TypedPropertyDescriptor<unknown>) => {
if (typeof indexOrPropertyDescriptor === "number") {
tagParameter(target, targetKey, indexOrPropertyDescriptor, metadata);
} else {
tagProperty(target, targetKey as string | symbol, metadata);
}
};
}

function _decorate(decorators: any[], target: any): void {
Reflect.decorate(decorators, target);
}
Expand Down Expand Up @@ -90,4 +109,4 @@ function decorate(
}
}

export { decorate, tagParameter, tagProperty };
export { decorate, tagParameter, tagProperty, createTaggedDecorator };
35 changes: 2 additions & 33 deletions src/annotation/inject.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,6 @@
import { UNDEFINED_INJECT_ANNOTATION } from "../constants/error_msgs";
import * as METADATA_KEY from "../constants/metadata_keys";
import { interfaces } from "../interfaces/interfaces";
import { Metadata } from "../planning/metadata";
import { tagParameter, tagProperty } from "./decorator_utils";
import { injectBase } from "./inject_base";

export type ServiceIdentifierOrFunc = interfaces.ServiceIdentifier<any> | LazyServiceIdentifer;

export class LazyServiceIdentifer<T = any> {
private _cb: () => interfaces.ServiceIdentifier<T>;
public constructor(cb: () => interfaces.ServiceIdentifier<T>) {
this._cb = cb;
}

public unwrap() {
return this._cb();
}
}

function inject(serviceIdentifier: ServiceIdentifierOrFunc) {
return function(target: any, targetKey: string, index?: number | PropertyDescriptor): void {
if (serviceIdentifier === undefined) {
throw new Error(UNDEFINED_INJECT_ANNOTATION(target.name));
}

const metadata = new Metadata(METADATA_KEY.INJECT_TAG, serviceIdentifier);

if (typeof index === "number") {
tagParameter(target, targetKey, index, metadata);
} else {
tagProperty(target, targetKey, metadata);
}

};
}
const inject = injectBase(METADATA_KEY.INJECT_TAG);

export { inject };
19 changes: 19 additions & 0 deletions src/annotation/inject_base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { UNDEFINED_INJECT_ANNOTATION } from "../constants/error_msgs";
import { Metadata } from "../planning/metadata";
import { createTaggedDecorator } from "./decorator_utils";
import { ServiceIdentifierOrFunc } from "./lazy_service_identifier";

export function injectBase(metadataKey:string){
return (serviceIdentifier: ServiceIdentifierOrFunc) => {
return (target: Object,
Copy link
Member

Choose a reason for hiding this comment

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

I think Record<string | symbol, unknown> | interfaces.Newable<unknown> is a better type for target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not acceptable by the compiler

is not assignable to type 'Record<string | symbol, unknown>'. Index signature is missing in type

I think that I have provided a solution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interface ConstructorFunction {
    new(...args:any[]):unknown,
    readonly prototype:unknown
}
interface Prototype {
    constructor: Function;
}
export type DecoratorTarget = Prototype | ConstructorFunction;

Copy link
Member

@notaphplover notaphplover May 11, 2021

Choose a reason for hiding this comment

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

It's indeed a valid solution. We want to introduce some linter rules in the future and that's the reason to ban some types. If you want I can create a PR to your branch with equivalent types which respect this rule :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate on CI ?

By all means change the typing. I hadn't seen the banning of Function. I look forward to seeing the equivalent types.

Copy link
Contributor

Choose a reason for hiding this comment

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

we can also just add this into out tslint setup right now and then just try out what works best, since ban-types are somehow related to both linters.

What was the problem with Record<string, unknown anyway ?

Copy link
Contributor Author

@tonyhallett tonyhallett May 12, 2021

Choose a reason for hiding this comment

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

What was the problem with Record<string, unknown anyway ?

When applied to a property typescript will complain

is not assignable to type 'Record<string | symbol, unknown>'. Index signature is missing in type CLASSNAME

This is why

class ClassWithDecorators {
  constructor(@decorator param:Dependency){}
  @decorator
  //@ts-ignore
  prop:Dependency

  @decorator
  static staticProp:Dependency

}
// here is the error
const typescriptError:Record<string,unknown> = ClassWithDecorators.prototype

the js

var ClassWithDecorators = /** @class */ (function () {
    function ClassWithDecorators(param) {
    }
    __decorate([
        decorator,
        __metadata("design:type", Dependency)
    ], ClassWithDecorators.prototype, "prop", void 0);
    __decorate([
        decorator,
        __metadata("design:type", Dependency)
    ], ClassWithDecorators, "staticProp", void 0);
    ClassWithDecorators = __decorate([
        __param(0, decorator),
        __metadata("design:paramtypes", [Dependency])
    ], ClassWithDecorators);
    return ClassWithDecorators;
}());

Copy link
Contributor Author

@tonyhallett tonyhallett May 12, 2021

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PodaruDragos
tell eslint to ignore the object and the Function types where they are used. The errors is there for lazy typing and errors that arise because of that.
In this instance the type signatures are ok and definitely better than unknown and of couse any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the class being decorated had
[key:string]:unknown
Then Record<string | symbol, unknown> | interfaces.Newable<unknown> will not error

targetKey:string | symbol | undefined, indexOrPropertyDescriptor?:number | TypedPropertyDescriptor<unknown>) => {
if (serviceIdentifier === undefined) {
const className = typeof target === "function" ? target.name : target.constructor.name;
throw new Error(UNDEFINED_INJECT_ANNOTATION(className));
}
return createTaggedDecorator(
new Metadata(metadataKey, serviceIdentifier)
)(target, targetKey,indexOrPropertyDescriptor);
};
}
}
14 changes: 14 additions & 0 deletions src/annotation/lazy_service_identifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { interfaces } from "../interfaces/interfaces";

export type ServiceIdentifierOrFunc = interfaces.ServiceIdentifier<any> | LazyServiceIdentifer;

export class LazyServiceIdentifer<T = any> {
private _cb: () => interfaces.ServiceIdentifier<T>;
public constructor(cb: () => interfaces.ServiceIdentifier<T>) {
this._cb = cb;
}

public unwrap() {
return this._cb();
}
}
18 changes: 2 additions & 16 deletions src/annotation/multi_inject.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
import * as METADATA_KEY from "../constants/metadata_keys";
import { interfaces } from "../interfaces/interfaces";
import { Metadata } from "../planning/metadata";
import { tagParameter, tagProperty } from "./decorator_utils";
import { injectBase } from "./inject_base";

function multiInject(serviceIdentifier: interfaces.ServiceIdentifier<any>) {
return function(target: any, targetKey: string, index?: number) {

const metadata = new Metadata(METADATA_KEY.MULTI_INJECT_TAG, serviceIdentifier);

if (typeof index === "number") {
tagParameter(target, targetKey, index, metadata);
} else {
tagProperty(target, targetKey, metadata);
}

};
}
const multiInject = injectBase(METADATA_KEY.MULTI_INJECT_TAG);

export { multiInject };
11 changes: 2 additions & 9 deletions src/annotation/named.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import * as METADATA_KEY from "../constants/metadata_keys";
import { Metadata } from "../planning/metadata";
import { tagParameter, tagProperty } from "./decorator_utils";
import { createTaggedDecorator } from "./decorator_utils";

// Used to add named metadata which is used to resolve name-based contextual bindings.
function named(name: string | number | symbol) {
return function(target: any, targetKey: string, index?: number) {
const metadata = new Metadata(METADATA_KEY.NAMED_TAG, name);
if (typeof index === "number") {
tagParameter(target, targetKey, index, metadata);
} else {
tagProperty(target, targetKey, metadata);
}
};
return createTaggedDecorator(new Metadata(METADATA_KEY.NAMED_TAG, name));
}

export { named };
14 changes: 2 additions & 12 deletions src/annotation/optional.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
import * as METADATA_KEY from "../constants/metadata_keys";
import { Metadata } from "../planning/metadata";
import { tagParameter, tagProperty } from "./decorator_utils";
import { createTaggedDecorator } from "./decorator_utils";

function optional() {
return function(target: any, targetKey: string, index?: number) {

const metadata = new Metadata(METADATA_KEY.OPTIONAL_TAG, true);

if (typeof index === "number") {
tagParameter(target, targetKey, index, metadata);
} else {
tagProperty(target, targetKey, metadata);
}

};
return createTaggedDecorator(new Metadata(METADATA_KEY.OPTIONAL_TAG, true));
}

export { optional };
11 changes: 2 additions & 9 deletions src/annotation/tagged.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import { Metadata } from "../planning/metadata";
import { tagParameter, tagProperty } from "./decorator_utils";
import { createTaggedDecorator } from "./decorator_utils";

// Used to add custom metadata which is used to resolve metadata-based contextual bindings.
function tagged(metadataKey: string | number | symbol, metadataValue: any) {
return function(target: any, targetKey: string, index?: number) {
const metadata = new Metadata(metadataKey, metadataValue);
if (typeof index === "number") {
tagParameter(target, targetKey, index, metadata);
} else {
tagProperty(target, targetKey, metadata);
}
};
return createTaggedDecorator(new Metadata(metadataKey, metadataValue));
}

export { tagged };
5 changes: 2 additions & 3 deletions src/interfaces/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ namespace interfaces {
setCurrentRequest(request: Request): void;
}

export interface ReflectResult {
[key: string]: Metadata[];
}
export type MetadataOrMetadataArray = Metadata | Metadata[];

export interface Metadata {
key: string | number | symbol;
Expand Down Expand Up @@ -160,6 +158,7 @@ namespace interfaces {
serviceIdentifier: ServiceIdentifier<any>;
type: TargetType;
name: QueryableString;
identifier: string | symbol;
metadata: Metadata[];
getNamedTag(): interfaces.Metadata | null;
getCustomTags(): interfaces.Metadata[] | null;
Expand Down
4 changes: 3 additions & 1 deletion src/inversify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ export const METADATA_KEY = keys;
export { Container } from "./container/container";
export { BindingScopeEnum, BindingTypeEnum, TargetTypeEnum } from "./constants/literal_types";
export { AsyncContainerModule, ContainerModule } from "./container/container_module";
export { createTaggedDecorator } from "./annotation/decorator_utils"
export { injectable } from "./annotation/injectable";
export { tagged } from "./annotation/tagged";
export { named } from "./annotation/named";
export { inject, LazyServiceIdentifer } from "./annotation/inject";
export { inject } from "./annotation/inject";
export { LazyServiceIdentifer } from "./annotation/lazy_service_identifier"
export { optional } from "./annotation/optional";
export { unmanaged } from "./annotation/unmanaged";
export { multiInject } from "./annotation/multi_inject";
Expand Down
30 changes: 20 additions & 10 deletions src/planning/reflection_utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LazyServiceIdentifer } from "../annotation/inject";
import { LazyServiceIdentifer } from "../annotation/lazy_service_identifier";
import * as ERROR_MSGS from "../constants/error_msgs";
import { TargetTypeEnum } from "../constants/literal_types";
import * as METADATA_KEY from "../constants/metadata_keys";
Expand Down Expand Up @@ -48,7 +48,7 @@ function getTargets(
);

// Target instances that represent properties to be injected
const propertyTargets = getClassPropsAsTargets(metadataReader, func);
const propertyTargets = getClassPropsAsTargets(metadataReader, func, constructorName);

const targets = [
...constructorTargets,
Expand Down Expand Up @@ -130,11 +130,22 @@ function getConstructorArgsAsTargets(
return targets;
}

function getClassPropsAsTargets(metadataReader: interfaces.MetadataReader, constructorFunc: Function) {
function _getServiceIdentifierForProperty(inject:any,multiInject:any,propertyName:string | symbol, className: string):any {
const serviceIdentifier = (inject || multiInject);
if(serviceIdentifier === undefined) {
const msg = `${ERROR_MSGS.MISSING_INJECTABLE_ANNOTATION} for property ${String(propertyName)} in class ${className}.`;
throw new Error(msg);
}
return serviceIdentifier;
}

function getClassPropsAsTargets(metadataReader: interfaces.MetadataReader, constructorFunc: Function, constructorName:string) {

const classPropsMetadata = metadataReader.getPropertiesMetadata(constructorFunc);
const classPropsMetadata:any = metadataReader.getPropertiesMetadata(constructorFunc);
let targets: interfaces.Target[] = [];
const keys = Object.keys(classPropsMetadata);
const symbolKeys = Object.getOwnPropertySymbols(classPropsMetadata);
const stringKeys:(string | symbol)[] = Object.keys(classPropsMetadata);
const keys:(string | symbol)[] = stringKeys.concat(symbolKeys);

for (const key of keys) {

Expand All @@ -144,14 +155,13 @@ function getClassPropsAsTargets(metadataReader: interfaces.MetadataReader, const
// the metadata formatted for easier access
const metadata = formatTargetMetadata(classPropsMetadata[key]);

// the name of the property being injected
const targetName = metadata.targetName || key;
const identifier = metadata.targetName || key;

// Take types to be injected from user-generated metadata
const serviceIdentifier = (metadata.inject || metadata.multiInject);
const serviceIdentifier = _getServiceIdentifierForProperty(metadata.inject,metadata.multiInject,key,constructorName);

// The property target
const target = new Target(TargetTypeEnum.ClassProperty, targetName, serviceIdentifier);
const target = new Target(TargetTypeEnum.ClassProperty, identifier, serviceIdentifier);
target.metadata = targetMetadata;
targets.push(target);
}
Expand All @@ -161,7 +171,7 @@ function getClassPropsAsTargets(metadataReader: interfaces.MetadataReader, const

if (baseConstructor !== Object) {

const baseTargets = getClassPropsAsTargets(metadataReader, baseConstructor);
const baseTargets = getClassPropsAsTargets(metadataReader, baseConstructor,constructorName);

targets = [
...targets,
Expand Down
Loading