Skip to content

Comments

feat: Add compiler option to read project's tsconfig file#51

Open
d-gubert wants to merge 2 commits intomasterfrom
improve/read-app-tsconfig-file
Open

feat: Add compiler option to read project's tsconfig file#51
d-gubert wants to merge 2 commits intomasterfrom
improve/read-app-tsconfig-file

Conversation

@d-gubert
Copy link
Member

@d-gubert d-gubert commented Jul 27, 2023

Adding a new option for the AppsCompiler to read and use compilerOptions from the tsconfig.json file 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

Copy link

@AlenDavid AlenDavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? Is there a reason for this options to be hardcoded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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']);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be included in app.json file? Also, can we add files here too?

Copy link
Member Author

@d-gubert d-gubert Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, let's not ignore the file if it throws any error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to ignore the tsconfig file if it is invalid, we can't risk breaking backward compatibility

@cuonghuunguyen
Copy link
Contributor

hi @d-gubert, may I ask why u stop working on this ticket?

@Dnouv
Copy link
Member

Dnouv commented May 6, 2025

Possible supersede by #55

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

Successfully merging this pull request may close these issues.

4 participants