tsyringe icon indicating copy to clipboard operation
tsyringe copied to clipboard

Container resolve does work anymore with default parameters since v4.4

Open woutervanbakel opened this issue 4 years ago • 20 comments

Describe the bug DI does not work properly when using default values.

Error: Cannot inject the dependency at position #0 of "Target" constructor. Reason:
    TypeInfo not known for "Object"

To Reproduce Build and run main.

import "reflect-metadata";
import {container, injectable} from "tsyringe";

@injectable()
class Target {
  constructor(subject = "hello world") {}
}

function main() {
    const t = container.resolve(Target);
}

Expected behavior t correctly resolved with t.subject == "hello world"

Version: tsyringe v4.4

woutervanbakel avatar Nov 17 '20 10:11 woutervanbakel

Does Target need the @injectable() decorator? It works if you remove the @injectable() decorator, since TSyringe will then use the constructor as written, rather than one that searches the container for tokens.

MeltingMosaic avatar Nov 17 '20 17:11 MeltingMosaic

@MeltingMosaic Yes sorry you are absolutely right. while I was minimizing the code for an easy example I missed out on the actual issue I faced. I've a class that needs to be a singleton.

import "reflect-metadata";
import {container, singleton} from "tsyringe";

@singleton()
class Target {
  constructor(subject: string = "hello world") {}
}

function main() {
    const t = container.resolve(Target);
}

This results in the following error:

Error: Cannot inject the dependency at position #0 of "Target" constructor. Reason: TypeInfo not known for "String"

It does however seems to work for Objects but not for Primitives. For example this is ok:

import "reflect-metadata";
import {container, singleton} from "tsyringe";

class Subject {}

@singleton()
class Target {
  constructor(public subject: Subject = new Subject()) {}
}

function main() {
    const t = container.resolve(Target);
}

Looks like a bug?

woutervanbakel avatar Nov 18 '20 08:11 woutervanbakel

Because this issue comes from an existing code-base I was able to do some more analysis. I found that if I downgrade to v4.3.0 it works again. Looking at the commits in 4.4.0 I do see some changes that are probably related.

woutervanbakel avatar Nov 18 '20 09:11 woutervanbakel

I tested this on 4.3.0, and it still fails:

import "reflect-metadata";
import { container, singleton } from "tsyringe";

test("should work", () => {
  @singleton()
  class Target {
    constructor(public subject: string = "hello world") {}
  }

  const t = container.resolve(Target);

  expect(t).toBeTruthy();
});

Could you point me at the code where this works on 4.3.0? It's possible I have not built a representative sample.

In your second case, it does work because the container can construct an undecorated object with a default constructor. However, the initializer will not run since Subject will be injected from the container:

test("should work", () => {
  class Subject {
    public message: string;
    constructor() {
      this.message = "Base Class";
    }
  }

  class SubjectDerivative extends Subject {
    constructor() {
      super();
      this.message = "Derived Class";
    }
  }

  @singleton()
  class Target {
    constructor(public subject: Subject = new SubjectDerivative()) {
      console.log(subject.message);
    }
  }

  const t = container.resolve(Target);

  expect(t).toBeTruthy();
});

Outputs:

    console.log
      Base Class

      at new Target (src/test.ts:22:15)

MeltingMosaic avatar Nov 19 '20 17:11 MeltingMosaic

Sorry for the late reply but I tested your examples with jest in on a local project and there it worked with 4.3.0. Did you checked your package-lock to confirm you are actually running 4.3? In my case I needed to force npm with

npm install [email protected]

woutervanbakel avatar Nov 30 '20 13:11 woutervanbakel

Yeah, I forced 4.3 (and verified that the new methods in 4.4 were absent from the JS)

MeltingMosaic avatar Dec 10 '20 23:12 MeltingMosaic

Still the same problem with 4.5.0

ffflorian avatar Mar 31 '21 12:03 ffflorian

Ok, I have a repro now.

MeltingMosaic avatar Mar 31 '21 18:03 MeltingMosaic

Hey, I have the same issue happening v4.5 but only once I import azure/service-bus I don't know if this helps or not but might be related somehow.

catharsisjelly avatar May 19 '21 13:05 catharsisjelly

Same problem here, are there any known workarounds? I'm using default arguments to enable unit testing in my case

ljleb avatar Jul 19 '21 16:07 ljleb

Still the same problem with 4.6.0

import { inject, injectable } from 'tsyringe';
import config from 'config';
import AWS from 'aws-sdk';
import Database from '../database/Database';

@injectable()
export default class SqsClient {
    constructor(
        private sqs: AWS.SQS = new AWS.SQS(),
        private queueUrl: string = <string> config.get('cu-enablement-queue-url')
    ) {
    }

    sendMessage(message) {
        const params = {
            MessageBody: JSON.stringify(message),
            QueueUrl: this.queueUrl
        };
        return this.sqs.sendMessage(params).promise();
    }
}

Error: Cannot inject the dependency at position #1 of "SqsClient" constructor. Reason:
    TypeInfo not known for "String

EDIT: In my example, if I change the "string" for an Object Tsyring will work soo I will start to think that the problem is possible with Generic Types and don't with classes

RaulGF92 avatar Jan 20 '22 12:01 RaulGF92

Why is it closed but it's not the best approach?

This is how I solved the problem indeed this solution is in the part of documentation...

https://github.com/microsoft/tsyringe#injecting-primitive-values-named-injection

import { container, inject, injectable } from 'tsyringe';
import config from 'config';
import AWS from 'aws-sdk';

container.register('SqsClientQueueUrl', {
    useValue: <string> config.get('cu-enablement-queue-url')
});

@injectable()
export default class SqsClient {
    constructor(
        private sqs: AWS.SQS = new AWS.SQS(),
        @inject('SqsClientQueueUrl') private queueUrl: string
    ) {
    }

    sendMessage(message) {
        const params = {
            MessageBody: JSON.stringify(message),
            QueueUrl: <string> this.queueUrl
        };
        return this.sqs.sendMessage(params).promise();
    }
}

But I don't think this is the best approach because you are forced to make an extern initialization... Hopefully, the dev team will implement the logical behaviour and we won't need to use that.

RaulGF92 avatar Jan 20 '22 14:01 RaulGF92

Yeah, having issues with my @singleton class:

import { singleton } from 'tsyringe';

@singleton()
export class Config {
  public readonly region: string;
  constructor(
    region?: string,
  ) {
    this.region = region || process.env.REGION || 'localhost';
  }
}

Basically I need the optional value outright ignored outside of testing, unit tests call the Config file with the constructor and add it to the container.

semiautomatixfu avatar Mar 25 '22 05:03 semiautomatixfu

Yeah, having issues with my @singleton class:

import { singleton } from 'tsyringe';

@singleton()
export class Config {
  public readonly region: string;
  constructor(
    region?: string,
  ) {
    this.region = region || process.env.REGION || 'localhost';
  }
}

Basically I need the optional value outright ignored outside of testing, unit tests call the Config file with the constructor and add it to the container.

That is your solution using Tsyring but if you need to create runtime Config instances I don't recommend you use singleton or Tsyringe, use a simple class.

import { singleton, inject } from 'tsyringe';

container.register('ConfigRegion', {
    useValue: process.env.REGION || 'localhost'
});

@singleton()
export class Config {
  public readonly region: string;
  constructor(
    @inject("ConfigRegion") region?: string,
  ) {
    this.region = region;
  }
}

RaulGF92 avatar Mar 25 '22 09:03 RaulGF92

Thanks for the input. I'm fairly unfamiliar with Tsyringe, having only moved our DI this week.

But yes, you're probably correct about a simple class, I'm was merely switching the repo to Tsyringe, I'll look at best practices once that's complete.

semiautomatixfu avatar Mar 27 '22 23:03 semiautomatixfu

Still in 4.7.0...

0xF6 avatar Jun 27 '22 22:06 0xF6

Why this problem still exists when i register object through .register(type, { useValue: val });?

0xF6 avatar Jun 27 '22 22:06 0xF6

Because there are idiots who don't care at Microsoft.

S1nPur1ty avatar Sep 24 '22 14:09 S1nPur1ty

Still having this problem in Feb 2023. Is this lib dead? Seems like a pretty big issue to me - injecting singletons as default arguments in constructors are the main thing I use tsyringe for, but if it doesnt work then I suppose I should move on?

nosachamos avatar Feb 12 '23 20:02 nosachamos

Still having this problem in Feb 2023. Is this lib dead? Seems like a pretty big issue to me - injecting singletons as default arguments in constructors are the main thing I use tsyringe for, but if it doesnt work then I suppose I should move on?

Project is dead, do not use it

0xF6 avatar Feb 12 '23 20:02 0xF6