Skip to content

Commit

Permalink
Fix false positive hydration warning for SVG attributes (#10676)
Browse files Browse the repository at this point in the history
* Fix false positive hydration warning for SVG attributes

* Undo the generic fix

* Pass namespace through and use it to determine sensitivity
  • Loading branch information
gaearon authored Sep 13, 2017
1 parent 65b9ad9 commit c55ffb3
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 18 deletions.
16 changes: 14 additions & 2 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ var ReactDOMFiberComponent = {
domElement: Element,
tag: string,
rawProps: Object,
parentNamespace: string,
rootContainerElement: Element | Document,
): null | Array<mixed> {
if (__DEV__) {
Expand Down Expand Up @@ -912,6 +913,8 @@ var ReactDOMFiberComponent = {
case 'selected':
break;
default:
// Intentionally use the original name.
// See discussion in https://github.com/facebook/react/pull/10676.
extraAttributeNames.add(attributes[i].name);
}
}
Expand Down Expand Up @@ -1007,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,
Expand Down
16 changes: 15 additions & 1 deletion src/renderers/dom/fiber/ReactDOMFiberEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,27 @@ var DOMRenderer = ReactFiberReconciler({
type: string,
props: Props,
rootContainerInstance: Container,
hostContext: HostContext,
internalInstanceHandle: Object,
): null | Array<mixed> {
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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<svg><image xlinkHref="http://i.imgur.com/w7GCRPb.png" /></svg>,
);
e = e.firstChild;
itRenders('svg child element with an attribute', async render => {
let e = await render(<svg viewBox="0 0 0 0" />);
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(
<svg><image xlinkHref="http://i.imgur.com/w7GCRPb.png" /></svg>,
);
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(
<svg><image xlinkhref="http://i.imgur.com/w7GCRPb.png" /></svg>,
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ if (__DEV__) {
module.exports = function<T, P, I, TI, PI, C, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
hostContext: HostContext<C, CX>,
hydrationContext: HydrationContext<C>,
hydrationContext: HydrationContext<C, CX>,
scheduleUpdate: (fiber: Fiber, priorityLevel: PriorityLevel) => void,
getPriorityContext: (fiber: Fiber, forceAsync: boolean) => PriorityLevel,
) {
Expand Down
3 changes: 2 additions & 1 deletion src/renderers/shared/fiber/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var invariant = require('fbjs/lib/invariant');
module.exports = function<T, P, I, TI, PI, C, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
hostContext: HostContext<C, CX>,
hydrationContext: HydrationContext<C>,
hydrationContext: HydrationContext<C, CX>,
) {
const {
createInstance,
Expand Down Expand Up @@ -289,6 +289,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
prepareToHydrateHostInstance(
workInProgress,
rootContainerInstance,
currentHostContext,
)
) {
// If changes to the hydrated node needs to be applied at the
Expand Down
12 changes: 9 additions & 3 deletions src/renderers/shared/fiber/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,22 @@ const {Deletion, Placement} = require('ReactTypeOfSideEffect');

const {createFiberFromHostInstanceForDeletion} = require('ReactFiber');

export type HydrationContext<C> = {
export type HydrationContext<C, CX> = {
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<T, P, I, TI, PI, C, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
): HydrationContext<C> {
): HydrationContext<C, CX> {
const {
shouldSetTextContent,
canHydrateInstance,
Expand Down Expand Up @@ -218,13 +222,15 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
function prepareToHydrateHostInstance(
fiber: Fiber,
rootContainerInstance: C,
hostContext: CX,
): boolean {
const instance: I = fiber.stateNode;
const updatePayload = hydrateInstance(
instance,
fiber.type,
fiber.memoizedProps,
rootContainerInstance,
hostContext,
fiber,
);
// TODO: Type this specific to this type of component.
Expand Down
1 change: 1 addition & 0 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export type HostConfig<T, P, I, TI, PI, C, CX, PL> = {
type: T,
props: P,
rootContainerInstance: C,
hostContext: CX,
internalInstanceHandle: OpaqueHandle,
) => null | PL,
hydrateTextInstance?: (
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
) {
const hostContext = ReactFiberHostContext(config);
const hydrationContext: HydrationContext<C> = ReactFiberHydrationContext(
const hydrationContext: HydrationContext<C, CX> = ReactFiberHydrationContext(
config,
);
const {popHostContainer, popHostContext, resetHostContainer} = hostContext;
Expand Down

0 comments on commit c55ffb3

Please sign in to comment.