Proposal: make use of injection-js

[sandangel]

I'm submitting a...

[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

• At this time we cannot use InjectionToken, an Angular-like feature, and have to manually type string to @Inject (I could not use const variable too, see my comment )

UPDATE: I have figured out this is an limitation of Nest when you could not declare token in the same file with Module. That's why I propose that we should use injection-js instead current DI system.

• Lack of type checking for components array declaration in Module. In Angular when we try to declare providers in @NgModule, we have nice type checking and auto completion. But we we don't have that nice feature in Nestjs @Module components declaration.

Expected behavior

We should make use of injectable-js, a light weight and well tested DI framework extracted from Angular core v4.

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

With that we can support InjectionToken for creating unique token across application as well as reuse code across server and client side. (Token in Angular use InjectionToken)

Environment

Nest version: 4.6.0


For Tooling issues:
- Node version: 8.9.4  
- Platform:  Mac

Others:

[shekohex]

I think you could use Symbol This will solve the problem about the unique tokens. OR do you mean something else by saying that @Inject() cannot use Const. ?

[sandangel]

Hi @shekohex I want to address 2 issues: 1/ assign token to a variable wont work.

// assign 'TOKEN' to variable
const TOKEN = 'TOKEN'
const URL = 'url'
@Module({
  // lack of type checking here
  components: [{provide: TOKEN, useValue: URL}]
})
class AppModule {}

@Controller('*')
class AppController {
  // this wont work
  constructor(@Inject(TOKEN) url: string) {}
}

2/ Lack of type checking in components declaration in @Module, see comment in code above. And I really think that symbols should be used for library, not application. Why we have to create our own DI framework while we already have a good one. In addition, if we use injection-js, we can share code between Angular app and Nestjs app

[kamilmysliwiec]

// this wont work
constructor(@Inject(TOKEN) url: string) {}

Why not?

[sandangel]

hi @kamilmysliwiec I have tried it with both useValue and useFactory but app has crashed with this error:

Error: Nest can't resolve dependencies of the AppController (?). Please verify whether [0] argument is available in the current context.

here is my simple app:

// app.module.ts
import { Module } from '@nestjs/common';
import { AppController } from './app.controller';
import { join } from 'path';

const DIST_FOLDER = join(process.cwd(), 'dist');
const DIST_INDEX = join(DIST_FOLDER, 'browser', 'index.html');
export const DIST_TOKEN = 'DIST_TOKEN';

@Module({
  controllers: [AppController],
  // I have tried with useValue: DIST_INDEX too
  components: [{ provide: DIST_TOKEN, useFactory: () => DIST_INDEX }]
})
export class ApplicationModule {}

// app.controller.ts
import { Controller, Get, Req, Res, Inject } from '@nestjs/common';
import { Request, Response } from 'express';

import { DIST_TOKEN } from './app.module';

@Controller('*')
export class AppController {
  renderCache: { [key: string]: string } = {};

  // use const variable not work
  // constructor(@Inject(DIST_TOKEN) private dist: string) {}
  // use string literal work
  constructor(@Inject('DIST_TOKEN') private dist: string) {}

  @Get()
  routesRender(@Req() req: Request, @Res() res: Response) {
    if (this.renderCache[req.originalUrl]) {
      return res.send(this.renderCache[req.originalUrl]);
    }

    return res.render(this.dist, { req }, (err, html) => {
      if (err) {
        console.error(err);
      }

      this.renderCache[req.originalUrl] = html;
      return res.send(html);
    });
  }
}

[kamilmysliwiec]

That's not possible. DIST_TOKEN variable and 'DIST_TOKEN' string is equal. Please, share your repo

[sandangel]

hi @kamilmysliwiec here is my repo. Please use dev branch https://github.com/sandangel/sand-admin/tree/dev

Steps:

git clone https://github.com/sandangel/sand-admin.git#dev
cd sand-admin
yarn install
yarn run ssr

source code for nest app is placed in /server folder

[kamilmysliwiec]

Have you tried moving this line:

export const DIST_TOKEN = 'DIST_TOKEN';

into a separated file? See that your app.module.ts file depends on app.controller.ts and vice versa. It creates circular dependency.

[sandangel]

@kamilmysliwiec Hi, I have put token in a separate file and it work as expected. But I think this was never a problem when using InjectionToken in Angular. You never have to put InjectionToken in a separate file. The error cyclic dependency only happens when some NgModule trying to create some service which depend on each other, not when declare TOKEN in same file with module. That's why this is an feature request.

ps: I think the error message should content more error context than just Nest can't resolve dependencies of the AppController (?)

[kamilmysliwiec]

We cannot differ real undefined and undefined that is a result of cyclic dependency, that's why error message has to be generic. According to this issue with InjectionToken - actually, I don't know, it's not a good practice to create a circular deps, and thus I don't think that we should solve this particular case at the framework-level, especially when defining tokens inside the same file as module is definitely not a good choice (cause it would always create those bidirectional dependencies)

[sandangel]

Hi @kamilmysliwiec I mean that cyclic dependency error in Angular and Nest are different:

• Angular:

  
  @Injectable()
  export class UserService {
  constructor(tokenService: TokenService) {}
  }

@Injectable() export class TokenService { constructor(userService: UserService) {} }

Angular don't know which to create first.

- Nest: declare token in the same file with module

I think this definitely a framework design problem and I think using injection-js is a good option to solve this.

[kamilmysliwiec]

Nest works same as Angular, it won't know which one to create first. Coming back to the main idea of the issue - making use of injection-js - that's not possible. I mean it just doesn't make sense. I don't see any pros in this solution. We would have to rebuild a lot of stuff without any visible profit.

[sandangel]

Hi @kamilmysliwiec,

Nest works same as Angular, it won't know which one to create first

I really don't understand what you mean. Is that Nest don't know create Token first or Controller first? or it don't know create Module first or Controller first? What is the difference between place token in separate file and don't? Is that TOKEN is just a variables and Inject take the reference to that variables to resolve dependency?

[kamilmysliwiec]

I mean that this following case:

@Injectable()
export class UserService {
  constructor(tokenService: TokenService) {}
}

@Injectable()
export class TokenService {
  constructor(userService: UserService) {}
}

Means the same for Nest.

[sandangel]

hi @kamilmysliwiec sorry for not to be clear. I do agree that both Angular and Nest will throw an error in that case and it's definitely a cyclic dependency error. What I want to mention here is define an InjectionToken in the same file with @NgModule has no problem in Angular, while define a Token in same file with @Module does not work in Nest.

/* Angular App */
// app.module.ts - declare both token and @NgModule in the same file work in Angular
const TOKEN = new InjectionToken<string>('TOKEN');
const distValue = 'distValue';
@NgModule({
  providers: [{provide: TOKEN, useValue: distToken}]
})

// app.component.ts
import {TOKEN} from './app.module';
@Component({
  template: ''
})
export class AppComponent {
  constructor(@Inject(TOKEN) dist: string) {}
}

/* Nest App */
// app.module.ts - declare both token and @Module in the same file does not work in Nest
export const TOKEN = 'TOKEN';
export const distValue = 'distValue';
@Module({
  providers: [{provide: TOKEN, useValue: distToken}]
})
export class AppModule {}

// app.controller.ts
import {TOKEN} from './app.module';
@Controller('*')
export class AppController {
  // error cannot resolve parameters
  constructor(@Inject(TOKEN) dist: string) {}
}

How this could be a cyclic dependency error ?

[sandangel]

And I have found that not only TOKEN variable, when you try to define a component in the same file with @Module, it won't work too. Does Nest enforce every component have to be in its own file?

[shekohex]

Hi @sandangel That sounds strange to me, Nest Is using Reflection under the hood , maybe imo it is a limitation of the library reflect-metadata or Typescript idk, but Angular or injection-js using another approach but it is the same concept. due to some limitation on the browser, and you know Angular SHOULD have support for massive number of browsers. actually, about month ago, I was playing with Injection-js and decided to create a simple core framework for learning purpose, something like Nest, but instead of creating my own Depancy Injection Engine, I prefered to go with Angular's, here is a minimal repo: Core 101. I learned a lot of techniques and how Angular or Nest works. anyway, I think there is no reason to change the Nest DI Engine with Angular's one. Hope this will help you.

[kamilmysliwiec]

Btw, if you want to use objects instead of strings, you can create your own Token class that just holds string.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.