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

feat(di): allow controllers to be scoped to environment #2551

Closed
wants to merge 1 commit into from

Conversation

abenerd
Copy link
Contributor

@abenerd abenerd commented Dec 12, 2023

Information

Type Breaking change
Feature No

Allow controllers to be scoped to an environment.

import {Get} from "@tsed/schema";
import {Controller} from "@tsed/di";

@Controller({
  path: "local",
  environments: ["development"]
})
export class LocalCtrl {
  @Get()
  index(): string {
    return [];
  }
}

Todos

  • Tests
  • Coverage
  • Example
  • Documentation

@abenerd abenerd requested a review from Romakita December 12, 2023 09:13
Copy link

Benchmarks

  • Machine: linux x64 | 4 vCPUs | 15.6GB Mem
  • Node: v16.20.2
  • Run: Tue Dec 12 2023 09:24:19 GMT+0000 (Coordinated Universal Time)
  • Method: autocannon -c 100 -d 10 -p 10 localhost:3000 (two rounds; one to warm-up, one to measure)
Version Router Requests/s Latency Throughput/Mb
fastify 3.29.4 54844.8 17.74 9.78
nest-fastify 8.4.3 47352.7 20.62 8.44
koa 2.13.4 38380.8 25.55 6.84
fastify-injector 3.29.4 27820.4 35.38 6.34
express 4.18.1 12025.8 82.45 2.14
tsed-koa 7.52.0 11960.4 82.89 10.45
fastify-big-json 3.29.4 11090.5 89.41 127.61
nest 8.4.3 10449.8 94.88 2.51
express-injector 4.18.1 10175.2 97.38 2.32
tsed-express 7.52.0 9460.2 104.91 1.73
express-morgan 4.18.1 6802.4 145.70 1.21

Explanation

The benchmark shows a performance difference between the frameworks. We note that Ts.ED is often last. In fact, Ts.ED uses features useful to a production application which reduce its performance.

For example, Ts.ED initializes a sandbox (async_hook) for each request in order to work in an isolated context if necessary.
It also initializes the elements necessary for monitoring requests in a log manager.

All this at a necessary cost that reflects the reality of a production application ;)

@abenerd abenerd force-pushed the feat/environment-controllers branch from d4c2ec3 to 0acd261 Compare December 12, 2023 09:25
Copy link

Benchmarks

  • Machine: linux x64 | 4 vCPUs | 15.6GB Mem
  • Node: v16.20.2
  • Run: Tue Dec 12 2023 09:35:29 GMT+0000 (Coordinated Universal Time)
  • Method: autocannon -c 100 -d 10 -p 10 localhost:3000 (two rounds; one to warm-up, one to measure)
Version Router Requests/s Latency Throughput/Mb
fastify 3.29.4 53996.8 18.03 9.63
nest-fastify 8.4.3 48112.0 20.29 8.58
koa 2.13.4 38430.6 25.50 6.85
fastify-injector 3.29.4 28270.4 34.84 6.44
express 4.18.1 12053.6 82.25 2.15
fastify-big-json 3.29.4 11616.4 85.28 133.66
tsed-koa 7.52.0 11601.6 85.32 10.14
nest 8.4.3 10865.1 91.18 2.61
express-injector 4.18.1 10164.7 97.58 2.32
tsed-express 7.52.0 9338.7 106.03 1.71
express-morgan 4.18.1 7005.6 141.44 1.25

Explanation

The benchmark shows a performance difference between the frameworks. We note that Ts.ED is often last. In fact, Ts.ED uses features useful to a production application which reduce its performance.

For example, Ts.ED initializes a sandbox (async_hook) for each request in order to work in an isolated context if necessary.
It also initializes the elements necessary for monitoring requests in a log manager.

All this at a necessary cost that reflects the reality of a production application ;)

@abenerd abenerd force-pushed the feat/environment-controllers branch from 0acd261 to 1fb96fc Compare December 12, 2023 13:37
Copy link

Benchmarks

  • Machine: linux x64 | 4 vCPUs | 15.6GB Mem
  • Node: v16.20.2
  • Run: Tue Dec 12 2023 13:47:26 GMT+0000 (Coordinated Universal Time)
  • Method: autocannon -c 100 -d 10 -p 10 localhost:3000 (two rounds; one to warm-up, one to measure)
Version Router Requests/s Latency Throughput/Mb
fastify 3.29.4 57040.0 17.03 10.17
nest-fastify 8.4.3 49241.6 19.85 8.78
koa 2.13.4 40327.3 24.29 7.19
fastify-injector 3.29.4 28645.1 34.37 6.53
express 4.18.1 12398.9 79.97 2.21
tsed-koa 7.52.0 11813.5 83.87 10.32
fastify-big-json 3.29.4 11630.2 85.20 133.80
nest 8.4.3 10965.5 90.45 2.64
express-injector 4.18.1 10341.6 95.90 2.36
tsed-express 7.52.0 9544.5 103.86 1.75
express-morgan 4.18.1 7485.5 132.33 1.33

Explanation

The benchmark shows a performance difference between the frameworks. We note that Ts.ED is often last. In fact, Ts.ED uses features useful to a production application which reduce its performance.

For example, Ts.ED initializes a sandbox (async_hook) for each request in order to work in an isolated context if necessary.
It also initializes the elements necessary for monitoring requests in a log manager.

All this at a necessary cost that reflects the reality of a production application ;)

@Romakita
Copy link
Collaborator

Romakita commented Dec 14, 2023

Hello @abenerd
it's a really good idea ! But your option is limited to the controller decorator and it would be great to have it for any kind of provider :). I suggest you to perform the filter implementation on DI level directly.

So to do that:

export function resolveControllers(settings: Partial<TsED.Configuration>): TokenRoute[] {
  const providers = lookupProperties.flatMap((property) => getTokens(settings[property]));
  
   /// get env from settings
  const env = settings.env

  /// filter this list
  return [...resolveRecursively(providers), ...providers].filter((provider) => !!provider.route) as any[];
}

Implementation: https://github.com/tsedio/tsed/blob/production/packages/di/src/common/utils/createContainer.ts#L5

export function createContainer(rootModule?: Type<any>) {
 // GlobalProviders.entries() => returns Provider
  const container = new Container(GlobalProviders.entries());

  if (rootModule) { // can be removed because there is no usage of rootModule signature in the code base (I check on tsed/cli also
    container.delete(rootModule);
  }

  return container;
}

Suggested:

import {Env} from "@tsed/core"
export function createContainer(env?: Env) { //or settings: DIConfiguration ?
  /// change the code implementation to 
}

To cover all this, we can do a test as follows:

@Controller({path: "/", environments: [Env.DEV]})
class MyController {
}

describe('Test env', () => {
   describe('Env = DEV', ()=> {
     beforeEach(() => PlatformTest.create({env: Env.DEV})
     afterEach(() => Platform.reset())
     
     it('should return the MyController provider', () => {
         expect(PlatformTest.injector.get(MyController)).toBeInstanceof(MyController)
     })
   })

    describe('Env = PROD', ()=> {
       beforeEach(() => PlatformTest.create({env: Env.PROD})
       afterEach(() => Platform.reset())
     
       it('should return the MyController provider', () => {
          expect(PlatformTest.injector.get(MyController)).not.toBeDefined()
      })
    })
})

See you :)

@abenerd
Copy link
Contributor Author

abenerd commented Dec 14, 2023

You're right that would be even better!
In your snippets you're using the Env Enum, I would actually like to avoid that and use a simple string to give people more control over what their environments are called, names like staging, and super-cute-cats would not be possible with that. @Romakita

@abenerd abenerd marked this pull request as draft December 14, 2023 09:42
@Romakita
Copy link
Collaborator

Yes you can use a simple string type instead of enum ;)

@Romakita Romakita force-pushed the production branch 6 times, most recently from f37ccd1 to 5306644 Compare February 18, 2024 10:58
@abenerd abenerd closed this Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants