From 4e900de807531f22a694b72d65f8596f4f7016ae Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 11 Sep 2017 20:17:15 +0100 Subject: [PATCH 1/3] Fix false positive hydration warning for SVG attributes --- .../dom/fiber/ReactDOMFiberComponent.js | 2 +- .../ReactDOMServerIntegration-test.js | 29 +++++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 4fe120cdaf479..a43a1e2073e43 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -912,7 +912,7 @@ var ReactDOMFiberComponent = { case 'selected': break; default: - extraAttributeNames.add(attributes[i].name); + extraAttributeNames.add(name); } } } diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 9016dad76cbdf..0d954c0d911a6 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -1325,19 +1325,30 @@ describe('ReactDOMServerIntegration', () => { expect(e.namespaceURI).toBe('http://www.w3.org/2000/svg'); }); - itRenders('svg child element', async render => { - let e = await render( - , - ); - e = e.firstChild; + itRenders('svg child element with an attribute', async render => { + let e = await render(); expect(e.childNodes.length).toBe(0); - expect(e.tagName).toBe('image'); + expect(e.tagName).toBe('svg'); expect(e.namespaceURI).toBe('http://www.w3.org/2000/svg'); - expect(e.getAttributeNS('http://www.w3.org/1999/xlink', 'href')).toBe( - 'http://i.imgur.com/w7GCRPb.png', - ); + expect(e.getAttribute('viewBox')).toBe('0 0 0 0'); }); + itRenders( + 'svg child element with a namespace attribute', + async render => { + let e = await render( + , + ); + e = e.firstChild; + expect(e.childNodes.length).toBe(0); + expect(e.tagName).toBe('image'); + expect(e.namespaceURI).toBe('http://www.w3.org/2000/svg'); + expect(e.getAttributeNS('http://www.w3.org/1999/xlink', 'href')).toBe( + 'http://i.imgur.com/w7GCRPb.png', + ); + }, + ); + itRenders('svg child element with a badly cased alias', async render => { let e = await render( , From ce09e189a2d73c39ae3bb812e23a72b59ee7d971 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 12 Sep 2017 19:47:15 +0100 Subject: [PATCH 2/3] Undo the generic fix --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index a43a1e2073e43..d9376f13c9eab 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -912,7 +912,9 @@ var ReactDOMFiberComponent = { case 'selected': break; default: - extraAttributeNames.add(name); + // Intentionally use the original name. + // See discussion in https://github.com/facebook/react/pull/10676. + extraAttributeNames.add(attributes[i].name); } } } From 8c30d5b477c3e6fcb929209cac8b911665aead42 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 12 Sep 2017 20:00:52 +0100 Subject: [PATCH 3/3] Pass namespace through and use it to determine sensitivity --- .../dom/fiber/ReactDOMFiberComponent.js | 14 ++++++++++++-- src/renderers/dom/fiber/ReactDOMFiberEntry.js | 16 +++++++++++++++- .../shared/fiber/ReactFiberBeginWork.js | 2 +- .../shared/fiber/ReactFiberCompleteWork.js | 3 ++- .../shared/fiber/ReactFiberHydrationContext.js | 12 +++++++++--- .../shared/fiber/ReactFiberReconciler.js | 1 + .../shared/fiber/ReactFiberScheduler.js | 2 +- 7 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index d9376f13c9eab..a6323f9643994 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -776,6 +776,7 @@ var ReactDOMFiberComponent = { domElement: Element, tag: string, rawProps: Object, + parentNamespace: string, rootContainerElement: Element | Document, ): null | Array { if (__DEV__) { @@ -1009,8 +1010,17 @@ var ReactDOMFiberComponent = { nextProp, ); } else { - // $FlowFixMe - Should be inferred as not undefined. - extraAttributeNames.delete(propKey.toLowerCase()); + let ownNamespace = parentNamespace; + if (ownNamespace === HTML_NAMESPACE) { + ownNamespace = getIntrinsicNamespace(tag); + } + if (ownNamespace === HTML_NAMESPACE) { + // $FlowFixMe - Should be inferred as not undefined. + extraAttributeNames.delete(propKey.toLowerCase()); + } else { + // $FlowFixMe - Should be inferred as not undefined. + extraAttributeNames.delete(propKey); + } serverValue = DOMPropertyOperations.getValueForAttribute( domElement, propKey, diff --git a/src/renderers/dom/fiber/ReactDOMFiberEntry.js b/src/renderers/dom/fiber/ReactDOMFiberEntry.js index 322a8262dfd3a..6fa9eef2416dd 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberEntry.js +++ b/src/renderers/dom/fiber/ReactDOMFiberEntry.js @@ -488,13 +488,27 @@ var DOMRenderer = ReactFiberReconciler({ type: string, props: Props, rootContainerInstance: Container, + hostContext: HostContext, internalInstanceHandle: Object, ): null | Array { precacheFiberNode(internalInstanceHandle, instance); // TODO: Possibly defer this until the commit phase where all the events // get attached. updateFiberProps(instance, props); - return diffHydratedProperties(instance, type, props, rootContainerInstance); + let parentNamespace: string; + if (__DEV__) { + const hostContextDev = ((hostContext: any): HostContextDev); + parentNamespace = hostContextDev.namespace; + } else { + parentNamespace = ((hostContext: any): HostContextProd); + } + return diffHydratedProperties( + instance, + type, + props, + parentNamespace, + rootContainerInstance, + ); }, hydrateTextInstance( diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index d9821063f47bd..99bf60d62772d 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -72,7 +72,7 @@ if (__DEV__) { module.exports = function( config: HostConfig, hostContext: HostContext, - hydrationContext: HydrationContext, + hydrationContext: HydrationContext, scheduleUpdate: (fiber: Fiber, priorityLevel: PriorityLevel) => void, getPriorityContext: (fiber: Fiber, forceAsync: boolean) => PriorityLevel, ) { diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index c733f036933da..aeced86f4cb61 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -50,7 +50,7 @@ var invariant = require('fbjs/lib/invariant'); module.exports = function( config: HostConfig, hostContext: HostContext, - hydrationContext: HydrationContext, + hydrationContext: HydrationContext, ) { const { createInstance, @@ -285,6 +285,7 @@ module.exports = function( prepareToHydrateHostInstance( workInProgress, rootContainerInstance, + currentHostContext, ) ) { // If changes to the hydrated node needs to be applied at the diff --git a/src/renderers/shared/fiber/ReactFiberHydrationContext.js b/src/renderers/shared/fiber/ReactFiberHydrationContext.js index e39202b2aead6..23437cb384c1c 100644 --- a/src/renderers/shared/fiber/ReactFiberHydrationContext.js +++ b/src/renderers/shared/fiber/ReactFiberHydrationContext.js @@ -22,18 +22,22 @@ const {Deletion, Placement} = require('ReactTypeOfSideEffect'); const {createFiberFromHostInstanceForDeletion} = require('ReactFiber'); -export type HydrationContext = { +export type HydrationContext = { enterHydrationState(fiber: Fiber): boolean, resetHydrationState(): void, tryToClaimNextHydratableInstance(fiber: Fiber): void, - prepareToHydrateHostInstance(fiber: Fiber, rootContainerInstance: C): boolean, + prepareToHydrateHostInstance( + fiber: Fiber, + rootContainerInstance: C, + hostContext: CX, + ): boolean, prepareToHydrateHostTextInstance(fiber: Fiber): boolean, popHydrationState(fiber: Fiber): boolean, }; module.exports = function( config: HostConfig, -): HydrationContext { +): HydrationContext { const { shouldSetTextContent, canHydrateInstance, @@ -218,6 +222,7 @@ module.exports = function( function prepareToHydrateHostInstance( fiber: Fiber, rootContainerInstance: C, + hostContext: CX, ): boolean { const instance: I = fiber.stateNode; const updatePayload = hydrateInstance( @@ -225,6 +230,7 @@ module.exports = function( fiber.type, fiber.memoizedProps, rootContainerInstance, + hostContext, fiber, ); // TODO: Type this specific to this type of component. diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 3e8d0a94563f9..1ebef2708168c 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -133,6 +133,7 @@ export type HostConfig = { type: T, props: P, rootContainerInstance: C, + hostContext: CX, internalInstanceHandle: OpaqueHandle, ) => null | PL, hydrateTextInstance?: ( diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 3af4081687dfc..963971d6cb7f2 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -152,7 +152,7 @@ module.exports = function( config: HostConfig, ) { const hostContext = ReactFiberHostContext(config); - const hydrationContext: HydrationContext = ReactFiberHydrationContext( + const hydrationContext: HydrationContext = ReactFiberHydrationContext( config, ); const {popHostContainer, popHostContext, resetHostContainer} = hostContext;