Make functional components work in strict mode #636
Conversation
Setting it causes the react/StrictMode component to be inserted at the top level, wrapping Reagent root.
This is my first take at making the effect in `functional-render` idempotent. The problem is that watches removed transitively by `ratom/dispose!` do not get restored. (They are never snapshotted, to begin with.)
This should recreate the watches relationships thoroughly but fails whenever the component uses React hooks (those must not be run from effects).
|
@matej-ibis-ai, Thanks! The issue and the proposed fix looks interesting. I will need to take some time to understand this myself. On a quick glance, one thing I'm kind of afraid, is that does storing the values on dispose to the snapshot value affect garbage collection, i.e., can the state value be thrown out. But this is just a hunch and I don't really know what will happen. The test utils option is now merged to master, with a small advanced optimization fix, you can rebase this branch if you want to. I will be on vacation for most of August so it will be some time before I get around to working on this. |
|
Enjoy your vacation! I have debugged what I needed to debug in the React Strict mode so I am in no hurry for merging these changes. I agree the storing of watchers in the "snapshot" field of the object may prevent garbage collection from running. Not that I tested it but it seems natural. I should have pointed it out when opening the PR, in fact... If only there was a way to modify the behaviour depending on whether the strict mode is on or off, then we could optimize by not storing the snapshot and completely disposing of the watchers like before... but that would defeat the purpose of the strict mode so it's probably not a great way to deal with this. Perhaps the watchers (i.e. the snapshot that remembers them) could be discarded when some time has passed -- that could resolve the garbage collection concerns but if something goes wrong, debugging could get quite complicated. |
|
I started investigating the issue. "Proper" way for a React Effect to work, would be that the entry call would also initialize the state like regular. We can't do that. The ratom is initialized by render call, and we can't do any tricks to move that render call to happen inside useEffect, because render call might contain other Hook uses, and they can't be called inside the Effect hook. Another idea I considered to avoid potential problems with garbage collection, was to store the snapshot values into the component closure, like this: because component render isn't called again between entry->dispose->entry, the same object would be visible for second effect entry and this value could be used to restore the ratoms. In this case the snapshot value is disposed when render is called again, which I think would ensure the references to old snapshots aren't kept for as long? Maybe. But this approach gets inconvenient because there could be many disposed ratoms, when dispose recursively disposes any watched atoms (without other watchers.) |
|
Replaced by #640 |
2 participants
Building on #635, this PR
functional-renderto make it idempotent, to solve the related issue, Functional components forget their watched atoms in strict mode #634.I have retained commits with some other approaches I tried but which didn't work. The current latest commit contains only the solution which seems to work well.