microsoft / tsyringe

Lightweight dependency injection container for JavaScript/TypeScript
MIT License
5.18k stars 172 forks source link

Circular solution failed in nodejs (NextJs - Api function area) #154

Open TrungRueta opened 3 years ago

TrungRueta commented 3 years ago

Describe the bug Deal Tsyringe team, First i want say thankyou for your masterpiece, tsyringe is wonderful and fast! Below is a issue i found when implement tsyringe to my project use NextJs as base:

in your document has section explains about circular dependencies when example:

@injectable()
export class Foo {
  constructor(@inject(delay(() => Bar)) public bar: Bar) {}
}

@injectable()
export class Bar {
  constructor(@inject(delay(() => Foo)) public foo: Foo) {}
}

// construction of foo is possible
const foo = container.resolve(Foo);

// property bar will hold a proxy that looks and acts as a real Bar instance.
foo.bar instanceof Bar; // true

This work well if put into test file and test use jest + ts-jest. but failed when add to a nextjs-api file. Look like circular not working correct in Nodejs ENV. Error shown:

ReferenceError: Cannot access 'Bar' before initialization. 

Attention that Nextjs use typescript along with Babel. After read next example in document i see other case you show how to resolve circular issue with Interface, this make me idea maybe issue start from how babel reference mismatch class with interface, so i try to change constructor(@inject(delay(() => Bar)) public bar: Bar) {} to constructor(@inject(delay(() => Bar)) public bar: typeof Bar) {} and code working.

I'm has not enough experience to go deeper issue, but i guess issue because code after compile think ...: Bar as a class , not reference as interface, and at the time 2 classes circular one of them is not initialization yet.

Of couse use interface as middleware is a option, but it duplicate a lot of work in project if project service classes is huge. Maybe you guys can research to have better solution.

To Reproduce

  1. create new project nextjs use yarn create next-app. Setup file and typescript follow nextjs instruction.
  2. modify tsconfig (see below)
  3. add file babelrc and add plugins (see below). This part in tsyring's document still lack info. If someone has time maybe can help clear exactly what babel plugins require for tsyringe in browser/node env.
  4. create new file in pages/api folder named test.ts add content (see below).
  5. run api , error will show.
  6. change constructors of 2 classes : Foo/Bar to : typeof Foo/Bar and run api again.

tsconfig.json

{
  "compilerOptions": {
    "target": "ES6",
    "lib": [
      "dom",
      "dom.iterable",
      "esnext"
    ],
    "allowJs": true,
    "skipLibCheck": true,
    "skipDefaultLibCheck": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noEmit": true,
    "esModuleInterop": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "jsx": "preserve",
    "baseUrl": ".",
    "paths": {
      "@/*": [
        "./*"
      ]
    },
    "experimentalDecorators": true,
    "emitDecoratorMetadata": false,
    "strictPropertyInitialization": false,
  },
  "include": [
    "next-env.d.ts",
    "**/*.ts",
    "**/*.tsx"
  ],
  "exclude": [
    "node_modules"
  ]
}

.babelrc

{
  "presets": [
    [
      "next/babel",
      // {
      //   "class-properties": {
      //     "loose": true
      //   }
      // }
    ]
  ],
  "plugins": [
    "babel-plugin-parameter-decorator",
    "babel-plugin-transform-typescript-metadata",
    [
      "@babel/plugin-proposal-decorators",
      {
        "legacy": true
        // "decoratorsBeforeExport": true
      }
    ],
    [
      "@babel/plugin-proposal-class-properties",
      {
        "loose": true
      }
    ]    
  ]
}

next js test api - pages/api/test.ts

import 'reflect-metadata';
import { NextApiHandler } from 'next';
import { container, delay, inject, injectable } from 'tsyringe';

@injectable()
export class Foo {
  constructor(@inject(delay(() => Bar)) public bar: Bar) {}
}

@injectable()
export class Bar {
  constructor(@inject(delay(() => Foo)) public foo: Foo) {}
}

const api: NextApiHandler = async (req, res) => {
  const test = container.resolve(Foo);
  console.log(test);
  return;
};
export default api;

Expected behavior Find solution where we can reference Class as Interface.

Version: "tsyringe": "^4.4.0",

emilioastarita commented 3 years ago

@TrungRueta Did you try the example giving each class a different file ?

TrungRueta commented 3 years ago

@emilioastarita Yes, in large project i tried to write each class in separate file and import together but result is same. as i can guess now issue only comes from ...: <class ref as interface> syntax. if i replace with any everything work, but in IDE we will lost type hint.

For better workaround i used this:

type BarType = Bar;

@injectable()
export class Foo {
  constructor(@inject(delay(() => Bar)) public bar: BarType) {}
}

With extra line on top i can force typescript understand that BarType is 100% type/interface not class and it look like bypassed every compilers in next steps.

karl-finch commented 3 years ago

I've also been struggling with this issue. Any help would be appreciated.

abriginets commented 3 years ago

Having the exact same issue with NextJS and it seems that NextJS itself is the reason because it uses Webpack to create chunks for you that will be deployed to serverless functions later in prod https://github.com/webpack/webpack/issues/12724

I believe Webpack somehow transforms the constructor argument types to classes or something and it affects the code that NextJS runs. Not sure if it's necessary to create an issue in their repo though since we are using decorators which are not the part of spec yet and it may become an blocker for their investigation.

@karl-finch you can fix this by explicitly extracting the old type into a new one:

@Inject(delay(() => DatabaseService)) private readonly databaseService: Readonly<DatabaseService>,
// ------------------------------------- wrapped the type into readonly ^^^^^^^^

You can use the Partial<> wrapper as well. It doesn't really matter.

zoltan-nz commented 2 years ago

After a few hours of investigation, I think, the problem here is the isolatedModules: true.

This test will fail https://github.com/microsoft/tsyringe/blob/master/src/__tests__/errors.test.ts#L55

if you add isolatedModules: true to jest.config.js: https://github.com/microsoft/tsyringe/blob/master/test/jest.config.js#L14

  globals: {
    "ts-jest": {
      isolatedModules: true,
      tsconfig: "src/__tests__/tsconfig.json"
    }
  },

With isolated modules, the Reflect provides an extra function Object() token, so the circular dependency detection fails there because it does not exist in the registry.

    Expected pattern: /Cannot inject the dependency "b" at position #0 of "A01" constructor\. Reason:\s+Cannot inject the dependency "a" at position #0 of "B01" constructor\. Reason:\s+Attempted to construct an undefined constructor. Could mean a circular dependency problem. Try using `delay` function./
    Received message: "Cannot inject the dependency \"b\" at position #0 of \"A01\" constructor. Reason:
        Cannot inject the dependency \"a\" at position #0 of \"B01\" constructor. Reason:
            TypeInfo not known for \"Object\""

If you use babel as a transpiler, you have to use isolatedModules. Also, it is a good practice in Jest as well, because it heavily speeds up the test runner. (https://kulshekhar.github.io/ts-jest/docs/getting-started/options/isolatedModules/)