search

GJordao / simple-auth

An authentication service that aims to be simple and customisable
MIT License
3 stars 3 forks source link

replace Env service with nestjs ConfigService #14

Open fmagno opened 3 years ago

fmagno commented 3 years ago

WIP: I was trying to use simple-auth and noticed that an environment variable was missing in the README file example. I also noticed the current environment service could be made a little bit more declarative and take advantage of the validation mechanism of the nestjs/ConfigModule.

It still requires testing! :)

GJordao commented 3 years ago

I can't seem to be able to run this locally, its giving an error on all the non string env variables. I'm assuming that it probably doesn't convert them from string?

I didn't know about Nest's config service, it looks great but does it give you auto complete for the variables? The idea of the service was mostly because of that 😅

GJordao commented 3 years ago

screenshot for reference

image

fmagno commented 3 years ago

Gotcha! That issue should be fixed, now.

If you think env variables auto-complete is really a must then it is possible to create a thin wrapper service around ConfigService with a getter function for each variable - take a look here.

Issue #15 should be covered as well.

fmagno commented 3 years ago

WIP:

Converted the AppModule to a Dynamic module to allow passing environment variables programmatically. Implemented a few unit tests to test the validation function. Also added a few e2e tests to test the registration/login process. I will be adding some more soon.

MicroAnibal commented 3 years ago

Welcome @fmagno, Thanks for contributing :+1:

Gotcha! That issue should be fixed, now.

Confirmed, is fixed, solution is working locally.

If you think env variables auto-complete is really a must then it is possible to create a thin wrapper service around ConfigService with a getter function for each variable - take a look here.

Agree :100: with @GJordao, auto-complete is a must have to speed things up and save some brain cycles
Also agree that the thin wrapper is perfect for this case.

fmagno commented 3 years ago

Hey @MicroAnibal! Cheers! :) Autocomplete should now work with the service ConfigServiceApi.