fix(compiler-cli): readConfiguration existing options should override options in tsconfig

[alan-agius4]

fix(compiler-cli): extend angularCompilerOptions in tsconfig from node
TypeScript supports non rooted extends, we should do the same

https://github.com/microsoft/TypeScript/blob/b346f5764e4d500ebdeff7086e43690ea533a305/src/compiler/commandLineParser.ts#L2603-L2628

Closes: #36715

refactor(bazel): use readConfiguration to read config file
With this change we clean up the parsing of tsconfig files.

fix(compiler-cli): readConfiguration existing options should override options in tsconfig
At the moment, when passing an Angular Compiler option
in the existingOptions it doesn't override the defined in the TSConfig.

[petebacondarwin]

Ugh, I hate how convoluted this whole area of code is.
I appreciate that this is not the fault of this PR.
But I wonder if we ought to take this opportunity to clean it up rather than patching it after the fact, which is basically what we are doing in this PR.

As I understand it there are "TypeScript" configurations and "Angular" configurations.
We have to effectively parse the tsconfig.json twice with two different tools to extract each.
On top of this the caller can pass in an object of programmatically defined existingOptions which should override those that appear in the tsconfig.json. But since this existingOptions (despite the typing) can contain Angular options too, we have to do this overriding twice.

So I think that the config object should only contain Angular options and the parsed.options should only contain TypeScript options.

I believe that the call to ts.parseJsonConfigFileContent() should already override the TypeScript options with those from existingOptions but I think that this will not apply the Angular parts of existingOptions over the top of those in config.

So I think that the right thing to do is to fix the config object so that it has the Angular options overridden by those in existingOptions before passing it to createNgCompilerOptions(), which should not have to care about overriding.

Does that sound right?

[petebacondarwin]

LGTM - just a couple of suggested refactorings.

[alan-agius4]

@petebacondarwin updated.

[josephperrott]

LGTM

Reviewed-For: bazel

[alan-agius4]

I can re-add that method although I prefer not to, but if it’s widely used in G3 I will be easier.

[AndrewKushnir]

@alan-agius4 FYI I've started a new presubmit (+ TGP) after @kyliau submitted an internal refactoring to avoid using createNgCompilerOptions function (thanks Keen!). Will keep this thread updated.

[AndrewKushnir]

@alan-agius4 FYI presubmits were successful for the changes in this PR, adding the "merge" label back. Thank you.

[kyliau]

thanks @AndrewKushnir !

[alan-agius4]

Thanks @AndrewKushnir & @kyliau for your help.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}