deepkit / deepkit-framework

A new full-featured and high-performance TypeScript framework
https://deepkit.io/
MIT License
3.14k stars 116 forks source link

module directory name starting with 'app' will break dependency injection #353

Open NexZhu opened 1 year ago

NexZhu commented 1 year ago

To reproduce:

Create a module directory named apple or application or anything starting with app and add a config property, for example:

src/apple/module.ts:

import { Config } from '~/config'

export class AppleModule extends createModule({
  config: Config,
}) {}

And import this module in app.ts, the application will failed to start. Rename the directory to pineapple and everything works fine.

I guess there's a problematic regex somewhere like: /^app/

marcj commented 1 year ago

the application will failed to start

What does that mean? Any error message?

NexZhu commented 1 year ago

Something like:

DependenciesUnmetError: Undefined dependency "config: SomethingConfig" of ChainController(?). Type has no provider in scope http.
NexZhu commented 1 year ago

@marcj I'm experience various strange issues related to config injection, I've push a very simple reproduction repo here: https://github.com/NexZhu/deepkit-config-reprod

I have a single SomethingModule like this:

import { createModule } from '@deepkit/app'

import { Config } from '~/config'

import { SomethingConfig } from './config'
import { Service } from './service'

export class SomethingModule extends createModule({
  config: Config,
  providers: [Service],
  exports: [Service],
}) {}

Service:

import { Config } from '~/config'

export class Service {
  constructor(private config: Config['something']) {
    console.log(config.str)
  }
}

In the root config:

import { SomethingConfig } from '~/something' // This does not work
// import { SomethingConfig } from '~/something/config' // This works

export class Config {
  something: SomethingConfig = new SomethingConfig()
}

Here if I import the SomethingConfig from ~/something (re-exported from src/something/index.ts), I'll get this error:

[0] DependenciesUnmetError: Undefined dependency "config: SomethingConfig" of Service(?). Type has no provider in no scope.
[0]     at Injector.createFactoryProperty (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:661:19)
[0]     at Injector.createFactory (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:511:32)
[0]     at Injector.buildProvider (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:397:28)
[0]     at Injector.build (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:325:37)
[0]     at new Injector (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:227:14)
[0]     at RootAppModule.getOrCreateInjector (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/module.ts:359:25)
[0]     at InjectorContext.getInjector (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:885:23)
[0]     at InjectorContext.getRootInjector (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+injector@1.0.1-alpha.75_sk743gtqozjtjhzzbep32syq3m/node_modules/@deepkit/injector/src/injector.ts:889:21)
[0]     at ServiceContainer.process (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+app@1.0.1-alpha.75_t7pp4epngxsztnoxsahxnk46sa/node_modules/@deepkit/app/src/service-container.ts:95:30)
[0]     at ServiceContainer.getInjectorContext (/root/dev/deepkit-config-reprod/node_modules/.pnpm/@deepkit+app@1.0.1-alpha.75_t7pp4epngxsztnoxsahxnk46sa/node_modules/@deepkit/app/src/service-container.ts:115:14)

If I import it from ~/something/config, everything works fine. It's very hard to find out what's going wrong when dependency injection issues happen, such as above import path problem.

something/config.ts:

export class SomethingConfig {
  str: string = 'default'
}
NexZhu commented 1 year ago

Guess there's something related to module resolution order

colorcube commented 1 year ago

I'm playing around with deepkit and had also some problems to get config injected.

I have a DependenciesUnmetError when I import my Config class from a seperate package/lib in a Nx Workspace like so:

import { Config } from '@myworkspace/lib-config';

This works in main.ts, but in any other file/class (eg. controller) I get the error:

DependenciesUnmetError: Undefined dependency "allSettings: never" of IndexController(?). Type has no provider in scope http.

My workaround in main.ts:

import {GlobalConfig} from '@myworkspace/lib-config';

export class Config extends GlobalConfig {}

new App({
  config: Config,

Now injecting Config in other classes works, but not in modules:

import {createModule} from "@deepkit/app";
import {Config} from "../../main";
import {ConfigCommand} from "./config-command";
import {InitCommand} from './init-command';

export class CommandModule extends createModule({
  config: Config,
  controllers: [ConfigCommand, InitCommand],
}) {
}
import { Logger } from "@deepkit/logger";
import { cli, Command } from "@deepkit/app";
import { Config } from "../../main";

@cli.controller("config")
export class ConfigCommand implements Command {
  constructor(protected logger: Logger, private config: Config) {}

DependenciesUnmetError: Undefined dependency "config: Config" of ConfigCommand(✓, ?). Type has no provider in scope cli.

The solution is to use extend GlobalConfig in module scope again and use a ModuleConfig:

import {GlobalConfig} from '@myworkspace/lib-config';

export class ModuleConfig extends GlobalConfig {}

export class CommandModule extends createModule({
  config: ModuleConfig,
  controllers: [ConfigCommand, InitCommand],
}) {
}

This doesn't work:

import {Config} from "../../main";

export class ModuleConfig extends Config {}

Is it related to

NexZhu commented 1 year ago

@colorcube Have you tried putting GlobalConfig into the same package as your Deepkit app? May be related to Deepkit's Typescript compiler specific runtime type information which is needed for DI not added to your dependency package.

colorcube commented 1 year ago

I could resolve the problem by changing the import path setup in tsconfig.json from

"@myworkspace/lib-config": ["packages/lib-config/src/index.ts"],

to

"@myworkspace/lib-config/*": ["packages/lib-config/src/*"],

and importing GlobalConfig with it's path

import {GlobalConfig} from '@myworkspace/lib-config/model';

This way there's no need to have a local config class like:

export class ModuleConfig extends GlobalConfig {}

GlobalConfig can just be used.

This is fine for me but I guess there's something off. The path shouldn't make any difference?!

@NexZhu seems to me it's the same issue you mentioned - reexporting with index.ts and paths

NexZhu commented 1 year ago

@colorcube Yeah my guess is that there's some cyclic dependency issue going on in the way Deepkit is preparing the DI environment (context).

colorcube commented 1 year ago

still not fully resolved

When starting the app with ts-node ...

NODE_ENV=development ts-node -r tsconfig-paths/register -P packages/example/tsconfig.app.json packages/example/src/main.ts

it works as described above.

But Nx uses Webpack for build and serve. Then again config injection fails. I tried to inject the webpack config as described here https://docs.deepkit.io/english/runtime-types.html#_webpack. But no luck

PS: I think I hijacked this ticket - sorry

colorcube commented 1 year ago

I tested a bit more and tried to debug all kinds of config injection.

With ts-node the app starts but the config is not merged from env for a custom module: APP_DATABASE_URL="mongodb://27017/other"

But APP_FRAMEWORK_PORT=8888 is merged.

For example:

export class DatabaseConfig {
    url = "mongodb://localhost:27017/test"
}

export class DatabaseModule extends createModule(
    {
        config: DatabaseConfig,
        providers: [MyDatabase],
    },
    'database',
) {}

new App({
    config: AppConfig,
    imports: [
        new FrameworkModule({
            debug: true,
        }),
        new DatabaseModule(),
    ],

In parseEnv() where the .env config is merged, the reflection looks like this:

ReflectionClass {
  type: {
    kind: 20,
    classType: [class DatabaseConfig] { __type: [Array] },
    types: [],
    typeArguments: undefined
  },
  parent: undefined,
  description: '',
  data: {},
  disableConstructor: false,
  singleTableInheritance: false,
  indexes: [],
  propertyNames: [],
  methodNames: [],
  properties: [],
  methods: [],
  references: [],
  primaries: [],
  autoIncrements: [],
  subClasses: []
}

which is empty and therefore nothing is merged.

Other config classes have properties in it:

  propertyNames: [ 'listen', 'path', 'markdown', 'markdownFile' ],
  methodNames: [],
  properties: [
    ReflectionProperty {
      property: [Object],
      reflectionClass: [Circular *1],
      data: {},
      type: [Object],
      symbol: Symbol(listen)
    },
...

The configuration in the Framework Debugger is empty because of an error:

Uncaught (in promise) Error: No property url found in DatabaseConfig

If needed I would setup an example repo

colorcube commented 1 year ago

I've setup a demo repo of the bug: https://github.com/colorcube/deepkit-config-test

colorcube commented 1 year ago

This was a bit stupid but also not exactly obvious

This config doesn't work:

export class DatabaseConfig {
    url = "mongodb://localhost:27017/test"
}

This works:

export class DatabaseConfig {
    url :string = "mongodb://localhost:27017/test"
}

The type ":string" was missing

I don't know if it's mentioned in the docs that the type needs to be defined even if the property is initialized. Seems to be obvious that the type needs to be defined...

...but the default typescript eslint rule

@typescript-eslint/no-inferrable-types https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-inferrable-types.md

would mark that type annotation as unnecessary which is misleading.

I use following eslint rules to makes the missing type an error:

      "rules": {
        "@typescript-eslint/no-inferrable-types": "off",
        "@typescript-eslint/typedef": [
          "error",
          {
            "memberVariableDeclaration": true
          }
        ]
      }
marcus-sa commented 1 year ago

This was a bit stupid but also not exactly obvious

This config doesn't work:

export class DatabaseConfig {
    url = "mongodb://localhost:27017/test"
}

This works:

export class DatabaseConfig {
    url :string = "mongodb://localhost:27017/test"
}

The type ":string" was missing

I don't know if it's mentioned in the docs that the type needs to be defined even if the property is initialized. Seems to be obvious that the type needs to be defined...

...but the default typescript eslint rule

@typescript-eslint/no-inferrable-types typescript-eslint/typescript-eslint@main/packages/eslint-plugin/docs/rules/no-inferrable-types.md

would mark that type annotation as unnecessary which is misleading.

I use following eslint rules to makes the missing type an error:

      "rules": {
        "@typescript-eslint/no-inferrable-types": "off",
        "@typescript-eslint/typedef": [
          "error",
          {
            "memberVariableDeclaration": true
          }
        ]
      }

There used to be a section in the documentation where it stated that types had to be explicitly defined and couldn't be inferred.

I've forgotten it myself a couple of times, so you're not alone 😵‍💫