feat: Add compiler option to read project's tsconfig file#51
feat: Add compiler option to read project's tsconfig file#51
Conversation
AlenDavid
left a comment
There was a problem hiding this comment.
This PR is very important. I'm approving but I left a list of comments, so please answer them if possible. Thank you so much for this work.
| ) { | ||
| this.compilerOptions = { | ||
| // Things we shouldn't allow the app dev changing | ||
| this.nonConfigurableCompilerOptions = { |
There was a problem hiding this comment.
Why not? Is there a reason for this options to be hardcoded?
There was a problem hiding this comment.
Because these options affect the output of the compilation, and that's up to the host system to decide, not the app developer
|
|
||
| async function walkDirectory(directory: string, projectExcludes: string[] = []): Promise<ICompilerFile[]> { | ||
| const dirents = await fs.readdir(directory, { withFileTypes: true }); | ||
| const dirsToIgnore = projectExcludes.concat(['node_modules', '.git']); |
There was a problem hiding this comment.
Can this be included in app.json file? Also, can we add files here too?
There was a problem hiding this comment.
That's what the projectExcludes parameter is for
| const a = JSON.parse(tsconfigFile.toString()) as TSConfig; | ||
| return a; | ||
| } catch { | ||
| logger.warn('Invalid tsconfig.json file - ignoring'); |
There was a problem hiding this comment.
Please, let's not ignore the file if it throws any error.
There was a problem hiding this comment.
We need to ignore the tsconfig file if it is invalid, we can't risk breaking backward compatibility
|
hi @d-gubert, may I ask why u stop working on this ticket? |
|
Possible supersede by #55 |
Adding a new option for the AppsCompiler to read and use
compilerOptionsfrom thetsconfig.jsonfile inside the app's root folder.The default behavior is to not use the file - it is opt-in. This is because, since the compiler never used the settings from that file, some config might cause compilation errors that never occured before.
The CLI will handle the addition of this option
ARCH-1143