[Types]: unmanaged decorator should return a function able to receive an undefined targetKey param

[notaphplover]

Summary

The unmanaged decorator (at lib/annotation/unmanaged.js) receives a targetKey param. Looking at type definitions:

Typescript

declare function unmanaged(): (target: any, targetKey: string, index: number) => void;

In strict type check mode, target key must be a string. This targetKey could also be undefined

Looking at the source files:

lib/annotation/unmanaged.js

function unmanaged() {
    return function (target, targetKey, index) {
        var metadata = new metadata_1.Metadata(METADATA_KEY.UNMANAGED_TAG, true);
        decorator_utils_1.tagParameter(target, targetKey, index, metadata);
    };
}

Looking at the tagParameter function:

lib/annotation/decorator_utils.js

function tagParameter(annotationTarget, propertyName, parameterIndex, metadata) {
    var metadataKey = METADATA_KEY.TAGGED;
    _tagParameterOrProperty(metadataKey, annotationTarget, propertyName, metadata, parameterIndex);
}

...

function _tagParameterOrProperty(metadataKey, annotationTarget, propertyName, metadata, parameterIndex) {
    var paramsOrPropertiesMetadata = {};
    var isParameterDecorator = (typeof parameterIndex === "number");
    var key = (parameterIndex !== undefined && isParameterDecorator) ? parameterIndex.toString() : propertyName;
    if (isParameterDecorator && propertyName !== undefined) {
        throw new Error(ERROR_MSGS.INVALID_DECORATOR_OPERATION);
    }
    if (Reflect.hasOwnMetadata(metadataKey, annotationTarget)) {
        paramsOrPropertiesMetadata = Reflect.getMetadata(metadataKey, annotationTarget);
    }
    var paramOrPropertyMetadata = paramsOrPropertiesMetadata[key];
    if (!Array.isArray(paramOrPropertyMetadata)) {
        paramOrPropertyMetadata = [];
    }
    else {
        for (var _i = 0, paramOrPropertyMetadata_1 = paramOrPropertyMetadata; _i < paramOrPropertyMetadata_1.length; _i++) {
            var m = paramOrPropertyMetadata_1[_i];
            if (m.key === metadata.key) {
                throw new Error(ERROR_MSGS.DUPLICATED_METADATA + " " + m.key.toString());
            }
        }
    }
    paramOrPropertyMetadata.push(metadata);
    paramsOrPropertiesMetadata[key] = paramOrPropertyMetadata;
    Reflect.defineMetadata(metadataKey, paramsOrPropertiesMetadata, annotationTarget);
}

As you can read, if propertyName is undefined, the key takes the parameterIndex string value.

Why do I think this matters?

The current type definition won't allow me to call Reflect.decorate with the unmanaged decorator as long as it requires a ClassDecorator[] decorators param:

import {  injectable, unmanaged } from 'inversify';

class A {
  constructor(public readonly param1: number) {}
}

export const B: typeof A = Reflect.decorate(
  [
    injectable(),
    (target: unknown) => {
      unmanaged()(target, undefined, 0);
    },
  ],
  A,
) as typeof A;

This code throws an error if typescript is in strict type checking mode: Argument of type 'undefined' is not assignable to parameter of type 'string'.

Even if I could use the inversify decorate function, I want to directly call the Reflect API and the unmanaged decorator accepts undefined as targetKey. You can even see it in any compiled code.

This ts code:

import { injectable, unmanaged } from 'inversify';

import { Capsule } from '../../../common/domain';
import { TaskGraphNode, TaskGraphNodeStatus } from './TaskGraphNode';

@injectable()
export abstract class BaseTaskGraphNode<TId, TOutput>
  implements TaskGraphNode<TId, TOutput> {
  private innerOutput: Capsule<TOutput> | null;
  private innerPerformPromise: Promise<null | Capsule<TOutput>> | undefined;
  private innerStatus: TaskGraphNodeStatus;

  constructor(
    @unmanaged()
    public readonly dependsOn: Iterable<TId>,
    @unmanaged()
    public readonly id: TId,
  ) {
    this.innerOutput = null;
    this.innerPerformPromise = undefined;
    this.innerStatus = TaskGraphNodeStatus.NotStarted;
  }
}

Is compiled into the following one:

"use strict";
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = (this && this.__metadata) || function (k, v) {
    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
var __param = (this && this.__param) || function (paramIndex, decorator) {
    return function (target, key) { decorator(target, key, paramIndex); }
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.BaseTaskGraphNode = void 0;
const inversify_1 = require("inversify");
const TaskGraphNode_1 = require("./TaskGraphNode");
let BaseTaskGraphNode = class BaseTaskGraphNode {
    constructor(dependsOn, id) {
        this.dependsOn = dependsOn;
        this.id = id;
        this.innerOutput = null;
        this.innerPerformPromise = undefined;
        this.innerStatus = TaskGraphNode_1.TaskGraphNodeStatus.NotStarted;
    }
};

BaseTaskGraphNode = __decorate([
    inversify_1.injectable(),
    __param(0, inversify_1.unmanaged()),
    __param(1, inversify_1.unmanaged()),
    __metadata("design:paramtypes", [Object, Object])
], BaseTaskGraphNode);
exports.BaseTaskGraphNode = BaseTaskGraphNode;
//# sourceMappingURL=BaseTaskGraphNode.js.map

Which is calling inversify_1.unmanaged() decorator with an undefined targetKey param.

Expected Behavior

I expect unmanaged declare function to return a function with a string | undefined targetKey:

declare function unmanaged(): (target: any, targetKey: string | undefined, index: number) => void;
export { unmanaged };

Current Behavior

The unmanaged declare function returns a function which requires a string targetKey instead.

Possible Solution

See the Expected Behavior section

Steps to Reproduce (for bugs)

1. Create a project with Typescript 4.x with strictNullChecks option enabled.
2. Install reflect-metadata and inversify packages (latest version)
3. Create a the Dummy.ts file in the source folder of your project:

src/Dummy.ts

import { injectable, unmanaged } from 'inversify';

class A {
  constructor(public readonly param1: number) {}
}

export const B: typeof A = Reflect.decorate(
  [
    injectable(),
    (target: unknown) => {
      unmanaged()(target, undefined, 0);
    },
  ],
  A,
) as typeof A;

The compiler should throw the following error: Argument of type 'undefined' is not assignable to parameter of type 'string'

Context

I want to directly call the Reflect.decorate and provide unmanaged as one of the decorator elements. You can see more details in the Why do I think this matters? section

Your Environment

• NodeJS 12.X (not relevant).
• Typescript 4.X with strictNullChecks option enabled.
• inversify 5.0.5
• reflect-metadata 0.1.13

Stack trace

There's no stack trace associated

[sandeepraheja]

This issue is still happening with TS 5 and Inversify 6. Any resolution?

[anthonyma94]

Now that this is merged into master, when will we see an npm release of it?