Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

setState suspension #183

Merged
merged 29 commits into from
Feb 27, 2019

Conversation

ZoteTheMighty
Copy link
Contributor

@ZoteTheMighty ZoteTheMighty commented Feb 15, 2019

Closes #180.

Work in progress for setState suspension. So far I've gotten a quick'n'dirty implementation in, and I've culled the similar functionality from SingleEventManager.

Wondering if a slightly better approach would just be to track all lifecycle states as one big enumerated set and then, in setState, granularly react based on it. (e.g. if lifecyclePhase == shouldUpdate or lifecyclePhase == willUpdate or...). Then when we are supposed to error, we can do so with the name known. Not that different from tracking setStateBlockedReason as we did previously.

Also, fixes a broken test from before (not sure how this happened) where createFragment wasn't added to the public api test.

Some basic tests are written, but more need to be written for cases like:

  • Ensure that multiple pending state updates are combined properly
  • Test interactions between suspension and setState aborting (via functional setState) functionality
  • Test interactions with getDerivedStateFromProps

Checklist before submitting:

  • Update error messages for disallowed rendering
  • Clean up or move no-longer-relevant tests for SingleEventManager
  • Change the LifecyclePhase object to be more enum-like
  • Added entry to CHANGELOG.md
  • Added/updated documentation (we should explain setState suspension somewhere in the docs)

@ZoteTheMighty ZoteTheMighty requested review from AmaranthineCodices and LPGhatguy and removed request for AmaranthineCodices February 20, 2019 01:43
@ZoteTheMighty
Copy link
Contributor Author

I'd still like to add more tests, but this is starting to get to a place where I'd like some eyes on it and some feedback.

The challenging bits were with getDerivedStateFromProps and making sure to avoid creating new state tables until I was sure state had actually changed. Maybe there are still better ways to simplify these things.

Also, better method name suggestions are always welcome.

@ZoteTheMighty ZoteTheMighty changed the title WIP: Setstate suspension setState suspension Feb 27, 2019
@ZoteTheMighty ZoteTheMighty merged commit 6e74824 into Roblox:new-reconciler Feb 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant