Skip to content

Commit

Permalink
Use latest instance when restoring controlled state (facebook#8443)
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiebits authored and acusti committed Mar 15, 2017
1 parent ff45722 commit 46f77b7
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 7 deletions.
3 changes: 0 additions & 3 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
* should throw on full document render w/ no markup
* supports findDOMNode on full-page components

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should control a value in reentrant events

src/renderers/shared/__tests__/ReactPerf-test.js
* should count no-op update as waste
* should count no-op update in child as waste
Expand Down
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should properly control a value even if no event listener exists
* should control a value in reentrant events
* should control values in reentrant events with different targets
* should display `defaultValue` of number 0
* only assigns defaultValue if it changes
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function createAndAccumulateChangeEvent(inst, nativeEvent, target) {
);
event.type = 'change';
// Flag this event loop as needing state restore.
ReactControlledComponent.enqueueStateRestore(inst);
ReactControlledComponent.enqueueStateRestore(target);
EventPropagators.accumulateTwoPhaseDispatches(event);
return event;
}
Expand Down
12 changes: 10 additions & 2 deletions src/renderers/shared/shared/event/ReactControlledComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

'use strict';

var EventPluginUtils = require('EventPluginUtils');

var invariant = require('invariant');

// Use to restore controlled state after a change event has fired.
Expand All @@ -28,15 +30,21 @@ var ReactControlledComponentInjection = {
var restoreTarget = null;
var restoreQueue = null;

function restoreStateOfTarget(internalInstance) {
function restoreStateOfTarget(target) {
// We perform this translation at the end of the event loop so that we
// always receive the correct fiber here
var internalInstance = EventPluginUtils.getInstanceFromNode(target);
if (!internalInstance) {
// Unmounted
return;
}
if (typeof internalInstance.tag === 'number') {
invariant(
fiberHostComponent &&
typeof fiberHostComponent.restoreControlledState === 'function',
'Fiber needs to be injected to handle a fiber target for controlled ' +
'events.'
);
// TODO: Ensure that this instance is the current one. Props needs to be correct.
fiberHostComponent.restoreControlledState(
internalInstance.stateNode,
internalInstance.type,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ReactTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ function makeSimulator(eventType) {
ReactGenericBatching.batchedUpdates(function() {
// Normally extractEvent enqueues a state restore, but we'll just always
// do that since we we're by-passing it here.
ReactControlledComponent.enqueueStateRestore(targetInst);
ReactControlledComponent.enqueueStateRestore(node);

EventPluginHub.enqueueEvents(event);
EventPluginHub.processEventQueue(true);
Expand Down

0 comments on commit 46f77b7

Please sign in to comment.