feat: use default library prefix #1563
Conversation
Co-authored-by: Kamil Mysliwiec <mail@kamilmysliwiec.com>
|
@kamilmysliwiec Thanks : ) |
src/lib/library/library.factory.ts
Outdated
| function getDefaultLibraryPrefix(defaultLibraryPrefix = '@app') { | ||
| try { | ||
| const nestCliJson = JSON.parse( | ||
| readFileSync('./nest-cli.json', 'utf-8'), |
There was a problem hiding this comment.
at @netjs/cli repo we have this code to read the configuration file: https://github.com/nestjs/nest-cli/blob/d7c855ad3d8493bc5e43ebcb3199aab75232d7a4/lib/configuration/nest-configuration.loader.ts#L17
I'm not sure if we should do the same here...
There was a problem hiding this comment.
I know that. There are a few json files.
But latest version of nestjs generates 'nest-cli.json' file. doesn't it?
This feature that I suggested is new, so I think supporting only 'nest-cli.json' is ok.
But becuase @micalevisk 's suggestion is very good idea and It is not that hard to fix, I don't care.
which is better?
There was a problem hiding this comment.
We should use the NestConfigurationLoader here
There was a problem hiding this comment.
@kamilmysliwiec
Thank you for your advice.
I have investigated whole code in schematics repo, but schematics project only care about these two files, 'nest-cli.json', 'nest.json'.
Only nest-cli project has NestConfigurationLoader. and it's FileSystemReader class read some json files.
Well, I just pushed new code that is checking both nest-cli.json and nest.json to get defaultLibraryPrefix.
If you want something else, give me any directions in more detail. I'm ready to willingly apply that.
There was a problem hiding this comment.
Could you add FileSystemReader class to this package (https://github.dev/nestjs/nest-cli/blob/ad212b1b4c6a7777f232ab6c93f83a5fa0bc5fac/lib/readers/file-system.reader.ts#L5) and use it instead?
There was a problem hiding this comment.
done : )
I have just added FileSystemReader class to this package. test files, too.
But the methods in FileSystemReader return Promise.
If I use these methods that return promise, I have to add async keyword to a lot of function.
So I put new sync methods in FileSystemReader
|
lgtm |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #1562
What is the new behavior?
if defaultLibraryPrefix key exists, use this prefix first.
Does this PR introduce a breaking change?
Other information
I wanna change x-prompt message dynamically, but there is no way to make it dynamically for now.
so I have changed the message, but I think the message is not that good.
let me advise.
It is my first time to contribute to nestjs/schematics, You can give me any feedback, it's all welcome.