Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

"TypeScript's constructor assignments" is not working #1555

Closed
mizozobu opened this issue Feb 3, 2024 · 7 comments
Closed

"TypeScript's constructor assignments" is not working #1555

mizozobu opened this issue Feb 3, 2024 · 7 comments
Assignees

Comments

@mizozobu
Copy link

mizozobu commented Feb 3, 2024

"TypeScript's constructor assignments" as stated here is not working.

Expected Behavior

No errors are thrown.

Current Behavior

Throws Error: Missing required @inject or @multiInject annotation in: argument 0 in class Ninja.

Possible Solution

With @inject decorator, everything works perfectly.

Steps to Reproduce (for bugs)

import 'reflect-metadata';
import { Container, injectable } from 'inversify';

@injectable()
class Ninja {

  public constructor(private _dagger:Dagger) {}

  public throwDagger() {
    this._dagger.throw();
  }
}

@injectable()
class Dagger {

  public throw() {
    console.log('throw dagger');
  }
}

const container = new Container();

container.bind<Ninja>(Ninja).toSelf();
container.bind<Dagger>(Dagger).toSelf();

container.get(Ninja).throwDagger();

Context

N/A

Your Environment

  • Version used: 6.0.2
  • Environment name and version (e.g. Chrome 39, node.js 5.4): Node.js 18.19.0
  • Operating System and version (desktop or mobile): macOS 14.2.1
  • Link to your project: N/A

Stack trace

Error: Missing required @inject or @multiInject annotation in: argument 0 in class Ninja.
    at getConstructorArgsAsTarget (/xxx/node_modules/inversify/lib/planning/reflection_utils.js:78:19)
    at getConstructorArgsAsTargets (/xxx/node_modules/inversify/lib/planning/reflection_utils.js:90:22)
    at getTargets (/xxx/node_modules/inversify/lib/planning/reflection_utils.js:56:30)
    at getDependencies (/xxx/node_modules/inversify/lib/planning/reflection_utils.js:41:12)
    at /xxx/node_modules/inversify/lib/planning/planner.js:124:71
    at Array.forEach (<anonymous>)
    at _createSubRequests (/xxx/node_modules/inversify/lib/planning/planner.js:112:20)
    at plan (/xxx/node_modules/inversify/lib/planning/planner.js:154:9)
    at /xxx/node_modules/inversify/lib/container/container.js:626:46
    at Container._get (/xxx/node_modules/inversify/lib/container/container.js:596:38)
@mathysth
Copy link

Hey, I have the same problem did you found an anwser ?

@mizozobu
Copy link
Author

Nope. I'm using @inject instead.

@cloudhx
Copy link

cloudhx commented Sep 17, 2024

Have the same issue, filed another issue

@notaphplover
Copy link
Member

Hey @mizozobu, @cloudhx , I just recreated it with a test and worked like a charm:

import { expect } from "chai";
import { Container, injectable } from "../../src/inversify";

describe("Issue 1555", () => {
  it("should instantiate object with injected constructor parameters", () => {
    @injectable()
    class Katana {
      public hit() {
        return "cut!";
      }
    }

    @injectable()
    class Shuriken {
      public throw() {
        return "hit!";
      }
    }

    @injectable()
    class Ninja {
      public constructor(public katana: Katana, public shuriken: Shuriken) {}

      public fight() {
        return this.katana.hit();
      }
      public sneak() {
        return this.shuriken.throw();
      }
    }

    const container = new Container();
    container.bind<Ninja>(Ninja).to(Ninja);
    container.bind<Katana>(Katana).to(Katana);
    container.bind<Shuriken>(Shuriken).to(Shuriken);

    const ninja = container.get<Ninja>(Ninja);

    expect(ninja.katana).to.deep.eq(new Katana());
    expect(ninja.shuriken).to.deep.eq(new Shuriken());
  });
});

Please try to recreate it in codesandbox or in any other platform in which we can see the issue. We cannot identify nor solve the issue without recreating it

@rsaz
Copy link
Contributor

rsaz commented Oct 23, 2024

Hi @mizozobu and @cloudhx

I tested with the environment provided, library version, os and nodejs version, and everything works just fine. Would be nice to have your tsconfig file just to make sure you have all the necessary elements to run your container correctly as well as the Typescript lib version, if exists on your package.json

Here are the steps:

MINIMUM TSCONFIG (you can use any supported target, I am just using the latest one)

{
    "compilerOptions": {
        "target": "ES2023",
        "experimentalDecorators": true,
        "emitDecoratorMetadata": true,
        "module": "commonjs",
        "forceConsistentCasingInFileNames": true,
        "strict": true,
        "skipDefaultLibCheck": true,
        "skipLibCheck": true,
        "strictPropertyInitialization": false
    }
}

MUST HAVE DEPENDENCY

npm i reflect-metadata

Obs: Reflect metadata has to be the first thing to be initialized on top of your project

CODE

import "reflect-metadata";

import { Container, injectable } from "inversify";

@injectable()
class Dagger {
    public throw() {
        console.log("throw dagger");
    }
}

@injectable()
class Ninja {
    public constructor(private _dagger: Dagger) {}

    public throwDagger() {
        this._dagger.throw();
    }
}

const container = new Container();

container.bind<Ninja>(Ninja).toSelf();
container.bind<Dagger>(Dagger).toSelf();

container.get(Ninja).throwDagger();

Should work just fine. Please, let us know if the issue has been resolved based on the configuration I gave you, so that we can close this topic.

OUTPUT

image

@rsaz rsaz self-assigned this Oct 23, 2024
@mizozobu
Copy link
Author

When I created a brand new project with @rsaz's code and compiled it with tsc, it worked. But I'm having a hard time making it work on a remix app.

Since it just worked fine on its own, I'm guessing it's an issue with the remix compiler or just how I set it up. Here's the minimium repro. What am I missing?

https://github.com/mizozobu/inversify-remix-repro

@rsaz
Copy link
Contributor

rsaz commented Oct 23, 2024

Hi @mizozobu

The issue here is architectural. Let me walk you through the situation:

1 - In your code below, you're trying to inject a class, in this case Dagger before its definition, like below

@injectable()
  class Ninja {

    public constructor(private _dagger:Dagger) {}

    public throwDagger() {
      this._dagger.throw();
    }
  }

  @injectable()
  class Dagger {

    public throw() {
      console.log('throw dagger');
    }
  }

  const container = new Container();

  container.bind<Ninja>(Ninja).toSelf();
  container.bind<Dagger>(Dagger).toSelf();

  container.get(Ninja).throwDagger();

Change the order to:

 @injectable()
    class Dagger {
        public throw() {
            console.log("throw dagger");
        }
    }

    @injectable()
    class Ninja {
        public constructor(@inject(Dagger) private _dagger: Dagger) {}

        public throwDagger() {
            this._dagger.throw();
        }
    }

    const container = new Container();

    container.bind<Ninja>(Ninja).toSelf();
    container.bind<Dagger>(Dagger).toSelf();

    container.get(Ninja).throwDagger();

Now related to the issue referred in this ticket: Missing required @inject or @multiInject annotation in: argument 0 in class Ninja.

The reason why you're getting the issue above is just the nature of Typescript works. As we all know, Typescript doesn't have a good reflection system, so Inversify relies heavily on reflect-metadata to capture information about your classes.

When your declaration of classes are straightforward, injected outside of any extra context (in this case handleRequest), this type of declaration `constructor(private _dagger: Dagger), works out-of-the-box as reflect-metadata can do it's job. However, when are there other contexts or component layers, in your case a default react component, that has its own life cycle, can interfere in the gathering of metadata information.

Inversify resolves this issue forcing the use of the inject decorator to be able to read the metadata of the classes

1 - using the @inject() decorator, so all data is captured in the decorator level
constructor(@inject(Dagger) private _dagger: Dagger) {}

FINAL CODE

import "reflect-metadata";

import { Container, inject, injectable } from "inversify";

/**
 * By default, Remix will handle generating the HTTP Response for you.
 * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨
 * For more information, see https://remix.run/file-conventions/entry.server
 */

import { PassThrough } from "node:stream";

import type { AppLoadContext, EntryContext } from "@remix-run/node";
import { createReadableStreamFromReadable } from "@remix-run/node";
import { RemixServer } from "@remix-run/react";
import { isbot } from "isbot";
import { renderToPipeableStream } from "react-dom/server";

const ABORT_DELAY = 5_000;

export default function handleRequest(
    request: Request,
    responseStatusCode: number,
    responseHeaders: Headers,
    remixContext: EntryContext,
    // This is ignored so we can keep it in the template for visibility.  Feel
    // free to delete this parameter in your app if you're not using it!
    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    loadContext: AppLoadContext
) {
    @injectable()
    class Dagger {
        public throw() {
            console.log("throw dagger");
        }
    }

    @injectable()
    class Ninja {
        public constructor(@inject(Dagger) private _dagger: Dagger) {}

        public throwDagger() {
            this._dagger.throw();
        }
    }

    const container = new Container();

    container.bind<Ninja>(Ninja).toSelf();
    container.bind<Dagger>(Dagger).toSelf();

    container.get(Ninja).throwDagger();

    return isbot(request.headers.get("user-agent") || "")
        ? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext)
        : handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext);
}

function handleBotRequest(
    request: Request,
    responseStatusCode: number,
    responseHeaders: Headers,
    remixContext: EntryContext
) {
    return new Promise((resolve, reject) => {
        let shellRendered = false;
        const { pipe, abort } = renderToPipeableStream(
            <RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
            {
                onAllReady() {
                    shellRendered = true;
                    const body = new PassThrough();
                    const stream = createReadableStreamFromReadable(body);

                    responseHeaders.set("Content-Type", "text/html");

                    resolve(
                        new Response(stream, {
                            headers: responseHeaders,
                            status: responseStatusCode,
                        })
                    );

                    pipe(body);
                },
                onShellError(error: unknown) {
                    reject(error);
                },
                onError(error: unknown) {
                    responseStatusCode = 500;
                    // Log streaming rendering errors from inside the shell.  Don't log
                    // errors encountered during initial shell rendering since they'll
                    // reject and get logged in handleDocumentRequest.
                    if (shellRendered) {
                        console.error(error);
                    }
                },
            }
        );

        setTimeout(abort, ABORT_DELAY);
    });
}

function handleBrowserRequest(
    request: Request,
    responseStatusCode: number,
    responseHeaders: Headers,
    remixContext: EntryContext
) {
    return new Promise((resolve, reject) => {
        let shellRendered = false;
        const { pipe, abort } = renderToPipeableStream(
            <RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
            {
                onShellReady() {
                    shellRendered = true;
                    const body = new PassThrough();
                    const stream = createReadableStreamFromReadable(body);

                    responseHeaders.set("Content-Type", "text/html");

                    resolve(
                        new Response(stream, {
                            headers: responseHeaders,
                            status: responseStatusCode,
                        })
                    );

                    pipe(body);
                },
                onShellError(error: unknown) {
                    reject(error);
                },
                onError(error: unknown) {
                    responseStatusCode = 500;
                    // Log streaming rendering errors from inside the shell.  Don't log
                    // errors encountered during initial shell rendering since they'll
                    // reject and get logged in handleDocumentRequest.
                    if (shellRendered) {
                        console.error(error);
                    }
                },
            }
        );

        setTimeout(abort, ABORT_DELAY);
    });
}

OUTPUT
image

image

@inversify inversify locked and limited conversation to collaborators Oct 23, 2024
@rsaz rsaz converted this issue into discussion #1595 Oct 23, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
Status: Done
Development

No branches or pull requests

5 participants