Why `initialState` has no longer access to `props`?

[paluh]

Hi,

I've checked that with introduction of receiveProps type of initialState was changed. Do you think that after latest changes (and introduction of isFirstMount) we can make initialState a function again?

It would be more consistent with original react API and from my perspective would simplify state type and rendering in case of caching scenarios etc.

What do you think? Am I missing something?

[maddie927]

In short, we were trying to keep it as simple as possible. Having a type of initialState :: props -> state is extremely limiting (no effects) and can really only be used for copying props into state, which probably isn't something you want to do anyway. Having initialState :: props -> Effect state is slightly more useful but now it's somewhat ambiguous whether receiveProps is called on the first mount/render. We found removing it didn't prevent any use cases we could think of, even before isFirstMount existed.

FWIW we'll probably make some changes soon related to #53 and #52. Any feedback we can get on what's weird about the current API will be helpful. I'm playing with some ideas reason-react uses, which itself is a little Elm-inspired. I've already been using "actions/messages" in some of my own components because it seems to make intention a lot more apparent (and helps keep code out of event callbacks inside render functions.. haha).

[paluh]

Hi @spicydonuts,

Thanks for quick reply! You are right that I'm really interested in initialState :: props -> Effect state.

I think that current API is somewhat limiting when you want to do some precomputing based on properties and cache its results in component state.
In such a cases you are not able to fully initialize your object statically and you have to add Maybe to related parts of your state. In render you have to handle these unreachable "incomplete state" cases too (as far as I understand receiveProps is always called before render).
To summarize: in such scenarios your component has state type which can represent values which are only used in initialState and never again.
Maybe I'm missing something? Maybe there is a better way to implement this kind of caching?

P.S. I'm going to familiarize myself with #53 and #52 - thanks for hints! I want to only mention that we are debugging shallowEqual behavior a lot in our app and it seems that we are forced to use react.Component in some places instead of react.PureComponent. Do you think that this topic should be raised in any of the above threads. Do you think that this lib can expose such a low level react API details?

[maddie927]

I think it's fine to expose low level hooks as long as it's easy to ignore them for the simple cases (v2 moved this way a little bit by exposing the instance and alternate setState functions without changing the things that were working much).

Are you collecting this data you want to cache synchronously? If not, you still need one render cycle to run on the "Nothing" states. Also, are you ignoring future prop updates and only caching on the first mount? If so, why? Putting that logic in receiveProps would keep your cache in sync with your props.

I tend to use Map for those caching cases as it avoids race condition bugs:

1. every time receive props runs, lookup key in the cache
2. if missing, initiate fetch
3. fetch always puts its result into its key's slot
4. render always operates on lookup props.key state.cache, which keeps it in sync

You can use a custom ADT too if you want to represent Loading, LoadingLong, Failed Error, etc. We pulled this logic out into a FetchCache { id :: String, fetch :: id -> Aff b, render :: Maybe b -> JSX } (Ideally String would be Eq a, but that requires the typeclass changes I mentioned)

[paluh]

Hi @spicydonuts,

Thanks for response and for your suggestions!

Are you collecting this data you want to cache synchronously?

Yes. In general you are right that more intense precomputing / caching is done in Aff in some of our components too.

Also, are you ignoring future prop updates and only caching on the first mount?

No, we are making these calculation on every props change (like window resize or layout configuration change).

I've written "cache" but I thought more about precomputing some state values based on props in components which are performance critical in our app (their are handling dragging, scrolling etc. etc.).
In such a components we want to perform some not very expensive computations like caching some layout pieces or creating some filters bitmaps (we are performing some simple image processing) upfront.
We also want to minimize render time as much as possible so even handling these Maybe pattern matching is a waste of cycles.
And at the end we have also this aesthetic problem - state type inconsistency with app logic :-)

Of course I understand your argument that this change can affect clarity of current API of this lib so I'm not sure... maybe I should close this ticket? ;-)

[maddie927]

The discussion is good but I'm not sure how to address it with the current API. As you say, the only real use case is performance optimization and that will be difficult to pull off without dropping to JS so you can also implement shouldComponentUpdate haha (given the equality issues referenced earlier).

[maddie927]

I've proposed some significant changes in this PR. Mentioning it here since this issue also discusses the API.