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

Codesandbox example does not use the AppService #3

Closed
BrunnerLivio opened this issue Dec 27, 2018 · 5 comments
Closed

Codesandbox example does not use the AppService #3

BrunnerLivio opened this issue Dec 27, 2018 · 5 comments

Comments

@BrunnerLivio
Copy link
Member

The Codesandbox.io example on the start page is kinda questionable. The app.controller.ts does inject the AppService, but does not call anything from it, making it unneeded.

@Controller()
export class AppController {
  constructor(private readonly appService: AppService) {}

  @Get()
  render(@Res() res: Response) {
    res.sendFile(`index.html`, { root: __dirname + '/..' });
  }
}

I understand that you probably want to render the index.html so it looks more appealing. But maybe we should consider an example which actually uses a service, so the user better understands the benefits of NestJS? Maybe the app service returns a static array of benefits of NestJS which then get rendered by the index.html?

@kamilmysliwiec
Copy link
Member

I understand that you probably want to render the index.html so it looks more appealing

Exactly.

Maybe the app service returns a static array of benefits of NestJS which then get rendered by the index.html?

In this case, we would have to add some template engine whereas I tried to keep this sample as simple as possible Any ideas what could we do instead? I agree that it definitely doesn't show nest benefits.

@BrunnerLivio
Copy link
Member Author

I tried to keep this sample as simple as possible

That does totally make sense. Unfortunately to me it seems like there is no other good way other than using a template rendering engine.. Maybe adding this to the example would not be too bad of an idea, because as a newcomer you can directly see that NestJS is using express as underlying HTTP client. Seeing that you can easily modify the express instance was also one of the points which made me jump onto NestJS back then :)

@kamilmysliwiec
Copy link
Member

I updated the example. Would you mind to take a look? :)

@BrunnerLivio
Copy link
Member Author

Looking real good, clean, simple and usable.

One teeny tiny thing; seems like an uneeded import slipped in main.ts: import { join } from 'path'; :)

@kamilmysliwiec
Copy link
Member

Fixed!

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

No branches or pull requests

2 participants