nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
66.77k stars 7.55k forks source link

Extracting Express in a seperate module #531

Closed BrunnerLivio closed 5 years ago

BrunnerLivio commented 6 years ago

Big fan of this this framework, good work! I created a REST API with it and it works like a charm.

My use case is an application, which has nothing to do with an webserver. Its a standalone application. Now it would be really nice to still use decorators like @Component or @Module etc. If I would want to have these features, I have to use @nestjs/core as dependency in my application. Now I have the dependency express, even though my application does not even need it.

It would be nice to split the functionality of all the webserver-stuff in a seperate module. For me atleast, that would fit the pattern. Want NestJS? Install @nestjs/core. Need a webserver? Install @nestjs/web. Need websockets? Install @nestjs/websocket Need an orm? Install @nestjs/typeorm

Are there any plans to extract express in an seperate module?

shekohex commented 6 years ago

There's a helpful Links in Documentation see.

Execution Context :https://docs.nestjs.com/execution-context and what about a Microservice ? see https://github.com/nestjs/nest/issues/404 and here is awesome module by @fwoelffel called nest-bull Hope this helped

BrunnerLivio commented 6 years ago

Thanks @shekohex for the quick response. I know about these features, but that is not what I am refering to.

I would like to use the injection / module / component functionality from NestJS, without having all the Webserver stuff.

My use case is an application, which has nothing to do with an webserver. Its a standalone application. Now it would be really nice to still use decorators like @Component or @Module etc. If I would want to have these features, I have to use @nestjs/core as dependency in my application. Now I have the dependency express, even though my application does not even need it.

It would be nice to split the functionality of all the webserver-stuff in a seperate module. For me atleast, that would fit the pattern. Want NestJS? Install @nestjs/core. Need a webserver? Install @nestjs/web. Need websockets? Install @nestjs/websocket Need an orm? Install @nestjs/typeorm

This would bring a new level of modularity to NestJS. This framework could be used in dozen more application use cases, without having to worry about having uneeded dependencies in your project

shekohex commented 6 years ago

That's what we were talking about a long time ago with Kamil , and seems that we are going to have something like that soon in V5 but anyway, I was playing with Angular Depancy Injection Engine, and created small Module wrapper around it so it could be Modulear, see Core101 it's not a framework or even ready to use, but it could be. it was built for just learning. i thought it could give you an inspiration to build yours Hope I helped.

BrunnerLivio commented 6 years ago

That's what we were talking about a long time ago with Kamil , and seems that we are going to have something like that soon in V5

Thats awesome! Is this issue already a duplicate? If not, I think we should let it open for an open discussion, until it is merged.

see Core101

Nice work. Unfortunately I'm not allowed to use such "hacky" libraries in the company I'm working at. Building my own library is also not a good idea. I prefer to wait, until NestJS comes up with a solution

we are going to have something like that soon in V5

How is the progress with that? On the branch update/version-5 the core package seems to still heavily rely on express-features.

shekohex commented 6 years ago

I mean by smaller something like this.. to not only rellay on Expressjs, we could use another adapters or even, without any HttpServer at all, by just using NextJS Context, but I don't think we are going to sprate the adapters. maybe Kamil has another Idea or solution for this.

kamilmysliwiec commented 6 years ago

Hi @BrunnerLivio, The nest ecosystem can be easily used as a standalone application by making use of execution context feature. However, in what I have understood, you would like to use @nestjs/core without a necessity to install express as a part of it. The question is, why? I mean, express library is tiny, and it doesn't need a lot of resources. It's a part of a @nestjs/core because the most popular Nest use-case is still - a web application. Extracting even this single HTTP library would force everyone to separately install this package. That's why I'm wondering, what is more important since even though you install express, you don't have to use it. 🙂 Is there any reason why you cannot keep this dependency?

BrunnerLivio commented 6 years ago

Thanks for the feedback @kamilmysliwiec. By the way, I posted your framework on reddit. Many interested people :)

The question is, why? I mean, express library is tiny, and it doesn't need a lot of resources.

Imagine if any framework / library would say this. Then you would pull for any framework / library unneeded dependencies. What I'm trying to elaborate is the NodeJS-philosophy. Small packages, so you can download what you actually need.

Another thing is, in our company we need to validate frameworks / tools etc. The more dependencies it has, the harder it gets to validate. Off course, Express would not be a problem because it is well tested and widely used.

Also, I could imagine the core of the framework could then be used for frontend development? I would love that, because in some cases you do not want to have full framework like Angular 5 with their component-Functionality.

kamilmysliwiec commented 6 years ago

Hi @BrunnerLivio, With v5.0.0 you'll be able to use various (now express/fastify) adapters. This feature makes Nest a bit more portable and extensible so far. However, the plan is to completely extract express within v6.0.0 which is the next major release. Thanks for convincing me 🙂

BrunnerLivio commented 6 years ago

Oh wow thats awesome, thanks alot! Looking forward to contribute more to NestJS.

marcus-sa commented 6 years ago

I'm actually tearing apart all the server stuff from this framework at the moment, so I'm actually curious to hear how exactly you were planning on doing it @kamilmysliwiec

wbhob commented 6 years ago

A proposal I made a few months ago was to separate as such:

@nest/core - contains decorators and logic that most apps will need (currently split into common and core for some reason) @nest/platform-{express,fastify,etc} - contains additional logic needed to run webservers @nest/websockets - socket functionality @nest/microservices - microservices functionality etc.

This way, it more closely mirrors Angular's way of doing things. One can create a library with just @angular/core, but lib developers don't have that luxury with Nest as of right now.

@kamilmysliwiec knows I've really been pushing this hard for a long time :) I truly think it will be a big win for developers long-term. Yes, the migration will be a bit of a pain, but it will result in fewer silly errors (like the 'js' in @nestjs), and more consistency for devs coming from Angular.

wbhob commented 6 years ago

Also, splitting out frameworks would be good so that nest-middlewares can identify the framework in use and spit out the right middleware for the framework :)

BrunnerLivio commented 6 years ago

@nest/platform-electron anyone?

wbhob commented 6 years ago

Electron is a desktop application container, not a server framework @BrunnerLivio so idk how that would work

BrunnerLivio commented 6 years ago

@wbhob depends how much of @nest/core gets extracted I guess? If there's basically just @Inject and @Module-functionality in @nest/core, then I think it would be pluggable to Electron? But not sure how these decorators are built behind the scenes.

wbhob commented 6 years ago

you would use something like angular with Electron (https://electronjs.org/). Nest is a server framework and can't run in a container like electron, you would need to use a frontend framework instead

like using the decorators from nest in electron doesn't have a valid use case

BrunnerLivio commented 6 years ago

Oh boy its been a long day. Sorry I just had my head somewhere else I guess. You're right off course haha

marcus-sa commented 6 years ago

@BrunnerLivio if you want to use the same architecture for Electron, I'd suggest you checked out mine https://github.com/marcus-sa/one/tree/master/examples/electron

BrunnerLivio commented 6 years ago

@marcus-sa Looks nice! Really like the @Window()-decorator. Lurking through your repo I suspect you've built your own framework? So this is not compatible with Nest right?

marcus-sa commented 6 years ago

@BrunnerLivio you're right it's not compatible with Nest in any way. This uses Inversify which of course has it's limitations compared to coding my own Ioc, but in the end it's a way better solution because it'll make 3rd party modules way easier to intregate with providers.

Using MODULE_INITIALIZER it let's me modify the container before other modules / providers are injecting them.

https://github.com/marcus-sa/one/blob/22f72985bfabf96687a61a3437d93e6d9efe16ee/packages/electron/src/windows/windows.module.ts#L5-L32

https://github.com/marcus-sa/one/blob/22f72985bfabf96687a61a3437d93e6d9efe16ee/packages/electron/src/windows/windows.service.ts#L26-L38

https://github.com/marcus-sa/one/blob/22f72985bfabf96687a61a3437d93e6d9efe16ee/examples/electron/src/main/windows/main.window.ts#L5-L14

https://github.com/marcus-sa/one/blob/22f72985bfabf96687a61a3437d93e6d9efe16ee/packages/core/src/module/module.ts#L111-L140

So that'll inject the reference of BrowserWindow into the Window provider before it's being used anywhere.

(Doesn't show permalinks, dafuq?)

BrunnerLivio commented 6 years ago

I think we should concretely discuss how the new API will look like to use NestJS running with or without Express / Fastify, so we can estimate how drastic the breaking changes of 6.0.0 will be and how much time it will take to implement something like

Prestudy

Angular uses the packages @angular/platform-browser and @angular/platform-server to configure the platform it should run on. Lets have a look at @angular/platform-browser. This package exposes a BrowserModule, when imported into the ecosystem will automatically run Angular in the browser without any configuration

app.module.ts in an Angular Project


@NgModule({
  imports: [
    BrowserModule
    [...]
   ]
})
export class AppModule {}

I think we can potentially adapt this for NestJS. The problem is with this approach, you have to bootstrap the Express instance in the ecosystem of Nest, which is in some cases good enough, but in some cases you want to do this outside of any module, in a main.ts like before with the NestFactory

This problem could be solved with the approach Angular Universal used. With ngExpressEngine you can specify which providers are going to be injected. I think we could expand on this and also support imports.

main.ts in an Angular project


app.engine('html', ngExpressEngine({
  bootstrap: AppServerModuleNgFactory,
  providers: [
    provideModuleMap(LAZY_MODULE_MAP)
  ]
}));

Proposal

outside Context

Before

main.ts

import { NestFactory } from '@nestjs/core';
import { ApplicationModule } from './app.module';
import * as http from 'http';

async function bootstrap() {
  const server = http.createServer();
  const app = await NestFactory.create(ApplicationModule, server);
  await app.listen(3000);
}
bootstrap();

After main.ts

import { NestFactory } from '@nestjs/core';
import { ExpressModule } from '@nestjs/platform-express';
import { ApplicationModule } from './app.module';
import * as http from 'http';

async function bootstrap() {
  const server = http.createServer();
  const app = NestFactory.create({
    bootstrap: ApplicationModule,
    imports: [
      ExpressModule.forRoot({
        server,
        port: 3000
      })
    ]
  });

  // maybe also change `listen` to `bootstrap` or something different, because if
  // you would run it without an ExpressModule, the name `listen` would not make sense
  await app.listen();
}
bootstrap();

inside Context

app.module.ts

import { ExpressModule } from '@nestjs/platform-express';

@Module({
  imports: [ExpressModule]
})
export class ApplicationModule {}
kamilmysliwiec commented 6 years ago

The problem is that each possible solution complicates API instead of simplifying it, which does not seem to be a good idea.

BrunnerLivio commented 6 years ago

The problem is that each possible solution complicates API instead of simplifying it, which does not seem to be a good idea.

In my opinion it does not complicate it in most cases. If you do not care how the HTTP/Express instance is going to be initiated (which most people probably do not care), you can just easily import the ExpressModule into your ApplicationModule as described in "inside Context" from my previous comment.

For the more advanced use cases it may introduce more code (as described in "Propsoal>oustide Context>After" in my previous comment), but in my opinion modularity and consistency has its cost. But I do not think it is more complicated, it is just more code. But thanks to the Nest CLI you could bypass the initial boilerplate code.

kamilmysliwiec commented 6 years ago

create() method is supposed to be used with web apps. Plain context is created with createApplicationContext(). What if someone doesn't import anything, neither ExpressModule nor Fastify? Nest is mainly a web framework anyway, I believe we should provide a default option, for example, use platform-express internally.

BrunnerLivio commented 6 years ago

With my vision there would be no difference between createApplicationContext() and create(). The only purpose of NestFactory is to create an application context. The ExpressModule / FastifyModule will handle the HTTP tasks completely by itself. This would mean the class NestApplication would be completely removed and only NestApplicationContext would be used.

kamilmysliwiec commented 6 years ago

It would be worth considering the balance of usability in relation to the cost of implementation. Such changes would affect Nest ecosystem heavily, force a lot of people to adjust to the breaking changes and at the same time, brings a doubtful overall profit (since createApplicationContext() is in place already).

kamilmysliwiec commented 6 years ago

Closed by accident. Reopening

BrunnerLivio commented 6 years ago

I really do not think it would not break too much. Sure, the downside is every application would have to migrate, but in my opinion it would be pretty easy to migrate.

But maybe I'm just too ignorant and not seeing how much it would effect the ecosystem. So easiest way to find out would be to just run a feasibility. I'll try to get a ExpressModule as described running, just with the current createApplicationContext(). If I realize that this would be too hard, or it would break the public API too extremely, then we should definitely go a different route. But first I have to focus on other issues, before I can hack around :)

kamilmysliwiec commented 5 years ago

My thoughts:

  1. Extract express specific stuff, respectively: ExpressAdapter, ExpressFactory and all related interfaces to @nestjs/platform-express + MulterModule and FileInterceptor which is compatible only with express.
  2. Same with socket.io, ws and fastify ^
  3. Since express would be still used by default, we could dynamically require express package (same as we did we with class-validator inside ValidationPipe for example)
  4. A separate package enables us to force the installation of @types/express so we could provide a strong typings for express instance.

All this stuff shouldn't break API and should require a minimal amount of additional work.

shekohex commented 5 years ago

If we need more developer experience on migration, @nestjs/cli should help on this.

I think if we could have a sub-command nest fix or nest migrate or maybe nest doctor

Which would do a static analysis on the project or on the package.json itself and provide a warn or a notic about missing packages or @types.

BrunnerLivio commented 5 years ago

@kamilmysliwiec I think this would be a great step forward. But a downside what bugs me is, if a user want to create his own platform, he’d have to modify the core (correct me if I’m wrong). With Angular it is the other way round and in my opinion a bit of a cleaner solution. You simply have to use createPlatform.

What are your thoughts on this?

—-

@shekohex so you mean the equivelant of angular/upgrade ng upgrade => nest upgrade

Could be something for another issue

wbhob commented 5 years ago

I think we're taking this idea of "breaking api" too seriously. Things break between major versions, and it's for the overall, long-term improvement of the library. As long as we're offering adequate commnunity support (migration guides and explanations, primarily), I think devs will appreciate a cleaner design.

kamilmysliwiec commented 5 years ago

@kamilmysliwiec I think this would be a great step forward. But a downside what bugs me is, if a user want to create his own platform, he’d have to modify the core (correct me if I’m wrong). With Angular it is the other way round and in my opinion a bit of a cleaner solution. You simply have to use createPlatform.

He wouldn't have to modify the core in this solution :) create method would be changed to somewhat more generic one which accepts T parameter and returns a mixin object for example (standard app + merged platform-specific methods) = it would give flexibility & typings and wouldn't break API.

I think we're taking this idea of "breaking api" too seriously. Things break between major versions, and it's for the overall, long-term improvement of the library. As long as we're offering adequate commnunity support (migration guides and explanations, primarily), I think devs will appreciate a cleaner design.

The problem really is is that this change doesn't help 99% of the community at all. I would prefer to achieve cleaner design without forcing them to change stuff which completely doesn't affect them. We need to keep in mind that Nest is being used in the vast majority to create web apps.

kamilmysliwiec commented 5 years ago

What I mean by cleaner design is cleaner syntax/style. And, to that end, it does affect them because they will have a cleaner, more maintainable codebase. My one qualm with Nest apps, which I have already expressed, is that it's not logically organized. That isn't your fault, since Nest was never designed to be this big; when you started Nest, you just had to keep adding bits and pieces, and now it's in someways more cluttered than it needs to be.

I tend to disagree. The way of defining whether something is clean or not depends on the point of personal view and overall concept of the framework. Yours is different and it's fine but the way how you articulate your opinion is very unpleasant ("that isn't your fault" suggests in advance that someone made a mistake, it's unkind in such conversation and it's sad that you're doing it for such a long time already). Nest was designed not to be a copy of Angular(!), but also to stay as close to express as possible. This library is used across majorities of Node.js apps and it was a very important goal to make the transition smooth for most of the people. The target was always to become a web-platform and at the same time, to keep codebase platform-agnostic and flexible (but still with keeping in mind that web is the biggest target, the most essential one and steps to start building web app should be very easy, pretty straightforward). Please, keep in mind that we're going our path, we don't want to reflect Angular but combine what is best from both worlds.

Also, I would like to emphasize that I really care about the opinion of the community. People like @shekohex and @BrunnerLivio are heavily committed, they take care of dedicated 3rd-party libraries, they help with issues and are always open to help people which is incredibly awesome and I really appreciate it because it helps me a lot. Besides, they tend to express their opinion and propose solutions in a kind way, keeping the discussion away from personal attacks which I respect very strongly. I have my own vision how the core maintainers team should be built and I'm going to follow it because it's definitely a requirement to have a solid support of the people, who can help with additional packages as well as with typical issues & and core stuff. And this team is forming "automatically" in the background (without "offerring"), I believe that these people who have a real impact on the framework (like @BrunnerLivio who have recently become an organization member, @shekohex, @nartc with its swagger commitment, @weeco, @jbpionnier who went through the whole codebase already I believe 😅 and a lot of other people [I just can't mention everyone involved!] who are making pull requests that provide new features, propose functionalities, creates libraries and so on) and at the same time are able to have friendly and cultural conversations will at some point become the part of a bigger team (obviously, if they want to).

Furthermore, I would like to point out that you are already talking about the exact same topic in several places, in different issues, hence, this is my last response to your post with such suggestions ^ you can use it as a reference everywhere.

bashleigh commented 5 years ago

I think nest is a great framework! I've enjoyed learning more about the framework and functionality of it and contributing to third party packages in the nestjs-community. I hope I've not been a burden in doing so? If there's anything you wish for me to add to the packages please let me know! I'm worried about the fact that we had a logo made that was similar to nest was a bit of a dick move? I thought it was quite cool to follow on the cat theme! I also didn't want to just slap a nestjs logo on there without permission? Please let me know if there's anything you wish me to add!

I'd love to contribute to the main nestjs packages if possible? I'm just not sure what I can or shouldn't do! Plus I still have a lot to learn about TypeScript (as you've probably seen from my PRs).

Going forward I would like to build a tutorial and job listing site for nestjs if that's ok? I think some basic and advanced tutorials would help people understand the inner working of nestjs and what can be achieved! I could add these to the docs repo I suppose? Would you like me to add that? For example, recently I discovered that I can use the moduleRef with the modulesContainer to 'extract' providers from other modules which is great because I can create 'custom' decorators for different services https://github.com/nestjs-community/nestjs-braintree/blob/master/src/braintree.webhook.module.ts#L27-L56 I think this would help people create more third party packages! However, I'm not sure it's the best method? I did however get a little confused with the names of the ModulesContainer and the ModuleRef. I think that's probably because I'm used to using a container to store my services (Symfony and Laravel). But! I do think I sort of understand the reasons for the different names? The ModuleRef is storing the references for the services and the container is used for building a picture of the services prototypes or something like that?

I recently attended a Smyfony live conference and it's inspired me to contribute more to open source!
Sorry if this was the wrong place to add this comment (probably is :joy:)! Been meaning to add the first paragraph for a while! Also I've been asked a few times to do some public speaking on nestjs in my local area (Colchester digital). About 20-30 people would probably attend (apparently recorded :scream:). @kamilmysliwiec are you cool for me to do that? I know it's not that many. Plus is there anything you would you like me to speak about?

adrien2p commented 5 years ago

Hey @bashleigh, in fact the module container store all the modules through the app. You can go through them to find what you need for the custom decorator.

I used it in one of my 3rd party module if you want an example.

https://github.com/adrien2p/nestjs-dialogflow/blob/master/src/module/dialog-flow.provider.ts

I hope that can be used as an example, @kamilmysliwiec could tell me if something is wrong in the usage. I very appreciate the work done on the framework, that’s why i keep learning on it and stay informed about any issues and news throught my emails ^^

BrunnerLivio commented 5 years ago

@adrien2p @bashleigh Thanks a lot for your enthusiasm towards NestJS.

But this topic is regarding the extraction of the express module, so please focus on that subject. That is why I will mark as “off topic”, because the issue is too cluttered anyway.

@bashleigh Nonetheless I highly encourage you to break down your comment and create issues regarding your concerns (How we should handle the “nest community” org / moduleRef best practice) or contact Kamil directly using Gitter if your concern is subjective.

marcus-sa commented 5 years ago

@wbhob unfortunately Kamil doesn't approve of that, already created a PR regarding it and it was declined due to core being entirely rewritten and using Inversify over Nests own DI (yet the API is exactly the same). Imo there's no reason to reinvent the wheel, when Inversify improves developer experience and extensibility. https://github.com/marcus-sa/nest/tree/feat/one

BrunnerLivio commented 5 years ago

6.0.0 has been released with @nestjs/platform-express and @nestjs/platform-fastify which fixes this (2nd oldest open) issue :) 🎉 🎉

lock[bot] commented 4 years ago

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.