Add setup function, to wrap componentDidMount

[paf31]

No description provided.

[tippenein]

I'm generally 🆗 on this even though I have a strange feeling both setup and teardown should be included for some vaguely aesthetic reason

[codedmart]

I am partial to agree with @tippenein, but I am also ok with waiting til we actually hit a use case as well. Doing this with the Union trick could be good.

[tippenein]

Actually, If we're going to make these fields optional, initialState should be optional then too

[paf31]

I think it's required - what would the initial state be otherwise?

[tippenein]

empty state. stateless components are required to do initialState: \_ -> {} right?

[codedmart]

@tippenein you can do initialState: \_ -> unit

[tippenein]

sure. Why though? Isn't that the base case s/t you don't need it to create a valid stateless component?

[paf31]

But what if the state type isn't unit?

[paf31]

There's no way with Union to say "default the type of this label to X if it's not there" unfortunately.

[tippenein]

I see! because setup is pure unit we're covered

[maddie927]

I've been thinking about this for a bit.. I think there are a couple different things going on in these PRs so I've grouped them separately below:

Lifecycle hooks

We probably need at least one more lifecycle function in even the most basic stateful component. Here are the lifecycle functions in the reducer branch:

initialState :: props -> state
receiveProps :: props -> state -> state
didMount :: props -> state -> action
didUpdate :: props -> state -> action
reducer :: Reducer eff action state
render :: Render eff props state action
displayName :: String

The reducer/action bit is stylistic, but even if we replace action with Eff I still think we need at least one of these lifecycle hooks (besides render, initialState, and setup/didMount):

receiveProps :: props -> state -> state

I'm pretty sure receiveProps is necessary in a stateful component. The only alternatives are:

1. Completely remount the component when props (i.e. a new component instance) and run initialState again, but then the old state would get dropped, which is obviously bad for both business logic and performance.
2. Take away all component state -- this might work in a world where we strictly enforce that all state gets passed into the root, but I have tried that before and it got really tedious as the app grew.

didUpdate :: props -> state -> Eff

This allows side effects to be performed as a result of state or prop changes. In the reducer model this is how you would fire an action after props change, but with Eff it might make more sense to leave this out and change receiveProps's type to props -> state -> Eff state.. simpler API at the cost of a more complex receiveProps I suppose.

displayName :: String

Not a lifecycle hook, just a useful debugging tool. If the fields are optional I'd like to include this.

Eff vs reducer+actions

I feel the least strongly about this topic, since it's more style than core functionality. I like seeing a condensed list of everything that might update the state. Providing setState inside Eff feels too raw for most business logic.

For example, a click handler with a lambda callback using setState is much more difficult to skim through than onClick Submit. Additionally, most business logic in UIs is async (data fetching, form submission), in which case plain Eff + setState requires callbacks and more nesting. I worry about encouraging that pattern. Good async examples should help though.

This made me wonder if Aff would be a default for business logic. The problem with Aff though is it's unclear what should happen with failure. Requiring async operations to use runAff it a little verbose but it does force error handling back into the component author's hands.

setState

One last topic. The current setState function all by itself doesn't work with async, which is a problem even with the most basic event handlers since they're fired asynchronously relative to the render call they're created in. Rendering itself is also becoming more asynchronous as React evolves. Vanilla React has two solutions to this (sort of*):

1. You can read this.state any time (*this doesn't completely avoid the issue because of things like async rendering)
2. React's setState has a 2nd api -- you can pass it a function from state to state: this.setState(s => s + 1)

We can use this version instead simply by changing setState's type and hopefully never need to write readState 🎉

[maddie927]

ORRRR! (I typed all that and thought of something different while skimming it once more 😆)

We could do this: receiveProps :: props -> Maybe state -> ((state -> state) -> Eff Unit) -> Eff state

With that single "lifecycle" function we can drop initialState, didMount/setup, and didUpdate:

example :: R.ReactComponent ExampleProps
example = R.react { receiveProps, render }
  where
    receiveProps props state _setState = pure { counter: maybe 0 _.counter state }

    render props state setState =
      R.button { onClick: mkEffFn1 \_ -> do
                            setState \s -> s { counter = s.counter + 1 }
               }
               [ R.text label ]

Edit: different lifecycle option -- this still includes the setState change

Edit 2: receiveProps needs setState passed in as well if it's going to be able to do any async work

[maddie927]

Nevermind.. here's a slightly less trivial version:

profile :: R.ReactComponent { userId :: UserId }
profile = R.react { receiveProps, render }
  where
    receiveProps props Nothing setState = do
      ......
    receiveProps props (Just state) setState = when (props.userId /= state.userId) do
      ............in hindsight this is not great

    render _ { Loading } _ = R.text "loading..."
    render _ { Error _ } _ = R.text "oops..."
    render _ { Ready user } _ = R.text user.name

How about initialState + receiveProps:

profile :: R.ReactComponent { userId :: UserId }
profile = R.react { receiveProps, render }
  where
    initialState { userId } = { userId, Loading } -- or Empty?  Do we
                                                  -- need to call receiveProps anyway
                                                  -- for mount side effects?

    receiveProps props state setState = when (props.userId /= state.userId) do
      ............

    render _ { Loading } _ = R.text "loading..."
    render _ { Error _ } _ = R.text "oops..."
    render _ { Ready user } _ = R.text user.name

Maybe initialState as a constant? This is basically the same thing, but it's a little more clear that receiveProps will be called before the first render.

profile :: R.ReactComponent { userId :: UserId }
profile = R.react { receiveProps, render }
  where
    initialState = { userId: Nothing, user: Loading }

    receiveProps props state setState =
      when (Just props.userId /= state.userId) do
        runAff_ (\result ->
          setState \latestState ->
            if latestState.userId /= props.userId
              then latestState
              else case result of
                Left err -> Error err
                Right user -> Ready user
                -- well that was fun.. and probably has a bug,
                -- and doesn't cancel unused requests..
                -- maybe the only way to keep UI logic reasonable
                -- is to bake in FRP 🙂
        ) (fetchUser props.userId)

    render _ { Loading } _ = R.text "loading..."
    render _ { Error _ } _ = R.text "oops..."
    render _ { Ready user } _ = R.text user.name

(Is passing props into render redundant with receiveProps?)

Sorry, I think this got a little off topic.. I should go to bed

[tippenein]

this should be receiveProps?

[paf31]

Yes, thanks!

[tippenein]

It would be nice to get package-lock.json to not appear in diffs but I've not been able to get that to work. (using binary specification in the .gitattributes is what I tried FWIW)