New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new function: Signal.sendMessage : Message -> Task () () #356

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@rgrempel
Contributor

rgrempel commented Aug 18, 2015

It would sometimes be convenient for third-party libraries to create functions that take a Message for a parameter, rather than an Address and an action. For instance, contrast these two signatures:

sendMessageWhenSomethingHappens : Address a -> a -> Task () ()

sendMessageWhenSomethingHappens : Message -> Task () ()

At the moment, the second signature is legal, but doesn't accomplish anything, because there is nothing which a third-party library can do with a Message. (Since Message is an opaque type and there are no functions which take Message as a parameter).

I'm assuming that Message is an opaque type in part so that it is possible to provide a Message without exposing the Address (or, for that matter, the action) to the function. However, it is exactly that which would sometimes also be useful when it comes to third-party libraries.

So, what I am proposing with this pull request is to include a sendMessage function that would take the Message as a parameter and return a Task which, when executed, will send the Message. I would offer these further thoughts for your consideration:

  • Including the sendMessage function would still leave Message an opaque type -- one still would not be able to extract the Address or the action.
  • Message does not need to be parameterized, which has some advantages if the third-party library needs to put the messages in a List. That is, you could put several messages in a List Message even if the action types were different, which would not be possible for a List (Address a, a).
  • The sendMessage function would simply return a Task, which would still need to be sent to a port to be executed. This is why the function does not rely on Native.Signal.sendMessage, which does some magic to actually perform the Task. (We can't rely on that here, because then Signal.sendMessage would not be a pure function).
  • It would not be possible to put this function in a signal-extra library as a preliminary step, because it cannot be implemented in a third-party library.

In the end, it just seems odd that the Message type can't be used by third-party libraries -- I would submit that a Signal.sendMessage would cure that oddity without creating any problems.

@rgrempel rgrempel referenced this pull request Aug 22, 2015

Closed

Proposal: Mailbox Revamp #7

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Aug 22, 2015

Contributor

I believe that accepting this pull request would allow the following proposal to be experimentally implemented as a third-party library.

elm-lang/elm-plans#7

This might be nice, in that this change is a smaller change to core than that change would be.

Contributor

rgrempel commented Aug 22, 2015

I believe that accepting this pull request would allow the following proposal to be experimentally implemented as a third-party library.

elm-lang/elm-plans#7

This might be nice, in that this change is a smaller change to core than that change would be.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Aug 23, 2015

Member

Adding to #322, seems plausible to me.

Member

evancz commented Aug 23, 2015

Adding to #322, seems plausible to me.

@evancz evancz closed this Aug 23, 2015

@evancz evancz referenced this pull request Aug 23, 2015

Open

API Ideas #322

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Aug 25, 2015

Contributor

Alternatively, if the larger mailbox revamp was accepted, this would be part of it.

Contributor

mgold commented Aug 25, 2015

Alternatively, if the larger mailbox revamp was accepted, this would be part of it.

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Aug 25, 2015

Contributor

I should say that I do support the larger mailbox revamp, which would (as @mgold says) make this redundant.

Contributor

rgrempel commented Aug 25, 2015

I should say that I do support the larger mailbox revamp, which would (as @mgold says) make this redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment