Skip to content

Conversation

@zpao
Copy link
Member

@zpao zpao commented Jul 21, 2014

Rebasing #1758 and #1759 on master to apply and test internally. This PR will go away and should not be merged.

Callbacks passed to this setImmediate function are called at the end of the current update cycle, which is guaranteed to be asynchronous but in the same event loop (with the default batching strategy).

This is useful for new-style refs (facebook#1373, facebook#1554) and for fixing facebook#1698.

Test Plan: jest
Depends on facebook#1758.

Fixes facebook#1698.

Previously, controlled components would update too soon when using something like ReactLayeredComponentMixin (i.e., before the layer's updates could propagate from the parent), causing the cursor to jump even when always updating the new model value to match the DOM state. With this change, we defer the update until after all nested updates have had a chance to finish, which prevents the cursor from misbehaving.

Also cleaned up the logic around updating a bit -- the .value and .checked updates in ReactDOMInput weren't being relied on at all so I removed them and opted for a simple forceUpdate instead. I also got rid of _isChanging which hasn't been necessary since the introduction of update batching.

Test Plan: Tested the example in http://jsfiddle.net/Bobris/ZZtXn/2/ and verified that the cursor didn't jump. Changed the code to filter out numbers and verified that the field prevents typing numbers (attempting to do so still causes the cursor to jump to the end). Also verified that controlled and uncontrolled radio buttons, textareas, and select boxes work.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This logic is wrong but is right in #1908/#1758/#1759 now.)

@zpao zpao closed this Jul 22, 2014
@zpao zpao deleted the spicyj-gh-1698 branch October 30, 2014 01:08
josephsavona pushed a commit that referenced this pull request May 15, 2024
--- 

#1899 only propagated eliminated phi nodes to function expressions on the first 
iteration through blocks. However, this was buggy as later iterations could 
introduce rewrites that need to be propagated. 

[playground 
repro](https://0xeac7-forget.vercel.app/#eyJzb3VyY2UiOiJmdW5jdGlvbiBDb21wb25lbnQoKSB7XG4gIGNvbnN0IHggPSA0O1xuXG4gIGNvbnN0IGdldDQgPSAoKSA9PiB7XG4gICAgd2hpbGUgKGJhcigpKSB7XG4gICAgICBpZiAoYmF6KSB7XG4gICAgICAgIGJhcigpO1xuICAgICAgfVxuICAgIH1cbiAgICByZXR1cm4gKCkgPT4geDtcbiAgfTtcblxuICByZXR1cm4gZ2V0NDtcbn0ifQ==). 

I manually synced #1907 to check that this fix works for the VR Store codebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants