fix(compiler-cli): pass config path to ts.parseJsonConfigFileContent #29872
+3
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
You can preview 7ca6bc8 at https://pr29872-7ca6bc8.ngbuilds.io/. |
filipesilva
added
action: review
|
You can preview c65bd7b at https://pr29872-c65bd7b.ngbuilds.io/. |
The config path is an optional argument to `ts.parseJsonConfigFileContent`. When passed, it is added to the returned object as `options.configFilePath`, and `tsc` itself passes it in. The new TS 3.4 [incremental](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html) build functionality relies on this property being present: https://github.com/Microsoft/TypeScript/blob/025d82633915b67003ea38ba40b9239a19721c13/src/compiler/emitter.ts#L56-L57 When using The compiler-cli `readConfiguration` the config path option isn't passed, preventing consumers (like @ngtools/webpack) from obtaining a complete config object. This PR fixes this omission and should allow JIT users of @ngtools/webpack to set the `incremental` option in their tsconfig and have it be used by the TS program. I tested this in JIT and saw a small decrease in build times in a small project. In AOT the incremental option didn't seem to be used at all, due to how `ngc` uses the TS APIs. Related to https://github.com/angular/angular-cli/issues/13941.
|
You can preview d2f0592 at https://pr29872-d2f0592.ngbuilds.io/. |
alxhub
approved these changes
Apr 16, 2019
filipesilva
added
action: merge
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
filipesilva
added
the
merge: caretaker note
wKoza
pushed a commit
to wKoza/angular
that referenced
this pull request
Apr 17, 2019
…ngular#29872) The config path is an optional argument to `ts.parseJsonConfigFileContent`. When passed, it is added to the returned object as `options.configFilePath`, and `tsc` itself passes it in. The new TS 3.4 [incremental](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html) build functionality relies on this property being present: https://github.com/Microsoft/TypeScript/blob/025d82633915b67003ea38ba40b9239a19721c13/src/compiler/emitter.ts#L56-L57 When using The compiler-cli `readConfiguration` the config path option isn't passed, preventing consumers (like @ngtools/webpack) from obtaining a complete config object. This PR fixes this omission and should allow JIT users of @ngtools/webpack to set the `incremental` option in their tsconfig and have it be used by the TS program. I tested this in JIT and saw a small decrease in build times in a small project. In AOT the incremental option didn't seem to be used at all, due to how `ngc` uses the TS APIs. Related to https://github.com/angular/angular-cli/issues/13941. PR Close angular#29872
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?
The config path is an optional argument to
ts.parseJsonConfigFileContent. When passed, it is added to the returned object asoptions.configFilePath, andtscitself passes it in.The new TS 3.4 incremental build functionality relies on this property being present: https://github.com/Microsoft/TypeScript/blob/025d82633915b67003ea38ba40b9239a19721c13/src/compiler/emitter.ts#L56-L57
When using The compiler-cli
readConfigurationthe config path option isn't passed, preventing consumers (like @ngtools/webpack) from obtaining a complete config object.Related to https://github.com/angular/angular-cli/issues/13941.
What is the new behavior?
This PR fixes this omission and should allow JIT users of @ngtools/webpack to set the
incrementaloption in their tsconfig and have it be used by the TS program.I tested this in JIT and saw a small decrease in build times in a small project. In AOT the incremental option didn't seem to be used at all, due to how
ngcuses the TS APIs.Does this PR introduce a breaking change?
Other information