Skip to content
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

Dependency injection compatibility or how to share code between client/backend #4487

Closed
soyuka opened this issue Apr 2, 2020 · 5 comments
Closed
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@soyuka
Copy link
Contributor

soyuka commented Apr 2, 2020

Feature Request

Explanation details

Let's say we have a full-stack application, using Angular and Nest. Side note, the repository is setup with https://nx.dev. Now, the project needs a Logger, you set up a new library ng g @nrwl/workspace:lib logger and start working on sending your logs to Loggly via http:

Logger implementation
import { Observable } from 'rxjs';

export interface LogglyConfiguration {
  token: string;
  enabled: boolean;
  tags: string[];
  silent?: boolean;
}

export abstract class Logger {
  private url: string;

  constructor(
    private http: ??? (see below),
    private configuration: LogglyConfiguration
  ) {
    this.url = `https://logs-01.loggly.com/inputs/${
      configuration.token
    }/tag/${configuration.tags.join(',')}/`;
  }

  error(error: Error) {
    this.log(error.message, { stack: error.stack });
  }

  log(message: string, payload: any) {
    if (!this.configuration.silent) {
      console.error('Sending following log to loggly: ', message, payload);
    }

    if (!this.configuration.enabled) {
      return;
    }

    this.http.post(this.url, { message, payload }).subscribe({
      error: error => console.log('Error logging in loggly:', error)
    });
  }
}

Now comes the interesting part, first the HttpImplementation in angular you'd have HttpClient from @angular/common/http, in Nest HttpService from @nest/common. I had to type something like:

    Partial<{
      post: (url: string, data: any) => Observable<any>;
    }>

But there's probably something better to do here.

About injection I first thought naively that both Injectable implementations would work together and added:

import {Injectable as NestInjectable} from "@nest/common"
import {Injectable} from "@angular/common"

@NestInjectable()
@Injectable()
class Logger {}

This fails building with errors like:

WARNING in /home/soyuka/work/deviseur/node_modules/@nestjs/common/utils/load-package.util.js 8:39-59
Critical dependency: the request of a dependency is an expression

ERROR in /home/soyuka/work/deviseur/node_modules/@nestjs/common/cache/cache.providers.js
Module not found: Error: Can't resolve 'cache-manager' in '/home/soyuka/work/deviseur/node_modules/@nestjs/common/cache'

Let's see what we can do with the LogglyConfiguration interface. Note that apparently we can't import interfaces from other packages as nest logs:

WARNING in ./apps/api/src/app/logger.ts 14:190-209
"export 'LogglyConfiguration' was not found in '@scope/logger'

Also, when using Angular you'd typically use a token to inject the configuration for example:

    {
      provide: LOGGLY_CONFIG,
      useValue: () =>
        ({
          token: environment.loggly.token,
          enabled: environment.loggly.enabled,
          tags: environment.loggly.tags
        } as LogglyConfiguration)
    }

Where LOGGLY_CONFIG is a token (useful for typings):

export const LOGGLY_CONFIG = new InjectionToken<LogglyConfiguration>(
  'loggly.configuration'
);

In Nest, the recommandation is to use a string, but this looses the advantages of strict typing with the token.

Today I have to use two different implementations in the Nest project and in the Angular project see this gist.

Solution

Ideally both injection annotation (Angular's and Nest's) should be compatible and declaring a shared service could be easier then this. Do you think that this would be possible or are they limitation that would interfere that I'm not aware of? Is your injection system based on injection-js (didn't took the time to check Nest's code).
Also, WDYT about adding an InjectionToken to Nest's injection system ?

@soyuka soyuka added needs triage This issue has not been looked into type: enhancement 🐺 labels Apr 2, 2020
@kamilmysliwiec
Copy link
Member

About injection I first thought naively that both Injectable implementations would work together and added:

Nest is an independent framework that can be used with any front-end technology. There's no reason to stay coherent with Angular implementation.

Now comes the interesting part, first the HttpImplementation in angular you'd have HttpClient from @angular/common/http, in Nest HttpService from @nest/common. I had to type something like:

See above.

In Nest, the recommandation is to use a string, but this looses the advantages of strict typing with the token.

You can also use symbols and classes as tokens, see the documentation. We don't plan to introduce another way.

Ideally both injection annotation (Angular's and Nest's) should be compatible and declaring a shared service could be easier then this

There's no reason to make them compatible. Tbh I don't think that it's even feasible anymore with Ivy.

Is your injection system based on injection-js (didn't took the time to check Nest's code).

Nope

Let's say we have a full-stack application, using Angular and Nest. Side note, the repository is setup with https://nx.dev. Now, the project needs a Logger, you set up a new library ng g @nrwl/workspace:lib logger and start working on sending your logs to Loggly via http:

Please, report this issue in the Nx repo.

@soyuka
Copy link
Contributor Author

soyuka commented Apr 2, 2020

Don't you think that this should be at least discussed ? IMHO having the ability to share dependencies between front-end and back-end development is an important subject also is allowing tools to do so. I don't see what Ivy has to do with the dependency injection system it's just simpler then before Ivy.

About the token story, I can't import an Interface and therefore can't use this as a value in provide:

WARNING in ./apps/api/src/app/logger.ts 13:190-209
"export 'LogglyConfiguration' was not found in '@xxx/logger'

@kamilmysliwiec
Copy link
Member

Don't you think that this should be at least discussed ? IMHO having the ability to share dependencies between front-end and back-end development is an important subject also is allowing tools to do so. I don't see what Ivy has to do with the dependency injection system it's just simpler then before Ivy.

You can share interfaces, DTOs, TS classes, everything that simply doesn't require NestJS-related (or Angular) decorators.

@Bomchickawowow
Copy link

Although I think the provided example of @soyuka displays not the full potential value for DI compatibility, it should at least be properly discussed!

A big benefit of a Nodejs-Backend like Nestjs in general is the isomorphism of the programming language between server and client. This enables reusability of data and functions across both network nodes, thus saving development time and preventing programming errors. To further embrace this, there should be a DI implementation which can be used in both ends of an application.

At the moment injectable classes of nestjs cannot be reused in the client, thus disallowing the sharing of services between client and server. This behavior prevents the easy usage of DI for node-independent systems like event-sourcing or offline-ability. To get it working, you have to make a workaround by creating a DI-class for every source class in a separate file, which inherits the source class. (Although this approach comes with its own problems)

Angular DI uses (basically) injection-js, which is usable in the client as well as in the server. Sadly Nestjs DI is not based on injection-js, so the compatibility is missing. As Nestjs DI-API is nearly identical to injection-js, it should not be a huge effort to exchange or adjust it.

@kamilmysliwiec
Copy link
Member

At the moment injectable classes of nestjs cannot be reused in the client, thus disallowing the sharing of services between client and server.

If you want to use classes annotated with NestJS-specific decorators you can always instruct webpack (or any other bundler you use) to swap NestJS imports with shim files (see example shim file here https://github.com/nestjs/swagger/blob/master/lib/extra/swagger-shim.ts). This should be pretty simple to accomplish.

Please, use our Discord channel (support) for such questions. We are using GitHub to track bugs, feature requests, and potential improvements.

@nestjs nestjs locked and limited conversation to collaborators Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

3 participants