Skip to content

Unswap arguments#20212

Merged
amcasey merged 3 commits into
microsoft:masterfrom
amcasey:ToEventArgs
Nov 22, 2017
Merged

Unswap arguments#20212
amcasey merged 3 commits into
microsoft:masterfrom
amcasey:ToEventArgs

Conversation

@amcasey
Copy link
Copy Markdown
Member

@amcasey amcasey commented Nov 22, 2017

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
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 22, 2017

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

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

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
Copy link
Copy Markdown
Member Author

amcasey commented Nov 22, 2017

I'll port this to 2.6 tomorrow.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 22, 2017

I do not think the original change was ported to 2.6

@aozgaa
Copy link
Copy Markdown
Contributor

aozgaa commented Nov 22, 2017

Yes, I didn't port the original change.

@amcasey
Copy link
Copy Markdown
Member Author

amcasey commented Nov 22, 2017

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