Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ src/renderers/dom/shared/__tests__/ReactDOM-test.js
* throws in render() if the update callback is not a function

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should empty element when removing innerHTML
* should transition from innerHTML to children in nested el
* should warn for children on void elements
* should report component containing invalid styles
* should clean up input value tracking
Expand Down Expand Up @@ -104,7 +102,6 @@ src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js
* throws when rendering null at the top level

src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js
* should correctly handle all possible children for render and update
* should reorder keyed text nodes

src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
Expand Down
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,10 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
* should update arbitrary attributes for tags containing dashes
* should clear all the styles when removing `style`
* should update styles when `style` changes from null to object
* should empty element when removing innerHTML
* should transition from string content to innerHTML
* should transition from innerHTML to string content
* should transition from innerHTML to children in nested el
* should transition from children to innerHTML in nested el
* should not incur unnecessary DOM mutations for attributes
* should not incur unnecessary DOM mutations for string properties
Expand Down Expand Up @@ -1397,6 +1399,7 @@ src/renderers/shared/shared/__tests__/ReactMultiChildReconcile-test.js
* should insert non-empty children in middle where nulls were

src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js
* should correctly handle all possible children for render and update
* should throw if rendering both HTML and children
* should render between nested components and inline children

Expand Down
16 changes: 16 additions & 0 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,22 @@ var DOMRenderer = ReactFiberReconciler({
updateProperties(domElement, type, oldProps, newProps, root);
},

shouldSetTextContent(props : Props) : boolean {
return (
typeof props.children === 'string' ||
typeof props.children === 'number' ||
(
typeof props.dangerouslySetInnerHTML === 'object' &&
props.dangerouslySetInnerHTML !== null &&
typeof props.dangerouslySetInnerHTML.__html === 'string'
)
);
},

resetTextContent(domElement : Instance) : void {
domElement.textContent = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're concerned with performance then IIRC setting textContent or innerHTML is unintuitively significantly slower than simply clearing the children one by one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only gets called when there's a single text child

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not true because dangerouslySetInnerHTML can have multiple children.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right that's true

},

createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance {
var textNode : TextInstance = document.createTextNode(text);
precacheFiberNode(internalInstanceHandle, textNode);
Expand Down
9 changes: 9 additions & 0 deletions src/renderers/noop/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ var NoopRenderer = ReactFiberReconciler({
instance.prop = newProps.prop;
},

shouldSetTextContent(props : Props) : boolean {
return (
typeof props.children === 'string' ||
typeof props.children === 'number'
);
},

resetTextContent(instance : Instance) : void {},

createTextInstance(text : string) : TextInstance {
var inst = { text : text, id: instanceCounter++ };
// Hide from unit tests
Expand Down
16 changes: 15 additions & 1 deletion src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ var {
} = require('ReactPriorityLevel');
var {
Placement,
ContentReset,
} = require('ReactTypeOfSideEffect');
var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactFiberClassComponent = require('ReactFiberClassComponent');
Expand Down Expand Up @@ -210,14 +211,27 @@ module.exports = function<T, P, I, TI, C>(
}

function updateHostComponent(current, workInProgress) {
const nextProps = workInProgress.pendingProps;
const prevProps = current ? current.memoizedProps : null;
let nextChildren = workInProgress.pendingProps.children;
if (typeof nextChildren === 'string' || typeof nextChildren === 'number') {
const isDirectTextChild = config.shouldSetTextContent(nextProps);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This should've been cached in a local variable above instead of using the object form.

if (isDirectTextChild) {
// We special case a direct text child of a host node. This is a common
// case. We won't handle it as a reified child. We will instead handle
// this in the host environment that also have access to this prop. That
// avoids allocating another HostText fiber and traversing it.
nextChildren = null;
} else if (prevProps && (
config.shouldSetTextContent(prevProps) ||
prevProps.children === null ||
typeof prevProps.children === 'undefined' ||
typeof prevProps.children === 'boolean'
)) {
// If we're switching from a direct text child to a normal child, or to
// empty, we need to schedule the text content to be reset.
workInProgress.effectTag |= ContentReset;
}

if (workInProgress.pendingProps.hidden &&
workInProgress.pendingWorkPriority !== OffscreenPriority) {
// If this host component is hidden, we can bail out on the children.
Expand Down
40 changes: 35 additions & 5 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var {
Placement,
Update,
Callback,
ContentReset,
} = require('ReactTypeOfSideEffect');

module.exports = function<T, P, I, TI, C>(
Expand All @@ -38,6 +39,7 @@ module.exports = function<T, P, I, TI, C>(
) {

const commitUpdate = config.commitUpdate;
const resetTextContent = config.resetTextContent;
const commitTextUpdate = config.commitTextUpdate;

const appendChild = config.appendChild;
Expand Down Expand Up @@ -83,6 +85,17 @@ module.exports = function<T, P, I, TI, C>(
throw new Error('Expected to find a host parent.');
}

function getHostParentFiber(fiber : Fiber) : Fiber {
let parent = fiber.return;
while (parent) {
if (isHostParent(parent)) {
return parent;
}
parent = parent.return;
}
throw new Error('Expected to find a host parent.');
}

function isHostParent(fiber : Fiber) : boolean {
return (
fiber.tag === HostComponent ||
Expand Down Expand Up @@ -133,12 +146,29 @@ module.exports = function<T, P, I, TI, C>(
}

function commitPlacement(finishedWork : Fiber) : void {
// Clear effect from effect tag before any errors can be thrown, so that
// we don't attempt to do this again
finishedWork.effectTag &= ~Placement;

// Recursively insert all host nodes into the parent.
const parent = getHostParent(finishedWork);
const parentFiber = getHostParentFiber(finishedWork);
let parent;
switch (parentFiber.tag) {
case HostComponent:
parent = parentFiber.stateNode;
break;
case HostContainer:
parent = parentFiber.stateNode.containerInfo;
break;
case Portal:
parent = parentFiber.stateNode.containerInfo;
break;
default:
throw new Error('Invalid host parent fiber.');
}
if (parentFiber.effectTag & ContentReset) {
// Reset the text content of the parent before doing any insertions
resetTextContent(parent);
// Clear ContentReset from the effect tag
parentFiber.effectTag &= ~ContentReset;
}

const before = getHostSibling(finishedWork);
// We only have the top Fiber that was inserted but we need recurse down its
// children to find all the terminal nodes.
Expand Down
3 changes: 3 additions & 0 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ export type HostConfig<T, P, I, TI, C> = {
prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean,
commitUpdate(instance : I, oldProps : P, newProps : P, internalInstanceHandle : OpaqueNode) : void,

shouldSetTextContent(props : P) : boolean,
resetTextContent(instance : I) : void,

createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI,
commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void,

Expand Down
7 changes: 6 additions & 1 deletion src/renderers/shared/fiber/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ var {
Update,
PlacementAndUpdate,
Deletion,
ContentReset,
Callback,
Err,
} = require('ReactTypeOfSideEffect');
Expand Down Expand Up @@ -168,11 +169,15 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
pass1: while (true) {
try {
while (effectfulFiber) {
if (effectfulFiber.effectTag & ContentReset) {
config.resetTextContent(effectfulFiber.stateNode);
}

// The following switch statement is only concerned about placement,
// updates, and deletions. To avoid needing to add a case for every
// possible bitmap value, we remove the secondary effects from the
// effect tag and switch on that value.
let primaryEffectTag = effectfulFiber.effectTag & ~(Callback | Err);
let primaryEffectTag = effectfulFiber.effectTag & ~(Callback | Err | ContentReset);
switch (primaryEffectTag) {
case Placement: {
commitPlacement(effectfulFiber);
Expand Down
17 changes: 9 additions & 8 deletions src/renderers/shared/fiber/ReactTypeOfSideEffect.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@

'use strict';

export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 16;
export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 16 | 32;

module.exports = {
NoEffect: 0, // 0b00000
Placement: 1, // 0b00001
Update: 2, // 0b00010
PlacementAndUpdate: 3, // 0b00011
Deletion: 4, // 0b00100
Callback: 8, // 0b01000
Err: 16, // 0b10000
NoEffect: 0, // 0b000000
Placement: 1, // 0b000001
Update: 2, // 0b000010
PlacementAndUpdate: 3, // 0b000011
Deletion: 4, // 0b000100
ContentReset: 8, // 0b001000
Callback: 16, // 0b010000
Err: 32, // 0b100000
};