-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
Update Promise definition #30268
Update Promise definition #30268
Conversation
@rbuckton, |
// CC: @mjbvz |
@microsoft/vscode everyone should take a quick look at this |
I see a few additional |
Most of the changes outside of winjs.base.d.ts and monaco.d.ts were adding explicit type annotations in a few places where TypeScript could not determine the correct type. There are only 3 places where I changed runtime behavior:
All three cases were added to correctly set the resolved value based on the expected return type of the containing method. In all three instances a Alternatively, I can change the return type of the three methods above to something like a |
Also note, I just submitted microsoft/TypeScript#17077, which I hope to discuss at the next TypeScript design meeting. If we decide to accept that change for TypeScript 2.5, we could further improve the |
Wasn't aware that you can have default values for generic parameters. Cool. |
@dbaeumer, it's something we added in 2.3: https://blogs.msdn.microsoft.com/typescript/2017/04/27/announcing-typescript-2-3/#default-type-arguments |
@rbuckton It makes sense to me dropping the values in following
You can also resolve to I would ask @chrmarti to confirm the change (assigning @chrmarti) |
@sandy081 we treat |
@rbuckton Good to know. |
@rbuckton Sorry, this fell off my radar. Can you resolve the merge conflicts so we can merge this in. I can also take the PR if you'd prefer |
Looks like the latest iteration has a few build errors: https://travis-ci.org/Microsoft/vscode/jobs/265409458 |
Yeah, I just noticed that. It is building cleanly locally. I'll look into it. |
I'm trying to make sure I have the correct dependencies in node_modules and have been running into issues. I deleted node_modules and ran
I'm using node v8.1.4, npm 5.3.0, and Visual Studio 2015 |
You might also want to delete stuff in |
As a follow up to #30216 (comment), this updates the definition of Promise in winjs.base.d.ts to use type parameter defaults and addresses errors in several files where either TypeScript could not pick the correct type or where the code using the promise was trying to force the wrong type (e.g. trying to explicitly force a
TPromise<Something>
into aTPromise<void>
).