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

EventSource.emit docs are misleading for Hooks #48

Open
milesfrain opened this issue Jul 3, 2020 · 4 comments
Open

EventSource.emit docs are misleading for Hooks #48

milesfrain opened this issue Jul 3, 2020 · 4 comments
Labels
document me Improvements or additions to documentation good first issue Good for newcomers

Comments

@milesfrain
Copy link
Contributor

A fix is possible in the main Halogen code, but the issue only affects Hooks users, so I wouldn't consider it a regular Halogen issue.

The help docs for emit are a bit too specific, where a is assumed to be an "action".

emit :: forall m a. Emitter m a -> a -> m Unit

Emits an action via the emitter. For example:

data Action = Notify String

myEventSource = EventSource.affEventSource \emitter -> do
  Aff.delay (Milliseconds 1000.0)
  EventSource.emit emitter (Notify "hello")
  pure mempty

This assumption is fine for regular Halogen, and I can't think of a situation where you'd want to use anything besides a component's Action type, but this is misleading for usage with Hooks because a needs to be HookM m Unit.

I don't know what the best strategy is for improving Halogen docs that require a different interpretation for Hooks. Making the Halogen docs too generic may be more confusing for beginners. Perhaps more Hooks examples would help. There's nothing available yet demonstrating effectEventSource and affEventSource.

For context here's a partial Ace example with effectEventSource. Considering adding something like this to the cookbook.

    content /\ setContent <- useStateFn HK.put "some content"
    document /\ setDocument <- useStateFn HK.put (Nothing :: Maybe Document)
    HK.useLifecycleEffect do
      doc <-
        liftEffect do
          -- Create an editor
          editor <- Ace.edit "editor" Ace.ace
          session <- Editor.getSession editor
          docInner <- Session.getDocument session
          _ <- Document.setValue content docInner
          _ <- Editor.navigateFileStart editor
          pure docInner
      -- Ignoring subscription ID
      _ <-
        HK.subscribe do
          ES.effectEventSource \emitter -> do
            -- Ignoring DocumentEvent
            Document.onChange doc \_ -> do
              str <- Document.getValue doc
              ES.emit emitter $ setContent str
            -- No finalizer
            pure mempty
      setDocument $ Just doc
      pure Nothing
@thomashoneyman
Copy link
Owner

thomashoneyman commented Jul 3, 2020

Thanks for pointing this out, @milesfrain. I think there are two things we can do for now, and maybe we can do even more in the future to make this clear.

First, it's a decent approximation to replace any use of Action in Halogen with HookM m Unit in Hooks. In some cases it's not even an approximation, it's the actual case: for example, the HTML type used in Hooks is the same one as that used in Halogen, except that it uses HookM m Unit as the action type. So if you read in the documentation for emit that it "emits an action via the emitter", then in Hooks that means you would emit a HookM m Unit via the emitter.

Second, I'm happy to add an example to this repository that uses subscriptions and event sources, as there is no existing example. Ideally it would be something smaller than the Ace example (perhaps a timer or something like that) just to avoid needing to keep up to date with a third-party component.

I'm open to other suggestions for how to make this "action" distinction more clear as well.

@JordanMartinez
Copy link
Contributor

Just FYI. TimeHalogen in the cookbook is using the affEventSource, so there's already an example for that.

@thomashoneyman thomashoneyman added document me Improvements or additions to documentation good first issue Good for newcomers labels Aug 15, 2020
@thomashoneyman
Copy link
Owner

I've added labels for this as it's something that could be added to the project documentation (even if just by copy/pasting my description into the docs somewhere).

@CarstenKoenig
Copy link
Contributor

is this issue still relevant now that this functionality got moved into halogen-subscriptions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document me Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants