Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Unswap arguments#20212

Merged
amcasey merged 3 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
amcasey:ToEventArgsamcasey/TypeScript:ToEventArgsCopy head branch name to clipboard
Nov 22, 2017
Merged

Unswap arguments#20212
amcasey merged 3 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
amcasey:ToEventArgsamcasey/TypeScript:ToEventArgsCopy head branch name to clipboard

Conversation

@amcasey

@amcasey amcasey commented Nov 22, 2017

Copy link
Copy Markdown
Member

VS is ignoring all events because the event name is currently in the body field. As a result, projects are not refreshed when typings are installed.

@paulvanbrenk

Copy link
Copy Markdown
Contributor

👍

Comment thread src/server/server.ts

private writeToEventSocket(body: any, eventName: string): void {
this.eventSocket.write(formatMessage(toEvent(body, eventName), this.logger, this.byteLength, this.host.newLine), "utf8");
this.eventSocket.write(formatMessage(toEvent(eventName, body), this.logger, this.byteLength, this.host.newLine), "utf8");

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.

type of body should be object on twoEvent, as well as on writeToEventSocket. that should flag invalid uses like these in the future.

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.

not {}, object..

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.

{} is the top-type.. a string is assignable to {} since it has no required properties..

object is the non-primitive object, i.e. something that is not string, number, boolean, or symbol

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both callers have body: T, which seems not to be assignable to object. Do I need to make an additional change to make that work?

@mhegazy

mhegazy commented Nov 22, 2017

Copy link
Copy Markdown
Contributor

for some reason github does not like my comments, and think they are outdated.. so reposting:

{} is the top-type.. a string is assignable to {} since it has no required properties..
object is the non-primitive object, i.e. something that is not string, number, boolean, or symbol

@mhegazy mhegazy left a comment

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.

thanks!

@amcasey amcasey merged commit a0dec26 into microsoft:master Nov 22, 2017
@amcasey amcasey deleted the ToEventArgs branch November 22, 2017 03:01
@amcasey

amcasey commented Nov 22, 2017

Copy link
Copy Markdown
Member Author

I'll port this to 2.6 tomorrow.

@mhegazy

mhegazy commented Nov 22, 2017

Copy link
Copy Markdown
Contributor

I do not think the original change was ported to 2.6

@aozgaa

aozgaa commented Nov 22, 2017

Copy link
Copy Markdown
Contributor

Yes, I didn't port the original change.

@amcasey

amcasey commented Nov 22, 2017

Copy link
Copy Markdown
Member Author

Even better 😄

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.