From 5aeebed7dff270a7b1f4e852ee67ac22653c4023 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 6 Jun 2017 14:32:23 -0700 Subject: [PATCH 1/2] Don't hydrate any properties other than event listeners and text content This strategy assumes that the rendered HTML is correct if the tree lines up. Therefore we don't diff any attributes of the rendered HTML. However, as a precaution I ensure that textContent *is* updated. This ensures that if something goes wrong with keys lining up etc. at least there is some feedback that the event handlers might not line up. With what you expect. This might not be what you want e.g. for date formatting where it can different between server and client. It is expected that content will line up. To ensure that I will in a follow up ensure that the warning is issued if it doesn't line up so that in development this can be addressed. The text content updates are now moved to the commit phase so if the tree is asynchronously hydrated it doesn't start partially swapping out. I use the regular update side-effect with payload if the content doesn't match up. Since we no longer guarantee that attributes is correct I changed the bad mark up SSR integration tests to only assert on the textContent instead. --- scripts/fiber/tests-failing.txt | 6 +- scripts/fiber/tests-passing.txt | 4 + src/renderers/dom/fiber/ReactDOMFiber.js | 9 +- .../dom/fiber/ReactDOMFiberComponent.js | 138 +++++++++++++++++- .../dom/fiber/wrappers/ReactDOMFiberInput.js | 2 +- .../dom/fiber/wrappers/ReactDOMFiberOption.js | 2 +- .../dom/fiber/wrappers/ReactDOMFiberSelect.js | 6 +- .../fiber/wrappers/ReactDOMFiberTextarea.js | 2 +- .../ReactDOMServerIntegration-test.js | 41 +++++- .../shared/fiber/ReactFiberBeginWork.js | 2 +- .../shared/fiber/ReactFiberCommitWork.js | 18 ++- .../shared/fiber/ReactFiberCompleteWork.js | 35 +++-- .../fiber/ReactFiberHydrationContext.js | 42 ++++-- .../shared/fiber/ReactFiberReconciler.js | 5 +- .../shared/fiber/ReactFiberScheduler.js | 8 +- 15 files changed, 256 insertions(+), 64 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 789af77694f0..8b137891791f 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -1,5 +1 @@ -src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js -* should not blow away user-entered text on successful reconnect to an uncontrolled checkbox -* should not blow away user-entered text on successful reconnect to a controlled checkbox -* should not blow away user-selected value on successful reconnect to an uncontrolled select -* should not blow away user-selected value on successful reconnect to an controlled select + diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 930fb89124d8..4f724b760d07 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1226,6 +1226,10 @@ src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js * renders a controlled select with client render on top of good server markup * should not blow away user-entered text on successful reconnect to an uncontrolled input * should not blow away user-entered text on successful reconnect to a controlled input +* should not blow away user-entered text on successful reconnect to an uncontrolled checkbox +* should not blow away user-entered text on successful reconnect to a controlled checkbox +* should not blow away user-selected value on successful reconnect to an uncontrolled select +* should not blow away user-selected value on successful reconnect to an controlled select * renders class child with context with server string render * renders class child with context with clean client render * renders class child with context with client render on top of good server markup diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index b17a2d40f6d2..31cafd4873e1 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -48,6 +48,7 @@ var { setInitialProperties, diffProperties, updateProperties, + diffHydratedProperties, } = ReactDOMFiberComponent; var {precacheFiberNode, updateFiberProps} = ReactDOMComponentTree; @@ -409,19 +410,21 @@ var DOMRenderer = ReactFiberReconciler({ props: Props, rootContainerInstance: Container, internalInstanceHandle: Object, - ): void { + ): null | Array { precacheFiberNode(internalInstanceHandle, instance); // TODO: Possibly defer this until the commit phase where all the events // get attached. updateFiberProps(instance, props); - setInitialProperties(instance, type, props, rootContainerInstance); + return diffHydratedProperties(instance, type, props, rootContainerInstance); }, hydrateTextInstance( textInstance: TextInstance, + text: string, internalInstanceHandle: Object, - ): void { + ): boolean { precacheFiberNode(internalInstanceHandle, textInstance); + return textInstance.nodeValue !== text; }, scheduleAnimationCallback: ReactDOMFrameScheduling.rAF, diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 5c29bd810f8d..2406de28db1a 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -177,10 +177,10 @@ function setInitialDOMProperties( isCustomComponentTag: boolean, ): void { for (var propKey in nextProps) { - var nextProp = nextProps[propKey]; if (!nextProps.hasOwnProperty(propKey)) { continue; } + var nextProp = nextProps[propKey]; if (propKey === STYLE) { if (__DEV__) { if (nextProp) { @@ -406,7 +406,7 @@ var ReactDOMFiberComponent = { props = rawProps; break; case 'input': - ReactDOMFiberInput.mountWrapper(domElement, rawProps); + ReactDOMFiberInput.initWrapperState(domElement, rawProps); props = ReactDOMFiberInput.getHostProps(domElement, rawProps); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening @@ -414,11 +414,11 @@ var ReactDOMFiberComponent = { ensureListeningTo(rootContainerElement, 'onChange'); break; case 'option': - ReactDOMFiberOption.mountWrapper(domElement, rawProps); + ReactDOMFiberOption.validateProps(domElement, rawProps); props = ReactDOMFiberOption.getHostProps(domElement, rawProps); break; case 'select': - ReactDOMFiberSelect.mountWrapper(domElement, rawProps); + ReactDOMFiberSelect.initWrapperState(domElement, rawProps); props = ReactDOMFiberSelect.getHostProps(domElement, rawProps); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening @@ -426,7 +426,7 @@ var ReactDOMFiberComponent = { ensureListeningTo(rootContainerElement, 'onChange'); break; case 'textarea': - ReactDOMFiberTextarea.mountWrapper(domElement, rawProps); + ReactDOMFiberTextarea.initWrapperState(domElement, rawProps); props = ReactDOMFiberTextarea.getHostProps(domElement, rawProps); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening @@ -462,6 +462,9 @@ var ReactDOMFiberComponent = { case 'option': ReactDOMFiberOption.postMountWrapper(domElement, rawProps); break; + case 'select': + ReactDOMFiberSelect.postMountWrapper(domElement, rawProps); + break; default: if (typeof props.onClick === 'function') { // TODO: This cast may not be sound for SVG, MathML or custom elements. @@ -704,6 +707,131 @@ var ReactDOMFiberComponent = { } }, + diffHydratedProperties( + domElement: Element, + tag: string, + rawProps: Object, + rootContainerElement: Element | Document, + ): null | Array { + if (__DEV__) { + var isCustomComponentTag = isCustomComponent(tag, rawProps); + validatePropertiesInDevelopment(tag, rawProps); + if (isCustomComponentTag && !didWarnShadyDOM && domElement.shadyRoot) { + warning( + false, + '%s is using shady DOM. Using shady DOM with React can ' + + 'cause things to break subtly.', + getCurrentFiberOwnerName() || 'A component', + ); + didWarnShadyDOM = true; + } + } + + switch (tag) { + case 'audio': + case 'form': + case 'iframe': + case 'img': + case 'image': + case 'link': + case 'object': + case 'source': + case 'video': + case 'details': + trapBubbledEventsLocal(domElement, tag); + break; + case 'input': + ReactDOMFiberInput.initWrapperState(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 'option': + ReactDOMFiberOption.validateProps(domElement, rawProps); + break; + case 'select': + ReactDOMFiberSelect.initWrapperState(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.initWrapperState(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; + } + + assertValidProps(tag, rawProps, getCurrentFiberOwnerName); + + var updatePayload = null; + for (var propKey in rawProps) { + if (!rawProps.hasOwnProperty(propKey)) { + continue; + } + var nextProp = rawProps[propKey]; + if (propKey === CHILDREN) { + // For text content children we compare against textContent. This + // might match additional HTML that is hidden when we read it using + // textContent. E.g. "foo" will match "foo" but that still + // satisfies our requirement. Our requirement is not to produce perfect + // HTML and attributes. Ideally we should preserve structure but it's + // ok not to if the visible content is still enough to indicate what + // even listeners these nodes might be wired up to. + // TODO: Warn if there is more than a single textNode as a child. + // TODO: Should we use domElement.firstChild.nodeValue to compare? + if (typeof nextProp === 'string') { + if (domElement.textContent !== nextProp) { + updatePayload = [CHILDREN, nextProp]; + } + } else if (typeof nextProp === 'number') { + if (domElement.textContent !== '' + nextProp) { + updatePayload = [CHILDREN, '' + nextProp]; + } + } + } else if (registrationNameModules.hasOwnProperty(propKey)) { + if (nextProp) { + ensureListeningTo(rootContainerElement, propKey); + } + } + } + + switch (tag) { + case 'input': + // 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: any)); + ReactDOMFiberInput.postMountWrapper(domElement, rawProps); + break; + case 'textarea': + // 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: any)); + ReactDOMFiberTextarea.postMountWrapper(domElement, rawProps); + break; + case 'select': + case 'option': + // For input and textarea we current always set the value property at + // post mount to force it to diverge from attributes. However, for + // option and select we don't quite do the same thing and select + // is not resilient to the DOM state changing so we don't do that here. + // TODO: Consider not doing this for input and textarea. + break; + default: + if (typeof rawProps.onClick === 'function') { + // TODO: This cast may not be sound for SVG, MathML or custom elements. + trapClickOnNonInteractiveElement(((domElement: any): HTMLElement)); + } + break; + } + + return updatePayload; + }, + restoreControlledState( domElement: Element, tag: string, diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js index 17b0f3df0250..df430502db33 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js @@ -89,7 +89,7 @@ var ReactDOMInput = { return hostProps; }, - mountWrapper: function(element: Element, props: Object) { + initWrapperState: function(element: Element, props: Object) { if (__DEV__) { ReactControlledValuePropTypes.checkPropTypes( 'input', diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js index 7f0a25935a22..781f94924d14 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberOption.js @@ -39,7 +39,7 @@ function flattenChildren(children) { * Implements an