Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(compiler-cli): add support to extend angularCompilerOptions #22717

Closed
wants to merge 1 commit into from
Closed

feat(compiler-cli): add support to extend angularCompilerOptions #22717

wants to merge 1 commit into from

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Mar 12, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

TypeScript only supports merging and extending of compilerOptions.

Issue Number: #22684

What is the new behavior?

This is an implementation to support extending and inheriting of angularCompilerOptions from multiple files.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@kara kara added the area: core Issues related to the framework runtime label Mar 12, 2018
@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 15, 2018

//cc @chuckjaz & @mhevery

@alan-agius4
Copy link
Contributor Author

Bump @vicb and @alxhub

basePath = support.basePath;
});

function writeSomeRoutes() {
Copy link
Member

Choose a reason for hiding this comment

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

writeSomeConfigs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes better, updated.

@alan-agius4
Copy link
Contributor Author

Travis error looks to be a flake.

@mhevery mhevery added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Aug 9, 2018
Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

Can we please add a documentation for this change? we shouldn't merge new features like this without docs. thanks!

the current compiler options are documented here: aio/content/guide/aot-compiler.md

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Aug 14, 2018

@IgorMinar, I have added the requested docs. Can you please review them?

Please feel free to suggest or do any changes.

Ps: I have also rebased and thanks for your review.

@alan-agius4
Copy link
Contributor Author

The Travis failure is a flake.

@benlesh benlesh removed the action: merge The PR is ready for merge by the caretaker label Aug 14, 2018
@benlesh
Copy link
Contributor

benlesh commented Aug 14, 2018

@alan-agius4 or @IgorMinar ... flag this as PR action: merge when you're done with your review process, please. I'll keep an eye out.

@benlesh
Copy link
Contributor

benlesh commented Aug 14, 2018

For googlers: presubmit (attn: @IgorMinar)

@alan-agius4
Copy link
Contributor Author

@IgorMinar, do you mind re-checking this please? Thanks.

@@ -128,7 +128,37 @@ export function readConfiguration(
try {
const {projectFile, basePath} = calcProjectFileAndBasePath(project);

let {config, error} = ts.readConfigFile(projectFile, ts.sys.readFile);
const readExtendedConfigFile =
(configFile: string, existingConfig?: any): {config?: any; error?: ts.Diagnostic;} => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we prefer to comma-separate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

packages/compiler-cli/test/BUILD.bazel Show resolved Hide resolved
packages/compiler-cli/src/perform_compile.ts Show resolved Hide resolved
@jenniferfell
Copy link
Contributor

@alan-agius4 Thanks. We've been working on a related doc PR that is also about ready to merge. It contains some improvements to the order of sections within the AOT doc: #22353

I think things will merge more seamlessly if we land 22353 first. I've ask the author to accelerate and she should be done with final changes by EOD today.

Meanwhile, I'll take a look at your new section with an eye toward where it will fit best in the new structure and do a quick copy edit.

Will that work for you?

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Sep 7, 2018 via email

@mary-poppins
Copy link

You can preview ab8f729 at https://pr22717-ab8f729.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 70a5519 at https://pr22717-70a5519.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 3f594fa at https://pr22717-3f594fa.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 4f79913 at https://pr22717-4f79913.ngbuilds.io/.

`TypeScript` only supports merging and extending of `compilerOptions`. This is an implementation to support extending and inheriting of `angularCompilerOptions` from multiple files.

Closes: #22684
@mary-poppins
Copy link

You can preview 6c5ce1f at https://pr22717-6c5ce1f.ngbuilds.io/.

@alan-agius4
Copy link
Contributor Author

@jenniferfell CI is all green here as well

Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

thanks for updating the pr

@IgorMinar
Copy link
Contributor

IgorMinar commented Sep 19, 2018

@IgorMinar IgorMinar added the action: merge The PR is ready for merge by the caretaker label Sep 19, 2018
@kara kara closed this in d7e5bbf Sep 19, 2018
@alan-agius4 alan-agius4 deleted the feature/angular-compiler-opts-extends branch September 20, 2018 08:55
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
…ngular#22717)

`TypeScript` only supports merging and extending of `compilerOptions`. This is an implementation to support extending and inheriting of `angularCompilerOptions` from multiple files.

Closes: angular#22684

PR Close angular#22717
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview area: core Issues related to the framework runtime cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants