-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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
chore(typings): re-enable type-checking on the browser trees #4508
Conversation
3d51048
to
8ecd891
Compare
Does your change stop running the upgrade tests? If it is just deployment think it is fine, but if it disables tests, then we need to look for a quick solution |
"commit": "746b9a892629060bc853e792afff536e0ec4655e" | ||
"commit": "4b36b94d5910aa8a4d20bdcd5bd1f9ae6ad18d3c" | ||
}, | ||
"jquery/jquery.d.ts": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because angularjs/angular.d.ts has a relative reference path pointing here.
In fact, this has not correctly type-checked ever since Misko wrote the upgrade package. We just weren't running the typechecker on the browser tree. It wasn't type-checked by node_tree either because of include: ['angular2/**', 'benchpress/**', '**/e2e_test/**'],
. So @mhevery has been getting a free ride!
Tests can't run if the typescript files are never transpiled. Let's figure out a quick fix tomorrow - maybe another |
f21bd66
to
b2fab70
Compare
b2fab70
to
a83dab3
Compare
We had the typechecker disabled by accident, and many problems snuck in Fixes angular#4507
a83dab3
to
1a303da
Compare
Updated to fix the upgrade package - removed the dependency on the angular typings and jquery, instead we hand-wrote a subset of angular.d.ts just for the upgrade package. PTAL |
Merging PR #4508 on behalf of @alexeagle to branch presubmit-alexeagle-pr-4508. |
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. |
this was accidentally disabled in 5b5d31f
And we have accumulated some typings errors in the meantime.
One thing we did is introduce dependencies with typing conflicts.
See #4507
My solution here is to remove the upgrade module from this compilation unit. + @mhevery