-
Notifications
You must be signed in to change notification settings - Fork 30.5k
Ember workarounds and fixes for TS 3.4 #33420
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
Conversation
1. Ember-data tests had the wrong return type in a couple of places. TS 3.4 catches this error, and this PR fixes it. 2. Ember-data's Model class became invariant in 3.4 because of its use of the `this` type. This PR uses a `this` parameter with a method-local type parameter to avoid using Model's `this` type. 3. Ember v2's tests were broken by TS 3.4's more complete return-type inference, which caused a variable declaration's type annotation to break inference through contextual typing. This PR converts the type annotation to type parameters on the call, which short-circuits inference entirely. It's also shorter.
|
@mike-north This may not pass CI until tomorrow when Microsoft/TypeScript#29981 is in typescript@next, but maybe you can take a look now to see whether the changes make sense. |
|
@sandersn Thank you for submitting this PR! 🔔 @jedmao @bttf @dwickern @chriskrycho @theroncross @mfeckie @alexlafroscia @mike-north @BryanCrotaz - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
|
@sandersn The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
I just published |
|
I just published |
|
Thanks @sandersn this is a huge help! |
|
Just want to reiterate @mike-north's comment: thank you so much for figuring this out and getting everything straightened out, @sandersn! |
thistype. This PR uses athisparameter with a method-local type parameter to avoid using Model'sthistype.After microsoft/TypeScript#29981 is merged, Ember types should compile cleanly with typescript@next.