X Tutup
Skip to content

feat(typescript): update to 1.9 nightly.#8003

Closed
alexeagle wants to merge 2 commits intoangular:masterfrom
alexeagle:rm_readonly
Closed

feat(typescript): update to 1.9 nightly.#8003
alexeagle wants to merge 2 commits intoangular:masterfrom
alexeagle:rm_readonly

Conversation

@alexeagle
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

note, requires #8002

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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

@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

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.

7 participants

X Tutup