From cd0f963fa366ac6eaf154fd2b3e616a04feeef33 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 14:40:51 +0000 Subject: [PATCH 1/7] Fix up Flow annotations to be a bit more explicit where the unsoundness is --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 37eb47a100..186d2f0b5f 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -159,7 +159,7 @@ var mediaEvents = { topWaiting: 'waiting', }; -function trapClickOnNonInteractiveElement(node : any) { +function trapClickOnNonInteractiveElement(node : HTMLElement) { // Mobile Safari does not fire properly bubble click events on // non-interactive elements, which means delegated click listeners do not // fire. The workaround for this bug involves attaching an empty click @@ -496,7 +496,7 @@ var ReactDOMFiberComponent = { var div = ownerDocument.createElement('div'); div.innerHTML = ''; // This is guaranteed to yield a script element. - var firstChild = ((div.firstChild : any) : Element); + var firstChild = ((div.firstChild : any) : HTMLScriptElement); domElement = div.removeChild(firstChild); } else if (props.is) { domElement = ownerDocument.createElement(tag, props.is); @@ -622,7 +622,8 @@ var ReactDOMFiberComponent = { break; default: if (typeof props.onClick === 'function') { - trapClickOnNonInteractiveElement(domElement); + // TODO: This cast may not be sound for SVG, MathML or custom elements. + trapClickOnNonInteractiveElement(((domElement : any) : HTMLElement)); } break; } @@ -655,7 +656,8 @@ var ReactDOMFiberComponent = { default: if (typeof lastProps.onClick !== 'function' && typeof nextProps.onClick === 'function') { - trapClickOnNonInteractiveElement(domElement); + // TODO: This cast may not be sound for SVG, MathML or custom elements. + trapClickOnNonInteractiveElement(((domElement : any) : HTMLElement)); } break; } From a2ca759dfd4ff4d82124b30b1015d206260b6925 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 15:09:18 +0000 Subject: [PATCH 2/7] Handle controlled components in Fiber We need to adjust inputValueTracking a bit to handle the fact that Fiber attaches wrapper state on nodes. --- scripts/fiber/tests-failing.txt | 32 +----- scripts/fiber/tests-passing.txt | 28 ++++- src/renderers/dom/fiber/ReactDOMFiber.js | 4 + .../dom/fiber/ReactDOMFiberComponent.js | 6 +- .../dom/shared/inputValueTracking.js | 104 ++++++++++-------- .../shared/event/ReactControlledComponent.js | 8 +- 6 files changed, 101 insertions(+), 81 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 9f175a9402..5508db8337 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -14,15 +14,6 @@ src/addons/__tests__/renderSubtreeIntoContainer-test.js src/isomorphic/classic/__tests__/ReactContextValidator-test.js * should pass previous context to lifecycles -src/isomorphic/classic/class/__tests__/ReactBind-test.js -* Holds reference to instance -* works with mixins - -src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js -* should work with manual binding -* works with mixins that have not opted out of autobinding -* works with mixins that have opted out of autobinding - src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js * includes the owner name when passing null, undefined, boolean, or number @@ -47,11 +38,7 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js * should bubble simply * should continue bubbling if an error is thrown * should set currentTarget -* should support stopPropagation() -* should stop after first dispatch if stopPropagation * should not stopPropagation if false is returned -* should invoke handlers that were removed while bubbling -* should not invoke newly inserted handlers while bubbling * should have mouse enter simulated by test utils * should infer onTouchTap from a touchStart/End * should infer onTouchTap from when dragging below threshold @@ -63,8 +50,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn when mutating style * should empty element when removing innerHTML * should transition from innerHTML to children in nested el -* should track input values -* should track textarea values * should warn for children on void elements * should report component containing invalid styles * should clean up input value tracking @@ -90,11 +75,10 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js * should not fire duplicate events for a React DOM tree src/renderers/dom/shared/__tests__/inputValueTracking-test.js -* should return tracker from node +* should stop tracking src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js * should fire change for checkbox input -* should catch setting the value programmatically * should not fire change when setting the value programmatically * should not fire change when setting checked programmatically * should only fire change for checked radio button once @@ -134,16 +118,10 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js * should trigger load events 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 update `defaultValue` for uncontrolled input * should update `defaultValue` for uncontrolled date/time input -* should properly control a value of number `0` * should have the correct target value -* should control radio buttons -* should control radio buttons if the tree updates during render -* should have a this value of undefined if bind is not used * should update defaultValue to empty string * sets type, step, min, max before value always * resets value of date/time input to fix bugs in iOS Safari @@ -165,8 +143,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should remember value when switching to uncontrolled * should remember updated value when switching to uncontrolled * should not control defaultValue if readding options -* should refresh state on change -* should be able to safely remove select onChange * should select grandchild options nested inside an optgroup src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js @@ -177,7 +153,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should allow setting `value` to `false` * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations -* should properly control a value of number `0` * should keep value when switching to uncontrolled element if changed src/renderers/dom/stack/client/__tests__/ReactDOM-test.js @@ -324,7 +299,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should not thrash a server rendered layout with client side one -* should react to state changes from callbacks * should warn about `forceUpdate` on unmounted components * should warn about `setState` on unmounted components * should warn about `setState` in render @@ -365,7 +339,3 @@ src/renderers/shared/stack/reconciler/__tests__/refs-test.js src/test/__tests__/ReactTestUtils-test.js * should support injected wrapper components as DOM components -* should change the value of an input field -* should change the value of an input field in a component -* should not warn when simulating events with extra properties -* should set the type of the event diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 92c5cedbe8..369ffa6dcc 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -138,11 +138,16 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js * should pass next context to lifecycles src/isomorphic/classic/class/__tests__/ReactBind-test.js +* Holds reference to instance +* works with mixins * warns if you try to bind to this * does not warn if you pass an auto-bound method to setState src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js +* should work with manual binding * should not hold reference to instance +* works with mixins that have not opted out of autobinding +* works with mixins that have opted out of autobinding * does not warn if you try to bind to this * does not warn if you pass an manually bound method to setState @@ -522,6 +527,10 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should support custom attributes src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js +* should support stopPropagation() +* should stop after first dispatch if stopPropagation +* should invoke handlers that were removed while bubbling +* should not invoke newly inserted handlers while bubbling * should listen to events only once * should work with event plugins without dependencies * should work with event plugins with dependencies @@ -571,6 +580,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn about contentEditable and children * should respect suppressContentEditableWarning * should validate against invalid styles +* should track input values +* should track textarea values * should support custom elements which extend native elements * should warn against children for void elements * should warn against dangerouslySetInnerHTML for void elements @@ -626,7 +637,7 @@ src/renderers/dom/shared/__tests__/inputValueTracking-test.js * should coerce value to a string * should update value if it changed and return result * should track value and return true when updating untracked instance -* should stop tracking +* should return tracker from node src/renderers/dom/shared/__tests__/quoteAttributeValueForBrowser-test.js * should escape boolean to string @@ -644,6 +655,7 @@ src/renderers/dom/shared/eventPlugins/__tests__/BeforeInputEventPlugin-test.js * extract onBeforeInput from fallback objects src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js +* should catch setting the value programmatically * should unmount src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js @@ -727,6 +739,8 @@ src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js * sets innerHTML on it src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +* should properly control a value even if no event listener exists +* should control values in reentrant events with different targets * should display `defaultValue` of number 0 * only assigns defaultValue if it changes * should display "true" for `defaultValue` of `true` @@ -744,8 +758,12 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should allow setting `value` to `false` * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations +* should properly control a value of number `0` * should not set a value for submit buttons unnecessarily +* should control radio buttons +* should control radio buttons if the tree updates during render * should warn with value and no onChange handler and readOnly specified +* should have a this value of undefined if bind is not used * should warn with checked and no onChange handler with readOnly specified * should warn if checked and defaultChecked props are specified * should warn if value and defaultValue props are specified @@ -772,7 +790,9 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should support server-side rendering * should support server-side rendering with defaultValue * should support server-side rendering with multiple +* should refresh state on change * should warn if value and defaultValue props are specified +* should be able to safely remove select onChange src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should allow setting `defaultValue` @@ -785,6 +805,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should render value for SSR * should take updates to `defaultValue` for uncontrolled textarea * should take updates to children in lieu of `defaultValue` for uncontrolled textarea +* should properly control a value of number `0` * should treat children like `defaultValue` * should keep value when switching to uncontrolled element if not changed * should allow numbers as children @@ -1130,6 +1151,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should support module pattern components * should support rendering to different child types over time +* should react to state changes from callbacks * should rewire refs when rendering to different child types * should not cache old DOM nodes when switching constructors * should auto bind methods and values correctly @@ -1406,8 +1428,12 @@ src/test/__tests__/ReactTestUtils-test.js * can scryRenderedDOMComponentsWithClass with className contains \n * can scryRenderedDOMComponentsWithClass with multiple classes * traverses children in the correct order +* should change the value of an input field +* should change the value of an input field in a component * should throw when attempting to use ReactTestUtils.Simulate with shallow rendering +* should not warn when simulating events with extra properties * can scry with stateless components involved +* should set the type of the event src/test/__tests__/reactComponentExpect-test.js * should match composite components diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index b7e7a54db6..1e1516aeec 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -14,6 +14,7 @@ import type { HostChildren } from 'ReactFiberReconciler'; +var ReactControlledComponent = require('ReactControlledComponent'); var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); @@ -21,6 +22,9 @@ var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMInjection = require('ReactDOMInjection'); ReactDOMInjection.inject(); +ReactControlledComponent.injection.injectFiberControlledHostComponent( + ReactDOMFiberComponent +); var warning = require('warning'); diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 186d2f0b5f..a39c5c3bdb 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -33,7 +33,7 @@ var invariant = require('invariant'); var isEventSupported = require('isEventSupported'); var setInnerHTML = require('setInnerHTML'); var setTextContent = require('setTextContent'); -// var inputValueTracking = require('inputValueTracking'); +var inputValueTracking = require('inputValueTracking'); var warning = require('warning'); var didWarnShadyDOM = false; @@ -552,7 +552,7 @@ var ReactDOMFiberComponent = { props = ReactDOMFiberInput.getHostProps(domElement, props); // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. - //inputValueTracking.track(domElement); // TODO + inputValueTracking.trackNode(domElement); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. @@ -573,7 +573,7 @@ var ReactDOMFiberComponent = { case 'textarea': ReactDOMFiberTextarea.mountWrapper(domElement, props); props = ReactDOMFiberTextarea.getHostProps(domElement, props); - //inputValueTracking.track(domElement); // TODO + inputValueTracking.trackNode(domElement); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. diff --git a/src/renderers/dom/shared/inputValueTracking.js b/src/renderers/dom/shared/inputValueTracking.js index d69f31b383..257b81d47a 100644 --- a/src/renderers/dom/shared/inputValueTracking.js +++ b/src/renderers/dom/shared/inputValueTracking.js @@ -22,6 +22,9 @@ function isCheckable(elem) { } function getTracker(inst) { + if (typeof inst.tag === 'number') { + inst = inst.stateNode; + } return inst._wrapperState.valueTracker; } @@ -43,6 +46,54 @@ function getValueFromNode(node) { return value; } +function trackValueOnNode(node) { + var valueField = isCheckable(node) ? 'checked' : 'value'; + var descriptor = Object.getOwnPropertyDescriptor( + node.constructor.prototype, + valueField + ); + + var currentValue = '' + node[valueField]; + + // if someone has already defined a value or Safari, then bail + // and don't track value will cause over reporting of changes, + // but it's better then a hard failure + // (needed for certain tests that spyOn input values and Safari) + if ( + node.hasOwnProperty(valueField) || + typeof descriptor.get !== 'function' || + typeof descriptor.set !== 'function' + ) { + return; + } + + Object.defineProperty(node, valueField, { + enumerable: descriptor.enumerable, + configurable: true, + get: function() { + return descriptor.get.call(this); + }, + set: function(value) { + currentValue = '' + value; + descriptor.set.call(this, value); + }, + }); + + var tracker = { + getValue() { + return currentValue; + }, + setValue(value) { + currentValue = '' + value; + }, + stopTracking() { + detachTracker(inst); + delete node[valueField]; + }, + }; + return tracker; +} + var inputValueTracking = { // exposed for testing _getTrackerFromNode(node) { @@ -51,56 +102,19 @@ var inputValueTracking = { ); }, + trackNode: function(node) { + if (node._wrapperState.valueTracker) { + return; + } + node._wrapperState.valueTracker = trackValueOnNode(node); + }, + track: function(inst) { if (getTracker(inst)) { return; } - var node = ReactDOMComponentTree.getNodeFromInstance(inst); - var valueField = isCheckable(node) ? 'checked' : 'value'; - var descriptor = Object.getOwnPropertyDescriptor( - node.constructor.prototype, - valueField - ); - - var currentValue = '' + node[valueField]; - - // if someone has already defined a value or Safari, then bail - // and don't track value will cause over reporting of changes, - // but it's better then a hard failure - // (needed for certain tests that spyOn input values and Safari) - if ( - node.hasOwnProperty(valueField) || - typeof descriptor.get !== 'function' || - typeof descriptor.set !== 'function' - ) { - return; - } - - Object.defineProperty(node, valueField, { - enumerable: descriptor.enumerable, - configurable: true, - get: function() { - return descriptor.get.call(this); - }, - set: function(value) { - currentValue = '' + value; - descriptor.set.call(this, value); - }, - }); - - attachTracker(inst, { - getValue() { - return currentValue; - }, - setValue(value) { - currentValue = '' + value; - }, - stopTracking() { - detachTracker(inst); - delete node[valueField]; - }, - }); + attachTracker(inst, trackValueOnNode(node)); }, updateValueIfChanged(inst) { diff --git a/src/renderers/shared/shared/event/ReactControlledComponent.js b/src/renderers/shared/shared/event/ReactControlledComponent.js index c218a22059..29457eb079 100644 --- a/src/renderers/shared/shared/event/ReactControlledComponent.js +++ b/src/renderers/shared/shared/event/ReactControlledComponent.js @@ -36,7 +36,13 @@ function restoreStateOfTarget(internalInstance) { 'Fiber needs to be injected to handle a fiber target for controlled ' + 'events.' ); - fiberHostComponent.restoreControlledState(internalInstance); + // TODO: Ensure that this instance is the current one. Props needs to be correct. + fiberHostComponent.restoreControlledState( + internalInstance.stateNode, + internalInstance.type, + internalInstance.memoizedProps + ); + return; } invariant( typeof internalInstance.restoreControlledState === 'function', From 632ae806e8be646e9bb6c7860509f2d76d9cf0e5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 15:18:18 +0000 Subject: [PATCH 3/7] Give wrappers the raw props object Wrappers operate on the raw props object instead of the processed one. We should probably clean this up a bit since it is very confusing and unnecessary allocations to have two separate objects for props. --- .../dom/fiber/ReactDOMFiberComponent.js | 60 +++++++++++-------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index a39c5c3bdb..ecbd5a92b5 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -519,11 +519,11 @@ var ReactDOMFiberComponent = { setInitialProperties( domElement : Element, tag : string, - props : Object, + rawProps : Object, rootContainerElement : Element ) : void { - var isCustomComponentTag = isCustomComponent(tag, props); + var isCustomComponentTag = isCustomComponent(tag, rawProps); if (__DEV__) { if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { warning( @@ -536,6 +536,7 @@ var ReactDOMFiberComponent = { } } + var props : Object; switch (tag) { case 'audio': case 'form': @@ -546,10 +547,11 @@ var ReactDOMFiberComponent = { case 'source': case 'video': trapBubbledEventsLocal(domElement, tag); + props = rawProps; break; case 'input': - ReactDOMFiberInput.mountWrapper(domElement, props); - props = ReactDOMFiberInput.getHostProps(domElement, props); + ReactDOMFiberInput.mountWrapper(domElement, rawProps); + props = ReactDOMFiberInput.getHostProps(domElement, rawProps); // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. inputValueTracking.trackNode(domElement); @@ -559,26 +561,28 @@ var ReactDOMFiberComponent = { ensureListeningTo(rootContainerElement, 'onChange'); break; case 'option': - ReactDOMFiberOption.mountWrapper(domElement, props); - props = ReactDOMFiberOption.getHostProps(domElement, props); + ReactDOMFiberOption.mountWrapper(domElement, rawProps); + props = ReactDOMFiberOption.getHostProps(domElement, rawProps); break; case 'select': - ReactDOMFiberSelect.mountWrapper(domElement, props); - props = ReactDOMFiberSelect.getHostProps(domElement, props); + ReactDOMFiberSelect.mountWrapper(domElement, rawProps); + props = ReactDOMFiberSelect.getHostProps(domElement, rawProps); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. ensureListeningTo(rootContainerElement, 'onChange'); break; case 'textarea': - ReactDOMFiberTextarea.mountWrapper(domElement, props); - props = ReactDOMFiberTextarea.getHostProps(domElement, props); + ReactDOMFiberTextarea.mountWrapper(domElement, rawProps); + props = ReactDOMFiberTextarea.getHostProps(domElement, rawProps); inputValueTracking.trackNode(domElement); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. ensureListeningTo(rootContainerElement, 'onChange'); break; + default: + props = rawProps; } assertValidProps(tag, props); @@ -596,13 +600,13 @@ var ReactDOMFiberComponent = { // DOM yet. We need a special effect to handle this. switch (tag) { case 'input': - ReactDOMFiberInput.postMountWrapper(domElement, props); + ReactDOMFiberInput.postMountWrapper(domElement, rawProps); if (props.autoFocus) { focusNode(domElement); } break; case 'textarea': - ReactDOMFiberTextarea.postMountWrapper(domElement, props); + ReactDOMFiberTextarea.postMountWrapper(domElement, rawProps); if (props.autoFocus) { focusNode(domElement); } @@ -618,7 +622,7 @@ var ReactDOMFiberComponent = { } break; case 'option': - ReactDOMFiberOption.postMountWrapper(domElement, props); + ReactDOMFiberOption.postMountWrapper(domElement, rawProps); break; default: if (typeof props.onClick === 'function') { @@ -632,28 +636,32 @@ var ReactDOMFiberComponent = { updateProperties( domElement : Element, tag : string, - lastProps : Object, - nextProps : Object, + lastRawProps : Object, + nextRawProps : Object, rootContainerElement : Element ) : void { + var lastProps : Object; + var nextProps : Object; switch (tag) { case 'input': - lastProps = ReactDOMFiberInput.getHostProps(domElement, lastProps); - nextProps = ReactDOMFiberInput.getHostProps(domElement, nextProps); + lastProps = ReactDOMFiberInput.getHostProps(domElement, lastRawProps); + nextProps = ReactDOMFiberInput.getHostProps(domElement, nextRawProps); break; case 'option': - lastProps = ReactDOMFiberOption.getHostProps(domElement, lastProps); - nextProps = ReactDOMFiberOption.getHostProps(domElement, nextProps); + lastProps = ReactDOMFiberOption.getHostProps(domElement, lastRawProps); + nextProps = ReactDOMFiberOption.getHostProps(domElement, nextRawProps); break; case 'select': - lastProps = ReactDOMFiberSelect.getHostProps(domElement, lastProps); - nextProps = ReactDOMFiberSelect.getHostProps(domElement, nextProps); + lastProps = ReactDOMFiberSelect.getHostProps(domElement, lastRawProps); + nextProps = ReactDOMFiberSelect.getHostProps(domElement, nextRawProps); break; case 'textarea': - lastProps = ReactDOMFiberTextarea.getHostProps(domElement, lastProps); - nextProps = ReactDOMFiberTextarea.getHostProps(domElement, nextProps); + lastProps = ReactDOMFiberTextarea.getHostProps(domElement, lastRawProps); + nextProps = ReactDOMFiberTextarea.getHostProps(domElement, nextRawProps); break; default: + lastProps = lastRawProps; + nextProps = nextRawProps; if (typeof lastProps.onClick !== 'function' && typeof nextProps.onClick === 'function') { // TODO: This cast may not be sound for SVG, MathML or custom elements. @@ -679,15 +687,15 @@ var ReactDOMFiberComponent = { // Update the wrapper around inputs *after* updating props. This has to // happen after `updateDOMProperties`. Otherwise HTML5 input validations // raise warnings and prevent the new value from being assigned. - ReactDOMFiberInput.updateWrapper(domElement, nextProps); + ReactDOMFiberInput.updateWrapper(domElement, nextRawProps); break; case 'textarea': - ReactDOMFiberTextarea.updateWrapper(domElement, nextProps); + ReactDOMFiberTextarea.updateWrapper(domElement, nextRawProps); break; case 'select': // ); expect(log).toEqual([ - 'set data-reactroot', + ...(ReactDOMFeatureFlags.useFiber ? [] : ['set data-reactroot']), 'set type', 'set step', 'set min', @@ -1019,7 +1019,7 @@ describe('ReactDOMInput', () => { ReactTestUtils.renderIntoDocument(); expect(log).toEqual([ - 'node.setAttribute("data-reactroot", "")', + ...(ReactDOMFeatureFlags.useFiber ? [] : ['node.setAttribute("data-reactroot", "")']), 'node.setAttribute("type", "date")', 'node.setAttribute("value", "1980-01-01")', 'node.value = ""', From e06b335588beade38ac834c9f2453554ffcc9140 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 17:49:46 +0000 Subject: [PATCH 6/7] Add some Flow types to inputValueTracking Not perfect but better. --- .../dom/fiber/ReactDOMFiberComponent.js | 4 +- .../dom/shared/inputValueTracking.js | 44 +++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index ecbd5a92b5..a516224031 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -554,7 +554,7 @@ var ReactDOMFiberComponent = { props = ReactDOMFiberInput.getHostProps(domElement, rawProps); // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. - inputValueTracking.trackNode(domElement); + inputValueTracking.trackNode((domElement : any)); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. @@ -575,7 +575,7 @@ var ReactDOMFiberComponent = { case 'textarea': ReactDOMFiberTextarea.mountWrapper(domElement, rawProps); props = ReactDOMFiberTextarea.getHostProps(domElement, rawProps); - inputValueTracking.trackNode(domElement); + inputValueTracking.trackNode((domElement : any)); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. diff --git a/src/renderers/dom/shared/inputValueTracking.js b/src/renderers/dom/shared/inputValueTracking.js index 257b81d47a..a6311c184a 100644 --- a/src/renderers/dom/shared/inputValueTracking.js +++ b/src/renderers/dom/shared/inputValueTracking.js @@ -7,12 +7,26 @@ * of patent rights can be found in the PATENTS file in the same directory. * * @providesModule inputValueTracking + * @flow */ 'use strict'; + +import type { Fiber } from 'ReactFiber'; +import type { ReactInstance } from 'ReactInstanceType'; + +type ValueTracker = { + getValue() : string, + setValue(value : string) : void, + stopTracking() : void +}; +type WrapperState = { _wrapperState: { valueTracker: ?ValueTracker } }; +type ElementWithWrapperState = Element & WrapperState; +type InstanceWithWrapperState = ReactInstance & WrapperState; + var ReactDOMComponentTree = require('ReactDOMComponentTree'); -function isCheckable(elem) { +function isCheckable(elem : any) { var type = elem.type; var nodeName = elem.nodeName; return ( @@ -21,18 +35,18 @@ function isCheckable(elem) { ); } -function getTracker(inst) { +function getTracker(inst : any) { if (typeof inst.tag === 'number') { inst = inst.stateNode; } return inst._wrapperState.valueTracker; } -function attachTracker(inst, tracker) { +function attachTracker(inst : InstanceWithWrapperState, tracker : ?ValueTracker) { inst._wrapperState.valueTracker = tracker; } -function detachTracker(inst) { +function detachTracker(inst : InstanceWithWrapperState) { delete inst._wrapperState.valueTracker; } @@ -46,7 +60,7 @@ function getValueFromNode(node) { return value; } -function trackValueOnNode(node) { +function trackValueOnNode(node : any, inst : any) : ?ValueTracker { var valueField = isCheckable(node) ? 'checked' : 'value'; var descriptor = Object.getOwnPropertyDescriptor( node.constructor.prototype, @@ -96,35 +110,39 @@ function trackValueOnNode(node) { var inputValueTracking = { // exposed for testing - _getTrackerFromNode(node) { + _getTrackerFromNode(node : ElementWithWrapperState) { return getTracker( ReactDOMComponentTree.getInstanceFromNode(node) ); }, - trackNode: function(node) { + trackNode: function(node : ElementWithWrapperState) { if (node._wrapperState.valueTracker) { return; } - node._wrapperState.valueTracker = trackValueOnNode(node); + node._wrapperState.valueTracker = trackValueOnNode(node, node); }, - track: function(inst) { + track: function(inst : InstanceWithWrapperState) { if (getTracker(inst)) { return; } var node = ReactDOMComponentTree.getNodeFromInstance(inst); - attachTracker(inst, trackValueOnNode(node)); + attachTracker(inst, trackValueOnNode(node, inst)); }, - updateValueIfChanged(inst) { + updateValueIfChanged(inst : InstanceWithWrapperState | Fiber) { if (!inst) { return false; } var tracker = getTracker(inst); if (!tracker) { - inputValueTracking.track(inst); + if (typeof (inst : any).tag === 'number') { + inputValueTracking.trackNode((inst : any).stateNode); + } else { + inputValueTracking.track((inst : any)); + } return true; } @@ -141,7 +159,7 @@ var inputValueTracking = { return false; }, - stopTracking(inst) { + stopTracking(inst : InstanceWithWrapperState | Fiber) { var tracker = getTracker(inst); if (tracker) { tracker.stopTracking(); From c1daa6f2c1f6a6e3a771185d0decb6484f0a4c0b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 18:08:40 +0000 Subject: [PATCH 7/7] Record tests and fix BrowserEventEmitter test to not crash jest --- scripts/fiber/tests-failing.txt | 53 ----------------- scripts/fiber/tests-passing-except-dev.txt | 10 ---- scripts/fiber/tests-passing.txt | 59 +++++++++++++++++++ .../ReactBrowserEventEmitter-test.js | 6 +- 4 files changed, 62 insertions(+), 66 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 5508db8337..f0b52aab32 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -42,7 +42,6 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js * should have mouse enter simulated by test utils * should infer onTouchTap from a touchStart/End * should infer onTouchTap from when dragging below threshold -* should not onTouchTap from when dragging beyond threshold * should bubble onTouchTap src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -66,65 +65,28 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * can reconcile text arbitrarily split into multiple nodes on some substitutions only src/renderers/dom/shared/__tests__/ReactEventIndependence-test.js -* does not crash with other react inside -* does not crash with other react outside * does not when event fired on unmounted tree src/renderers/dom/shared/__tests__/ReactEventListener-test.js * should batch between handlers from different roots * should not fire duplicate events for a React DOM tree -src/renderers/dom/shared/__tests__/inputValueTracking-test.js -* should stop tracking - src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js -* should fire change for checkbox input -* should not fire change when setting the value programmatically * should not fire change when setting checked programmatically -* should only fire change for checked radio button once -* should deduplicate input value change events -* should listen for both change and input events when supported -* should only fire events when the value changes for range inputs - -src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js -* should skip extraction if no listeners are present -* should extract if an `onSelect` listener is present src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js -* A non-interactive tags click when disabled * A non-interactive tags clicks bubble when disabled -* should forward clicks when it starts out not disabled * should not forward clicks when it starts out disabled -* should forward clicks when it becomes not disabled * should not forward clicks when it becomes disabled -* should work correctly if the listener is changed -* should forward clicks when it starts out not disabled * should not forward clicks when it starts out disabled -* should forward clicks when it becomes not disabled * should not forward clicks when it becomes disabled -* should work correctly if the listener is changed -* should forward clicks when it starts out not disabled * should not forward clicks when it starts out disabled -* should forward clicks when it becomes not disabled * should not forward clicks when it becomes disabled -* should work correctly if the listener is changed -* should forward clicks when it starts out not disabled * should not forward clicks when it starts out disabled -* should forward clicks when it becomes not disabled * should not forward clicks when it becomes disabled -* should work correctly if the listener is changed - -src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js -* should trigger load events src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should control a value in reentrant events -* should update `defaultValue` for uncontrolled input -* should update `defaultValue` for uncontrolled date/time input -* should have the correct target value -* should update defaultValue to empty string -* sets type, step, min, max before value always -* resets value of date/time input to fix bugs in iOS Safari src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should allow ignoring `value` on option @@ -136,25 +98,13 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should allow setting `value` * should allow setting `value` with multiple * should not select other options automatically -* should reset child options selected when they are changed and `value` is set * should allow setting `value` with `objectToString` * should allow switching to multiple * should allow switching from multiple * should remember value when switching to uncontrolled -* should remember updated value when switching to uncontrolled * should not control defaultValue if readding options * should select grandchild options nested inside an optgroup -src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js -* should set defaultValue -* should update defaultValue to empty string -* should allow setting `value` to `giraffe` -* should allow setting `value` to `true` -* should allow setting `value` to `false` -* should allow setting `value` to `objToString` -* should not incur unnecessary DOM mutations -* should keep value when switching to uncontrolled element if changed - src/renderers/dom/stack/client/__tests__/ReactDOM-test.js * throws in render() if the mount callback is not a function * throws in render() if the update callback is not a function @@ -287,9 +237,6 @@ src/renderers/shared/hooks/__tests__/ReactHostOperationHistoryHook-test.js * gets reported when a child is inserted * gets reported when a child is removed -src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js -* should not prevent null listeners, at dispatch - src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js * should throw on invalid render targets * throws usefully when rendering badly-typed elements diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index 1c5ce695df..ee0f111d97 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -27,7 +27,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn for unknown prop * should group multiple unknown prop warnings together * should warn for onDblClick prop -* should work error event on element * should emit a warning once for a named custom component using shady DOM * should not warn when server-side rendering `onScroll` * warns on invalid nesting @@ -48,17 +47,8 @@ src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should warn if value is null -* should warn if controlled input switches to uncontrolled (value is undefined) * should warn if controlled input switches to uncontrolled (value is null) -* should warn if controlled input switches to uncontrolled with defaultValue * should warn if uncontrolled input (value is null) switches to controlled -* should warn if controlled checkbox switches to uncontrolled (checked is undefined) -* should warn if controlled checkbox switches to uncontrolled (checked is null) -* should warn if controlled checkbox switches to uncontrolled with defaultChecked -* should warn if controlled radio switches to uncontrolled (checked is undefined) -* should warn if controlled radio switches to uncontrolled (checked is null) -* should warn if controlled radio switches to uncontrolled with defaultChecked -* should warn if radio checked false changes to become uncontrolled src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should warn if value is null diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 369ffa6dcc..0d4380bd0f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -531,6 +531,7 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js * should stop after first dispatch if stopPropagation * should invoke handlers that were removed while bubbling * should not invoke newly inserted handlers while bubbling +* should not onTouchTap from when dragging beyond threshold * should listen to events only once * should work with event plugins without dependencies * should work with event plugins with dependencies @@ -567,6 +568,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should generate the correct markup with className * should escape style names and values * should handle dangerouslySetInnerHTML +* should work error event on element * should not duplicate uppercased selfclosing tags * should warn against children for void elements * should warn against dangerouslySetInnerHTML for void elements @@ -613,6 +615,10 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * can be toggled in and out of the markup * can reconcile text from pre-rendered markup +src/renderers/dom/shared/__tests__/ReactEventIndependence-test.js +* does not crash with other react inside +* does not crash with other react outside + src/renderers/dom/shared/__tests__/ReactEventListener-test.js * should dispatch events from outside React tree * should propagate events one level down @@ -638,6 +644,7 @@ src/renderers/dom/shared/__tests__/inputValueTracking-test.js * should update value if it changed and return result * should track value and return true when updating untracked instance * should return tracker from node +* should stop tracking src/renderers/dom/shared/__tests__/quoteAttributeValueForBrowser-test.js * should escape boolean to string @@ -655,8 +662,14 @@ src/renderers/dom/shared/eventPlugins/__tests__/BeforeInputEventPlugin-test.js * extract onBeforeInput from fallback objects src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js +* should fire change for checkbox input * should catch setting the value programmatically +* should not fire change when setting the value programmatically * should unmount +* should only fire change for checked radio button once +* should deduplicate input value change events +* should listen for both change and input events when supported +* should only fire events when the value changes for range inputs src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js * should set relatedTarget properly in iframe @@ -670,7 +683,24 @@ src/renderers/dom/shared/eventPlugins/__tests__/FallbackCompositionState-test.js * extracts when inserted within text * extracts when inserted at end of text +src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js +* should skip extraction if no listeners are present +* should extract if an `onSelect` listener is present + src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js +* A non-interactive tags click when disabled +* should forward clicks when it starts out not disabled +* should forward clicks when it becomes not disabled +* should work correctly if the listener is changed +* should forward clicks when it starts out not disabled +* should forward clicks when it becomes not disabled +* should work correctly if the listener is changed +* should forward clicks when it starts out not disabled +* should forward clicks when it becomes not disabled +* should work correctly if the listener is changed +* should forward clicks when it starts out not disabled +* should forward clicks when it becomes not disabled +* should work correctly if the listener is changed * does not add a local click to interactive elements * adds a local click listener to non-interactive elements @@ -738,6 +768,9 @@ src/renderers/dom/shared/utils/__tests__/getNodeForCharacterOffset-test.js src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js * sets innerHTML on it +src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js +* should trigger load events + src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should properly control a value even if no event listener exists * should control values in reentrant events with different targets @@ -745,6 +778,8 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * only assigns defaultValue if it changes * should display "true" for `defaultValue` of `true` * should display "false" for `defaultValue` of `false` +* should update `defaultValue` for uncontrolled input +* should update `defaultValue` for uncontrolled date/time input * should take `defaultValue` when changing to uncontrolled input * should render defaultValue for SSR * should render value for SSR @@ -759,23 +794,36 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations * should properly control a value of number `0` +* should have the correct target value * should not set a value for submit buttons unnecessarily * should control radio buttons * should control radio buttons if the tree updates during render * should warn with value and no onChange handler and readOnly specified * should have a this value of undefined if bind is not used * should warn with checked and no onChange handler with readOnly specified +* should update defaultValue to empty string * should warn if checked and defaultChecked props are specified * should warn if value and defaultValue props are specified +* should warn if controlled input switches to uncontrolled (value is undefined) +* should warn if controlled input switches to uncontrolled with defaultValue * should warn if uncontrolled input (value is undefined) switches to controlled +* should warn if controlled checkbox switches to uncontrolled (checked is undefined) +* should warn if controlled checkbox switches to uncontrolled (checked is null) +* should warn if controlled checkbox switches to uncontrolled with defaultChecked * should warn if uncontrolled checkbox (checked is undefined) switches to controlled * should warn if uncontrolled checkbox (checked is null) switches to controlled +* should warn if controlled radio switches to uncontrolled (checked is undefined) +* should warn if controlled radio switches to uncontrolled (checked is null) +* should warn if controlled radio switches to uncontrolled with defaultChecked * should warn if uncontrolled radio (checked is undefined) switches to controlled * should warn if uncontrolled radio (checked is null) switches to controlled * should not warn if radio value changes but never becomes controlled * should not warn if radio value changes but never becomes uncontrolled +* should warn if radio checked false changes to become uncontrolled +* sets type, step, min, max before value always * sets value properly with type coming later in props * does not raise a validation warning when it switches types +* resets value of date/time input to fix bugs in iOS Safari src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should flatten children to a string @@ -787,6 +835,8 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should not throw with `defaultValue` and without children * should not throw with `value` and without children +* should reset child options selected when they are changed and `value` is set +* should remember updated value when switching to uncontrolled * should support server-side rendering * should support server-side rendering with defaultValue * should support server-side rendering with multiple @@ -799,15 +849,23 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should display `defaultValue` of number 0 * should display "false" for `defaultValue` of `false` * should display "foobar" for `defaultValue` of `objToString` +* should set defaultValue * should not render value as an attribute * should display `value` of number 0 +* should update defaultValue to empty string +* should allow setting `value` to `giraffe` * should render defaultValue for SSR * should render value for SSR +* should allow setting `value` to `true` +* should allow setting `value` to `false` +* should allow setting `value` to `objToString` * should take updates to `defaultValue` for uncontrolled textarea * should take updates to children in lieu of `defaultValue` for uncontrolled textarea +* should not incur unnecessary DOM mutations * should properly control a value of number `0` * should treat children like `defaultValue` * should keep value when switching to uncontrolled element if not changed +* should keep value when switching to uncontrolled element if changed * should allow numbers as children * should allow booleans as children * should allow objects as children @@ -1090,6 +1148,7 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js * should prevent non-function listeners, at dispatch +* should not prevent null listeners, at dispatch src/renderers/shared/shared/event/__tests__/EventPluginRegistry-test.js * should be able to inject ordering before plugins diff --git a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js index b9ef438210..6462df35f6 100644 --- a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js +++ b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js @@ -496,9 +496,9 @@ describe('ReactBrowserEventEmitter', () => { ReactTestUtils.nativeTouchData(0, 0) ); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0] === CHILD).toBe(true); + expect(idCallOrder[1] === PARENT).toBe(true); + expect(idCallOrder[2] === GRANDPARENT).toBe(true); }); it('should not crash ensureScrollValueMonitoring when createEvent returns null', () => {