Skip to content
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

Make launchAff_ operate only on Aff Unit #202

Closed
i-am-the-slime opened this issue Jun 4, 2021 · 7 comments · Fixed by #203
Closed

Make launchAff_ operate only on Aff Unit #202

i-am-the-slime opened this issue Jun 4, 2021 · 7 comments · Fixed by #203
Labels
type: breaking change A change that requires a major version bump.

Comments

@i-am-the-slime
Copy link
Contributor

i-am-the-slime commented Jun 4, 2021

Problem

I’m finding launchAff_ to be quite dangerous. When I have an interface that requires an Effect Unit but I need to perform some network calls I usually provide the callback with launchAff_ . Now I ran into a situation where I used launchAff_ on an Aff (Either Error Unit) which is not what I intended. Do you think it would hurt much if launchAff_ only worked on Aff Unit values? That way it would be similar to the warning about implicitly discarding values in a do block.

Describe the solution you'd like

launchAff_ has the signature

launchAff_  Aff Unit  Effect Unit

Additional context
I'm aware that this should cause a major version bump since it's a breaking change. I'm hopeful however, that when this change requires somebody to change their code it will improve its clarity or even catch potential bugs.

@garyb
Copy link
Member

garyb commented Jun 4, 2021

+1 from me! It's the same kind of thing discard is meant to prevent in a monadic context.

@JordanMartinez
Copy link
Contributor

main :: Effect Unit
main = launchAff_ $ void do
  problemNotSolved

@i-am-the-slime
Copy link
Contributor Author

@JordanMartinez I don't understand, can you elaborate?

@garyb
Copy link
Member

garyb commented Jun 5, 2021

I don't really think that's an issue, using unsafeCoerce there would have the same effect 😉, it's the same situation as discard too, void is supposed to let you do that.

@JordanMartinez
Copy link
Contributor

Ah, I misread the opening issue. The issue is that launchAff_ will convert things to Aff Unit when you may not want it to. By changing the type signature, it brings this issue immediately to the developer. The developer will either think "Oh no! I don't want launchAff_ to discard that value! Let me fix that" (as is the case that opened this issue) or "Ah... I don't care about that value. Nothing a void can't fix." So, it's a minor annoyance (and breaking change) for most developers but otherwise makes the resulting code much safer.

Please disregard my former comment.

@JordanMartinez JordanMartinez added the type: breaking change A change that requires a major version bump. label Jun 5, 2021
@wclr
Copy link

wclr commented Jun 29, 2021

launchAff_ is just a shortcut for using void, to me, it's not justified why such things exist at all. Trying to make things simplier easier we get less predictable code and behaviour. So I would consider deprecating and removing such things.

@cryogenian
Copy link
Member

👍 for removing launchAff_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking change A change that requires a major version bump.
Development

Successfully merging a pull request may close this issue.

5 participants