This is a proposal for V2 project, where's the chance to get rid of setImmediate hacks regarding setState:
setState wrapper (which uses setImmediate hack) with proper React setState calls and with callback where appropriate, and eliminate setState wrapper.safeRenderCompletionsetImmediate hacks (at least outside of testing code)The code to be removed makes assumptions about React behaviour and JS engine performance which don't work as they changed (and will change) over time.
See discussion at #891 (older: #562)
Related currently open issues: #1164, #1023 (maybe), #544, #513 (maybe), #446 I'm sure there are many other (potentially obscure) errors, mostly race-conditions, coming from this hack. See for example my PR #1068.
There's the possibility that bugs are raised or performance is degraded by that change, but these problems should always be fixable without a false setImmediate "fix".
P.S: nice to see that the project gains traction again, thanks for all working on it. We're happy to contribute!
Thanks for making this issue. Do you know why exactly the custom setState wrapper with setImmediate was added in the first place? Will removing this necessarily be a backwards incompatible change that would require a v2?
No, I don't know why.
Some forensics:
setState wrapper was introduced in 6159cb4834a082b2af2154e6f978b9ad57e96d51, but this mostly refactored existing setImmediate hacks into this wrapper to enable central circumvention of setImmediate via safeRenderCompletion.setImmediate calls (all of them?) were introduced in PR #153 "Improved async rendering strategy" by @n1k0If I understand everything correctly:
setState was used properly, with onChange within callback (so no problems with correctness, race conditions)react-jsonschema-form is too slow: many components are stateless and there is no memoization. See #147. The examples linked there demonstrating the performance degradation are still working...setImmediate calls)setImmediate showed so much promise that PR #152 was aborted in favor of PR #153Regarding backwards compatibility:
In theory, after eliminating setState wrapper and setImmediate and after fixing the performance problems, everything should still just work™. But most definitely, some behavior will change slightly, people are relying on timings, and so on. So I would consider this a breaking change.
This is even more true if we take new React hooks into consideration, which could be a solution for the performance problem, since this requires upgrading the peer dependency on React to 16.8.0.
After just spending three hours debugging why some keystrokes were skipped when typing rapidly in our complex form and then discovering the need for safeRenderCompletion={true}, big 👍 for removing these hacks.
Thanks for digging into this. I never really understood the purpose of those hacks myself. I'd love to see them gone.
This is nearly fixed by #1454, but we still have a few setImmediate hacks in the playground and tests that need to be removed: https://github.com/rjsf-team/react-jsonschema-form/search?q=setImmediate&unscoped_q=setImmediate
Most helpful comment
After just spending three hours debugging why some keystrokes were skipped when typing rapidly in our complex form and then discovering the need for
safeRenderCompletion={true}, big 👍 for removing these hacks.