Skip to content

bugfix: Type improvements on "port" option in Socket and SocketOption…#680

Merged
darrachequesne merged 1 commit intosocketio:masterfrom
kaangokdemir:type-improvements-on-port-option
Nov 5, 2021
Merged

bugfix: Type improvements on "port" option in Socket and SocketOption…#680
darrachequesne merged 1 commit intosocketio:masterfrom
kaangokdemir:type-improvements-on-port-option

Conversation

@kaangokdemir
Copy link
Contributor

@kaangokdemir kaangokdemir commented Oct 31, 2021

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

port: string;

New behaviour

port: string | number;

@darrachequesne
Copy link
Member

Thanks for this 👍

I'm wondering if it would make sense to remove the string and use only number, like here. WDYT?

@kaangokdemir
Copy link
Contributor Author

kaangokdemir commented Nov 3, 2021

@darrachequesne I'm not sure since the port of browser's location object returns a string. Check that:

https://developer.mozilla.org/en-US/docs/Web/API/Location/port

@darrachequesne darrachequesne merged commit 8f68f77 into socketio:master Nov 5, 2021
@darrachequesne
Copy link
Member

Well, that makes sense. Thanks!

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.

2 participants