Skip to content

add stub dispatchEvent method to WebSocket (#1583)#1584

Merged
lpinca merged 1 commit intowebsockets:masterfrom
brainkim:dispatchEvent
Jun 12, 2019
Merged

add stub dispatchEvent method to WebSocket (#1583)#1584
lpinca merged 1 commit intowebsockets:masterfrom
brainkim:dispatchEvent

Conversation

@brainkim
Copy link
Contributor

@brainkim brainkim commented Jun 12, 2019

  • Update @types/ws to include the dispatchEvent method

@3rd-Eden
Copy link
Member

You probably want to include a test to ensure that they are not removed in future refactors

@brainkim
Copy link
Contributor Author

Added test and updated error message

@brainkim
Copy link
Contributor Author

tests are hanging one sec

@lpinca lpinca merged commit cec1e1c into websockets:master Jun 12, 2019
@lpinca
Copy link
Member

lpinca commented Jun 12, 2019

Thank you.

@brainkim
Copy link
Contributor Author

Thanks for the quick turnaround! May I leave the related issue open until I get a @types/ws definitions updated?

@brainkim
Copy link
Contributor Author

Never mind. I’m looking into the typings now and there are way more incompatibilities between the ws WebSocket type and the typescript lib WebSocket type 😓 including but not limited to the Event object lacking properties like Bubble, etc. Probably just gonna continue using type assertions at the end of the day.

@lpinca
Copy link
Member

lpinca commented Jun 12, 2019

Hmm ok then I think it's better to revert this as well.

@lpinca
Copy link
Member

lpinca commented Jun 12, 2019

15 min passed so I guess it's ok to force push. On my way.

@brainkim
Copy link
Contributor Author

Yeah if supporting the typescript websocket definition is desired, then I think supporting the Event/EventTarget interface more fully is required and I’m not sure if that’s what this project wants. Sorry for this.

@lpinca
Copy link
Member

lpinca commented Jun 12, 2019

No problem.

@brainkim brainkim deleted the dispatchEvent branch June 12, 2019 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants