build: TypeScript 3.5 upgrade#31615
Conversation
|
You can preview a93050d at https://pr31615-a93050d.ngbuilds.io/. |
|
//packages/compiler-cli/test/ngtsc:ngtsc is failing due to compiler host not being passed properly somewhere in between our test setup, tsickle, and typescript. My guess is that the test doesn't properly initialize the environment. @alxhub is this something you could look into please? Feel free to push to this branch. |
|
TypesScript should be TypeScript (no extra s) in the title.😉 |
|
All (the pull request submitter and all commit authors) CLAs are signed, but 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 by leaving a comment that contains only "I consent." in this pull request. Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
You can preview 4febac9 at https://pr31615-4febac9.ngbuilds.io/. |
bd8d99f to
86a3fd6
Compare
|
You can preview 86a3fd6 at https://pr31615-86a3fd6.ngbuilds.io/. |
86a3fd6 to
abfc719
Compare
|
You can preview abfc719 at https://pr31615-abfc719.ngbuilds.io/. |
|
You can preview 951f3ec at https://pr31615-951f3ec.ngbuilds.io/. |
|
You can preview b6985fd at https://pr31615-b6985fd.ngbuilds.io/. |
|
@EatonZ :-D thanks for pointing that out |
|
You can preview 57034a7 at https://pr31615-57034a7.ngbuilds.io/. |
|
You can preview d0ceee3 at https://pr31615-d0ceee3.ngbuilds.io/. |
d0ceee3 to
2a07df6
Compare
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
2a07df6 to
e390cda
Compare
|
You can preview 2a07df6 at https://pr31615-2a07df6.ngbuilds.io/. |
|
You can preview e390cda at https://pr31615-e390cda.ngbuilds.io/. |
82b374b to
e538486
Compare
e538486 to
6fff209
Compare
|
You can preview 82b374b at https://pr31615-82b374b.ngbuilds.io/. |
|
You can preview e538486 at https://pr31615-e538486.ngbuilds.io/. |
IgorMinar
left a comment
There was a problem hiding this comment.
notes on the changes I make to make the review easier.
There was a problem hiding this comment.
this is just a spec and the legacy ngc internal types are off, so any seems like the best way to go
There was a problem hiding this comment.
T otherwise end up defaults to unknown
There was a problem hiding this comment.
this is not a public api so it's ok to change and correct
| */ | ||
| export const MOCK_PLATFORM_LOCATION_CONFIG = new InjectionToken('MOCK_PLATFORM_LOCATION_CONFIG'); | ||
| export const MOCK_PLATFORM_LOCATION_CONFIG = | ||
| new InjectionToken<MockPlatformLocationConfig>('MOCK_PLATFORM_LOCATION_CONFIG'); |
There was a problem hiding this comment.
this makes the type more precise but I don't expect this to break anyone, the alternative is unknown which is likely more breaky.
| '<div>{{"hello" | aPipe}}</div>', | ||
| `Argument of type '"hello"' is not assignable to parameter of type 'number'.`, '0:5'); | ||
| }); | ||
| it('should report an index into a map expression', () => { |
There was a problem hiding this comment.
this error is now being caught by typescript, so the test fails because the diagnostic comes from tsc rather than ngc. that's why I removed the test.
| // i.e. users have to ask for what they need. With that, we can build better analysis tools | ||
| // and could do better codegen in the future. | ||
| export class ElementRef<T = any> { | ||
| export class ElementRef<T extends any = any> { |
There was a problem hiding this comment.
not ideal, but the goal was to make this backwards compatible now that unspecified generics result in unknown and the default value is not being honored.
There was a problem hiding this comment.
Would this not extend Node???
There was a problem hiding this comment.
because that would make it much more strict than what we have today and would break even those people that are not upgrading typescript.
we can update this stuff in v9 to a more precise type, but we can't do it in 8.2
| // i.e. users have to ask for what they need. With that, we can build better analysis tools | ||
| // and could do better codegen in the future. | ||
| export class ElementRef<T = any> { | ||
| export class ElementRef<T extends any = any> { |
There was a problem hiding this comment.
Would this not extend Node???
|
merge-assistance: global approval |
|
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. |
https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#typescript-35
Closes #31059