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: typescript 3 support #25275

Closed
wants to merge 3 commits into from
Closed

feat: typescript 3 support #25275

wants to merge 3 commits into from

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Aug 3, 2018

This adds support for TypeScript 3.0.

Notes:

  • for the time being tsickle doesn't support TypeScript 3.0
  • 3 unit tests from program_spec.ts have been xit. Anyone willing to help. Please feel free.

Closes: #25200

//cc @mhevery

@alan-agius4 alan-agius4 closed this Aug 3, 2018
@alan-agius4 alan-agius4 reopened this Aug 4, 2018
@alan-agius4 alan-agius4 changed the title feat: typescript 3 support feat: typescript 3 support (WIP) Aug 4, 2018
@@ -0,0 +1,31 @@
{
"name": "angular-integration",
"description": "Assert that users with TypeScript 2.9 can type-check an Angular application",
Copy link
Contributor

Choose a reason for hiding this comment

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

TypeScript 3.0?

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, nice catch

@alan-agius4 alan-agius4 changed the title feat: typescript 3 support (WIP) feat: typescript 3 support Aug 5, 2018
@IgorMinar IgorMinar self-assigned this Aug 8, 2018
@IgorMinar IgorMinar self-requested a review August 8, 2018 17:21
@IgorMinar IgorMinar added the target: major This PR is targeted for the next major release label Aug 8, 2018
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@googlebot googlebot added cla: no and removed cla: yes labels Aug 9, 2018
@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Aug 10, 2018

I confirm, I signed it.

Thanks Igor

@IgorMinar
Copy link
Contributor

IgorMinar commented Aug 10, 2018 via email

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@JoostK
Copy link
Member

JoostK commented Aug 10, 2018

@alan-agius4 they still seem to fail in Travis

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@IgorMinar
Copy link
Contributor

@alan-agius4 still looking into it.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@matsko
Copy link
Contributor

matsko commented Aug 27, 2018

@alan-agius4 can you please rebase and push again. Master on CircleCI should be ok now.

@alan-agius4
Copy link
Contributor Author

@matsko I m trying too, but I am getting Already up to date.

@IgorMinar IgorMinar added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Aug 27, 2018
@angular angular deleted a comment from googlebot Aug 27, 2018
@IgorMinar
Copy link
Contributor

caretaker note: presubmit results are green I just can't push the status right now.

@matsko
Copy link
Contributor

matsko commented Aug 27, 2018

Sorry @alan-agius4 it still needs one last rebase. It looks like the auto-squashing of the fixup! commits is failing. Can you please interactively rebase and do the squashing yourself in the interactive mode? This way we can merge this without the git autosquash bugging out.

Just do git rebase HEAD~10 and combine the fixup commits into the main commits.

Thank you.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@alan-agius4
Copy link
Contributor Author

@matsko, rebase with fixups done.

Need to a googler to verify the CLA.

alan-agius4 and others added 3 commits August 27, 2018 16:20
In tsc 3.0 the check that enables program structure reuse in tryReuseStructureFromOldProgram has changed
and now uses identity comparison on arrays within CompilerOptions. Since we recreate the options
on each incremental compilation, we now fail this check.

After this change the default set of options is reused in between incremental compilations, but we still
allow options to be overriden if needed.
@mary-poppins
Copy link

You can preview 007865c at https://pr25275-007865c.ngbuilds.io/.

@IgorMinar
Copy link
Contributor

I rebased one more time since there were more conflicts due to upstream merges.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@mary-poppins
Copy link

You can preview 463194a at https://pr25275-463194a.ngbuilds.io/.

@matsko matsko closed this in 5653fad Aug 28, 2018
matsko pushed a commit that referenced this pull request Aug 28, 2018
matsko pushed a commit that referenced this pull request Aug 28, 2018
…on (#25275)

In tsc 3.0 the check that enables program structure reuse in tryReuseStructureFromOldProgram has changed
and now uses identity comparison on arrays within CompilerOptions. Since we recreate the options
on each incremental compilation, we now fail this check.

After this change the default set of options is reused in between incremental compilations, but we still
allow options to be overriden if needed.

PR Close #25275
@alan-agius4 alan-agius4 deleted the feature_ts-3 branch September 19, 2018 09:08
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
…on (angular#25275)

In tsc 3.0 the check that enables program structure reuse in tryReuseStructureFromOldProgram has changed
and now uses identity comparison on arrays within CompilerOptions. Since we recreate the options
on each incremental compilation, we now fail this check.

After this change the default set of options is reused in between incremental compilations, but we still
allow options to be overriden if needed.

PR Close angular#25275
@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 cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: Add support for TypeScript 3
8 participants