Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upThrowing exception when a non-function callback is passed to `elm.ports[port].subscribe` #952
Conversation
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment
Hide comment
process-bot
Mar 27, 2018
Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!
Here is what to expect next, and if anyone wants to comment, keep these things in mind.
process-bot
commented
Mar 27, 2018
|
Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it! Here is what to expect next, and if anyone wants to comment, keep these things in mind. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
emilianobovetti commentedMar 27, 2018
•
edited
Edited 3 times
-
emilianobovetti
edited Apr 1, 2018 (most recent)
-
emilianobovetti
edited Apr 1, 2018
-
emilianobovetti
edited Mar 27, 2018
Currently calling
elm.ports[port].subscribe(undefined)correctly addsundefinedto subscribers list. This will produce the following exception when subscribers get called:I think this behaviour should be changed because that exception message can be confusing, the stacktrace contains only Elm code, but most important the error could be raised before pushing the callback in subscribers list.
Following the same logic as
send, that throws a runtime exception immediately when it receives invalid data, thesubscribewith this PR will raise this error:Why I think
typeof callback !== 'function'is safe:According to ECMAScript language specification the function call expression
currentSubs[i](value)will throw an error if IsCallable is false, which returns true if its argument has the[[Call]]internal method.typeofwill return "function" if an object implements [[Call]].If I'm not wrong, it's impossible that
typeofdoesn't return'function'andcurrentSubs[i](value)doesn't raise a TypeError, so if this is true I don't see any downside in throwing an error immediately!