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): update to 1.9 nightly. #8003

Closed
wants to merge 2 commits into from

Conversation

alexeagle
Copy link
Contributor

Tested by build.js.cjs, copy to another folder, and manually run compiler to cause 1.8 to type-check the .d.ts files.
It would be better to have a test that runs ts1.8 typechecker in travis, but it's a real pain to change versions of a dependency partway through a build.

@alexeagle alexeagle added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 10, 2016
@alexeagle
Copy link
Contributor Author

note, requires #8002

compiledTree = replace(compiledTree, {
files: ['**/*.d.ts'],
patterns: [
{match: /^(\s*)readonly\s+/mg, replacement: "$1"},
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this supposed to match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

matches
readonly foo: any;
replace with
foo: any;

are you asking for a comment here to explain?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I didn't understand the purpose. A code comment with a link would be helpful so we can confidently remove this someday when 1.8 is no longer supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added
// TODO(alexeagle): this can be removed once we drop support for using Angular 2 with TS 1.8

@jeffbcross
Copy link
Contributor

It would be better to have a test that runs ts1.8 typechecker in travis, but it's a real pain to change versions of a dependency partway through a build.

Would it be significant work (or an otherwise bad idea) to add a simple Travis job that installs TS 1.8 and runs test.unit.cjs, then remove typescript from node_modules so it doesn't get cached?

@alexeagle
Copy link
Contributor Author

Added a line in build_and_test.sh to cover the scenario where we make changes in angular2 that break ts1.8 users.

I don't want to add another matrix entry if possible, since that makes each build consume more travis workers. Since we already have this special typescript_next worker, it now has essentially this logic:

  • install typescript@next
  • run gulp build.js
  • install typescript@1.8.9
  • run gulp test.typings
  • install typescript at the shrinkwrapped version (allowing a passing result to be picked up into travis cache)

the test.typings attempts to compile all our examples against the locally-built angular.

@chuckjaz chuckjaz added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 12, 2016
@chuckjaz chuckjaz assigned alexeagle and unassigned chuckjaz Apr 12, 2016
@alexeagle alexeagle added the action: merge The PR is ready for merge by the caretaker label Apr 12, 2016
@alexeagle alexeagle assigned vsavkin and unassigned alexeagle Apr 12, 2016
To workaround microsoft/TypeScript#7573
we must remove the readonly keyword from generated .d.ts files.
This solution will not scale, but will probably buy enough time to require our users move to a 2.0 beta.
@mary-poppins
Copy link

Merging PR #8003 on behalf of @alxhub to branch presubmit-alxhub-pr-8003.

@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 8, 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 cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants