Remove safeRenderCompletion and setImmediate hacks

[sbusch]

This is a proposal for V2 project, where's the chance to get rid of setImmediate hacks regarding setState:

• replace calls to custom setState wrapper (which uses setImmediate hack) with proper React setState calls and with callback where appropriate, and eliminate setState wrapper.
• remove safeRenderCompletion
• remove any other setImmediate 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!

[epicfaace]

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?

https://github.com/mozilla-services/react-jsonschema-form/blob/f9d4c63cac24146e522528d33eb04f465464e052/src/utils.js#L786

[sbusch]

No, I don't know why.

Some forensics:

• the setState wrapper was introduced in 6159cb4, but this mostly refactored existing setImmediate hacks into this wrapper to enable central circumvention of setImmediate via safeRenderCompletion.
• before that, many setImmediate calls (all of them?) were introduced in PR Improved async rendering strategy #153 "Improved async rendering strategy" by @n1k0

If I understand everything correctly:

1. status quo: setState was used properly, with onChange within callback (so no problems with correctness, race conditions)
2. it became clear that react-jsonschema-form is too slow: many components are stateless and there is no memoization. See Slow performance  #147. The examples linked there demonstrating the performance degradation are still working...
3. first try to fix it was PR Attempt at providing memoization for stateless components. #152, using memoization (interestingly, the PR also introduces some setImmediate calls)
4. it seems that during implementation of PR Attempt at providing memoization for stateless components. #152, using setImmediate showed so much promise that PR Attempt at providing memoization for stateless components. #152 was aborted in favor of PR Improved async rendering strategy #153

Regarding 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.

[hoppula]

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.

[glasserc]

Thanks for digging into this. I never really understood the purpose of those hacks myself. I'd love to see them gone.

[epicfaace]

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