maxjhandsome
/
react
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Diff properties in the commit phase instead of generating an update payload (#26583) 

This removes the concept of `prepareUpdate()`, behind a flag.

React Native already does everything in the commit phase, but generates
a temporary update payload before applying it.

React Fabric does it both in the render phase. Now it just moves it to a
single host config.

For DOM I forked updateProperties into one that does diffing and
updating in one pass vs just applying a pre-diffed updatePayload.

There are a few downsides of this approach:

- If only "children" has changed, we end up scheduling an update to be
done in the commit phase. Since we traverse through it anyway, it's
probably not much extra.
- It does more work in the commit phase so for a large tree that is
mostly unchanged, it'll stall longer.
- It does some extra work for special cases since that work happens if
anything has changed. We no longer have a deep bailout.
- The special cases now have to each replicate the "clean up old props"
loop, leading to extra code.

The benefit is that this doesn't allocate temporary extra objects
(possibly multiple per element if the array has to resize). It's less
work overall. It also gives us an option to reuse this function for a
sync render optimization.

Another benefit is that if we do the loop in the commit phase I can do
further optimizations by reading all props that I need for special cases
in that loop instead of polymorphic reads from props. This is what I'd
like to do in future refactors that would be stacked on top of this
change.
This commit is contained in:
Sebastian Markbåge 2023-04-10 19:09:28 -04:00 committed by GitHub
parent dd0619b2ee
commit ca41adb8c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 675 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
packages/react-dom-bindings/src/client/CSSPropertyOperations.js vendored
View File

@ -11,6 +11,7 @@ import hyphenateStyleName from '../shared/hyphenateStyleName';
import warnValidStyle from '../shared/warnValidStyle';
import isUnitlessNumber from '../shared/isUnitlessNumber';
import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';
import {diffInCommitPhase} from 'shared/ReactFeatureFlags';
/**
* Operations for dealing with CSS properties.
@ -64,6 +65,42 @@ export function createDangerousStringForStyles(styles) {
}
}
function setValueForStyle(style, styleName, value) {
const isCustomProperty = styleName.indexOf('--') === 0;
if (__DEV__) {
if (!isCustomProperty) {
warnValidStyle(styleName, value);
}
}
if (value == null || typeof value === 'boolean' || value === '') {
if (isCustomProperty) {
style.setProperty(styleName, '');
} else if (styleName === 'float') {
style.cssFloat = '';
} else {
style[styleName] = '';
}
} else if (isCustomProperty) {
style.setProperty(styleName, value);
} else if (
typeof value === 'number' &&
value !== 0 &&
!isUnitlessNumber(styleName)
) {
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
} else {
if (styleName === 'float') {
style.cssFloat = value;
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(value, styleName);
}
style[styleName] = ('' + value).trim();
}
}
}
/**
* Sets the value for multiple styles on a node. If a value is specified as
* '' (empty string), the corresponding style property will be unset.
@ -71,7 +108,7 @@ export function createDangerousStringForStyles(styles) {
* @param {DOMElement} node
* @param {object} styles
*/
export function setValueForStyles(node, styles) {
export function setValueForStyles(node, styles, prevStyles) {
if (styles != null && typeof styles !== 'object') {
throw new Error(
'The `style` prop expects a mapping from style properties to values, ' +
@ -88,42 +125,39 @@ export function setValueForStyles(node, styles) {
}
const style = node.style;
for (const styleName in styles) {
if (!styles.hasOwnProperty(styleName)) {
continue;
}
const value = styles[styleName];
const isCustomProperty = styleName.indexOf('--') === 0;
if (diffInCommitPhase && prevStyles != null) {
if (__DEV__) {
if (!isCustomProperty) {
warnValidStyle(styleName, value);
}
validateShorthandPropertyCollisionInDev(prevStyles, styles);
}
if (value == null || typeof value === 'boolean' || value === '') {
if (isCustomProperty) {
style.setProperty(styleName, '');
} else if (styleName === 'float') {
style.cssFloat = '';
} else {
style[styleName] = '';
}
} else if (isCustomProperty) {
style.setProperty(styleName, value);
} else if (
typeof value === 'number' &&
value !== 0 &&
!isUnitlessNumber(styleName)
) {
style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
} else {
if (styleName === 'float') {
style.cssFloat = value;
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(value, styleName);
for (const styleName in prevStyles) {
if (
prevStyles.hasOwnProperty(styleName) &&
(styles == null || !styles.hasOwnProperty(styleName))
) {
// Clear style
const isCustomProperty = styleName.indexOf('--') === 0;
if (isCustomProperty) {
style.setProperty(styleName, '');
} else if (styleName === 'float') {
style.cssFloat = '';
} else {
style[styleName] = '';
}
style[styleName] = ('' + value).trim();
}
}
for (const styleName in styles) {
const value = styles[styleName];
if (styles.hasOwnProperty(styleName) && prevStyles[styleName] !== value) {
setValueForStyle(style, styleName, value);
}
}
} else {
for (const styleName in styles) {
if (styles.hasOwnProperty(styleName)) {
const value = styles[styleName];
setValueForStyle(style, styleName, value);
}
}
}
@ -167,7 +201,7 @@ function expandShorthandMap(styles) {
* becomes .style.fontVariant = ''
*/
export function validateShorthandPropertyCollisionInDev(
styleUpdates,
prevStyles,
nextStyles,
) {
if (__DEV__) {
@ -175,7 +209,30 @@ export function validateShorthandPropertyCollisionInDev(
return;
}
const expandedUpdates = expandShorthandMap(styleUpdates);
// Compute the diff as it would happen elsewhere.
const expandedUpdates = {};
if (prevStyles) {
for (const key in prevStyles) {
if (prevStyles.hasOwnProperty(key) && !nextStyles.hasOwnProperty(key)) {
const longhands = shorthandToLonghand[key] || [key];
for (let i = 0; i < longhands.length; i++) {
expandedUpdates[longhands[i]] = key;
}
}
}
}
for (const key in nextStyles) {
if (
nextStyles.hasOwnProperty(key) &&
(!prevStyles || prevStyles[key] !== nextStyles[key])
) {
const longhands = shorthandToLonghand[key] || [key];
for (let i = 0; i < longhands.length; i++) {
expandedUpdates[longhands[i]] = key;
}
}
}
const expandedStyles = expandShorthandMap(nextStyles);
const warnedAbout = {};
for (const key in expandedUpdates) {
@ -193,7 +250,7 @@ export function validateShorthandPropertyCollisionInDev(
"avoid this, don't mix shorthand and non-shorthand properties " +
'for the same value; instead, replace the shorthand with ' +
'separate values.',
isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
isValueEmpty(nextStyles[originalKey]) ? 'Removing' : 'Updating',
originalKey,
correctOriginalKey,
);

482
packages/react-dom-bindings/src/client/ReactDOMComponent.js vendored
View File

@ -69,6 +69,7 @@ import {
disableIEWorkarounds,
enableTrustedTypesIntegration,
enableFilterEmptyStringAttributesDOM,
diffInCommitPhase,
} from 'shared/ReactFeatureFlags';
import {
mediaEventTypes,
@ -273,6 +274,7 @@ function setProp(
key: string,
value: mixed,
props: any,
prevValue: mixed,
): void {
switch (key) {
case 'children': {
@ -314,7 +316,7 @@ function setProp(
break;
}
case 'style': {
setValueForStyles(domElement, value);
setValueForStyles(domElement, value, prevValue);
break;
}
// These attributes accept URLs. These must not allow javascript: URLS.
@ -655,6 +657,21 @@ function setProp(
);
break;
// Properties that should not be allowed on custom elements.
case 'is': {
if (__DEV__) {
if (prevValue != null) {
console.error(
'Cannot update the "is" prop after it has been initialized.',
);
}
}
// TODO: We shouldn't actually set this attribute, because we've already
// passed it to createElement. We don't also need the attribute.
// However, our tests currently query for it so it's plausible someone
// else does too so it's break.
setValueForAttribute(domElement, 'is', value);
break;
}
case 'innerText':
case 'textContent':
if (enableCustomElementPropertySupport) {
@ -689,10 +706,11 @@ function setPropOnCustomElement(
key: string,
value: mixed,
props: any,
prevValue: mixed,
): void {
switch (key) {
case 'style': {
setValueForStyles(domElement, value);
setValueForStyles(domElement, value, prevValue);
break;
}
case 'dangerouslySetInnerHTML': {
@ -859,7 +877,7 @@ export function setInitialProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
@ -892,7 +910,7 @@ export function setInitialProperties(
}
// defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
@ -1047,7 +1065,7 @@ export function setInitialProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
@ -1063,7 +1081,14 @@ export function setInitialProperties(
if (propValue == null) {
continue;
}
setPropOnCustomElement(domElement, tag, propKey, propValue, props);
setPropOnCustomElement(
domElement,
tag,
propKey,
propValue,
props,
null,
);
}
return;
}
@ -1078,7 +1103,7 @@ export function setInitialProperties(
if (propValue == null) {
continue;
}
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
@ -1188,15 +1213,428 @@ export function diffProperties(
}
if (styleUpdates) {
if (__DEV__) {
validateShorthandPropertyCollisionInDev(styleUpdates, nextProps.style);
validateShorthandPropertyCollisionInDev(lastProps.style, nextProps.style);
}
(updatePayload = updatePayload || []).push('style', styleUpdates);
}
return updatePayload;
}
// Apply the diff.
export function updateProperties(
domElement: Element,
tag: string,
lastProps: Object,
nextProps: Object,
): void {
if (__DEV__) {
validatePropertiesInDevelopment(tag, nextProps);
}
switch (tag) {
case 'div':
case 'span':
case 'svg':
case 'path':
case 'a':
case 'g':
case 'p':
case 'li': {
// Fast track the most common tag types
break;
}
case 'input': {
// Update checked *before* name.
// In the middle of an update, it is possible to have multiple checked.
// When a checked radio tries to change name, browser makes another radio's checked false.
if (nextProps.type === 'radio' && nextProps.name != null) {
updateInputChecked(domElement, nextProps);
}
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
switch (propKey) {
case 'checked': {
const checked = nextProps.defaultChecked;
const inputElement: HTMLInputElement = (domElement: any);
inputElement.checked =
!!checked &&
typeof checked !== 'function' &&
checked !== 'symbol';
break;
}
case 'value': {
// This is handled by updateWrapper below.
break;
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
switch (propKey) {
case 'checked': {
const checked =
nextProp != null ? nextProp : nextProps.defaultChecked;
const inputElement: HTMLInputElement = (domElement: any);
inputElement.checked =
!!checked &&
typeof checked !== 'function' &&
checked !== 'symbol';
break;
}
case 'value': {
// This is handled by updateWrapper below.
break;
}
case 'children':
case 'dangerouslySetInnerHTML': {
if (nextProp != null) {
throw new Error(
`${tag} is a void element tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
}
break;
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
}
if (__DEV__) {
const wasControlled =
lastProps.type === 'checkbox' || lastProps.type === 'radio'
? lastProps.checked != null
: lastProps.value != null;
const isControlled =
nextProps.type === 'checkbox' || nextProps.type === 'radio'
? nextProps.checked != null
: nextProps.value != null;
if (
!wasControlled &&
isControlled &&
!didWarnUncontrolledToControlled
) {
console.error(
'A component is changing an uncontrolled input to be controlled. ' +
'This is likely caused by the value changing from undefined to ' +
'a defined value, which should not happen. ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components',
);
didWarnUncontrolledToControlled = true;
}
if (
wasControlled &&
!isControlled &&
!didWarnControlledToUncontrolled
) {
console.error(
'A component is changing a controlled input to be uncontrolled. ' +
'This is likely caused by the value changing from a defined to ' +
'undefined, which should not happen. ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components',
);
didWarnControlledToUncontrolled = true;
}
}
// Update the wrapper around inputs *after* updating props. This has to
// happen after updating the rest of props. Otherwise HTML5 input validations
// raise warnings and prevent the new value from being assigned.
updateInput(domElement, nextProps);
return;
}
case 'select': {
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
switch (propKey) {
case 'value': {
// This is handled by updateWrapper below.
break;
}
// defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
switch (propKey) {
case 'value': {
// This is handled by updateWrapper below.
break;
}
// defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
}
// <select> value update needs to occur after <option> children
// reconciliation
updateSelect(domElement, lastProps, nextProps);
return;
}
case 'textarea': {
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
switch (propKey) {
case 'value': {
// This is handled by updateWrapper below.
break;
}
case 'children': {
// TODO: This doesn't actually do anything if it updates.
break;
}
// defaultValue is ignored by setProp
default: {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
switch (propKey) {
case 'value': {
// This is handled by updateWrapper below.
break;
}
case 'children': {
// TODO: This doesn't actually do anything if it updates.
break;
}
case 'dangerouslySetInnerHTML': {
if (nextProp != null) {
// TODO: Do we really need a special error message for this. It's also pretty blunt.
throw new Error(
'`dangerouslySetInnerHTML` does not make sense on <textarea>.',
);
}
break;
}
// defaultValue is ignored by setProp
default: {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
}
updateTextarea(domElement, nextProps);
return;
}
case 'option': {
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
switch (propKey) {
case 'selected': {
// TODO: Remove support for selected on option.
(domElement: any).selected = false;
break;
}
default: {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
switch (propKey) {
case 'selected': {
// TODO: Remove support for selected on option.
(domElement: any).selected =
nextProp &&
typeof nextProp !== 'function' &&
typeof nextProp !== 'symbol';
break;
}
default: {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
}
return;
}
case 'img':
case 'link':
case 'area':
case 'base':
case 'br':
case 'col':
case 'embed':
case 'hr':
case 'keygen':
case 'meta':
case 'param':
case 'source':
case 'track':
case 'wbr':
case 'menuitem': {
// Void elements
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
switch (propKey) {
case 'children':
case 'dangerouslySetInnerHTML': {
if (nextProp != null) {
// TODO: Can we make this a DEV warning to avoid this deny list?
throw new Error(
`${tag} is a void element tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
}
break;
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
}
return;
}
default: {
if (isCustomElement(tag, nextProps)) {
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
setPropOnCustomElement(
domElement,
tag,
propKey,
null,
nextProps,
lastProp,
);
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
setPropOnCustomElement(
domElement,
tag,
propKey,
nextProp,
nextProps,
lastProp,
);
}
}
return;
}
}
}
for (const propKey in lastProps) {
const lastProp = lastProps[propKey];
if (
lastProps.hasOwnProperty(propKey) &&
lastProp != null &&
!nextProps.hasOwnProperty(propKey)
) {
setProp(domElement, tag, propKey, null, nextProps, lastProp);
}
}
for (const propKey in nextProps) {
const nextProp = nextProps[propKey];
const lastProp = lastProps[propKey];
if (
nextProps.hasOwnProperty(propKey) &&
nextProp !== lastProp &&
(nextProp != null || lastProp != null)
) {
setProp(domElement, tag, propKey, nextProp, nextProps, lastProp);
}
}
}
// Apply the diff.
export function updatePropertiesWithDiff(
domElement: Element,
updatePayload: Array<any>,
tag: string,
@ -1252,7 +1690,7 @@ export function updateProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
}
@ -1313,7 +1751,7 @@ export function updateProperties(
}
// defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
}
@ -1346,7 +1784,7 @@ export function updateProperties(
}
// defaultValue is ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
}
@ -1367,7 +1805,7 @@ export function updateProperties(
break;
}
default: {
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
}
@ -1406,7 +1844,7 @@ export function updateProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
}
@ -1423,6 +1861,7 @@ export function updateProperties(
propKey,
propValue,
nextProps,
null,
);
}
return;
@ -1434,7 +1873,7 @@ export function updateProperties(
for (let i = 0; i < updatePayload.length; i += 2) {
const propKey = updatePayload[i];
const propValue = updatePayload[i + 1];
setProp(domElement, tag, propKey, propValue, nextProps);
setProp(domElement, tag, propKey, propValue, nextProps, null);
}
}
@ -2377,7 +2816,18 @@ export function diffHydratedProperties(
);
}
if (!isConcurrentMode || !enableClientRenderFallbackOnTextMismatch) {
updatePayload = ['children', children];
if (diffInCommitPhase) {
// We really should be patching this in the commit phase but since
// this only affects legacy mode hydration which is deprecated anyway
// we can get away with it.
// Host singletons get their children appended and don't use the text
// content mechanism.
if (!enableHostSingletons || tag !== 'body') {
domElement.textContent = (children: any);
}
} else {
updatePayload = ['children', children];
}
}
}
}

24
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js vendored
View File

@ -47,6 +47,7 @@ import {
setInitialProperties,
diffProperties,
updateProperties,
updatePropertiesWithDiff,
diffHydratedProperties,
diffHydratedText,
trapClickOnNonInteractiveElement,
@ -86,6 +87,7 @@ import {
enableFloat,
enableHostSingletons,
enableTrustedTypesIntegration,
diffInCommitPhase,
} from 'shared/ReactFeatureFlags';
import {
HostComponent,
@ -485,6 +487,10 @@ export function prepareUpdate(
newProps: Props,
hostContext: HostContext,
): null | Array<mixed> {
if (diffInCommitPhase) {
// TODO: Figure out how to validateDOMNesting when children turn into a string.
return null;
}
if (__DEV__) {
const hostContextDev = ((hostContext: any): HostContextDev);
if (
@ -642,14 +648,26 @@ export function commitMount(
export function commitUpdate(
domElement: Instance,
updatePayload: Array<mixed>,
updatePayload: any,
type: string,
oldProps: Props,
newProps: Props,
internalInstanceHandle: Object,
): void {
// Apply the diff to the DOM node.
updateProperties(domElement, updatePayload, type, oldProps, newProps);
if (diffInCommitPhase) {
// Diff and update the properties.
updateProperties(domElement, type, oldProps, newProps);
} else {
// Apply the diff to the DOM node.
updatePropertiesWithDiff(
domElement,
updatePayload,
type,
oldProps,
newProps,
);
}
// Update the props handle so that we know which props are the ones with
// with current event handlers.
updateFiberProps(domElement, newProps);

21
packages/react-native-renderer/src/ReactFiberConfigFabric.js vendored
View File

@ -42,6 +42,8 @@ const {
unstable_getCurrentEventPriority: fabricGetCurrentEventPriority,
} = nativeFabricUIManager;
import {diffInCommitPhase} from 'shared/ReactFeatureFlags';
const {get: getViewConfigForType} = ReactNativeViewConfigRegistry;
// Counter for uniquely identifying views.
@ -288,6 +290,9 @@ export function prepareUpdate(
newProps: Props,
hostContext: HostContext,
): null | Object {
if (diffInCommitPhase) {
return null;
}
const viewConfig = instance.canonical.viewConfig;
const updatePayload = diff(oldProps, newProps, viewConfig.validAttributes);
// TODO: If the event handlers have changed, we need to update the current props
@ -355,13 +360,27 @@ export function cloneInstance(
keepChildren: boolean,
recyclableInstance: null | Instance,
): Instance {
if (diffInCommitPhase) {
const viewConfig = instance.canonical.viewConfig;
updatePayload = diff(oldProps, newProps, viewConfig.validAttributes);
// TODO: If the event handlers have changed, we need to update the current props
// in the commit phase but there is no host config hook to do it yet.
// So instead we hack it by updating it in the render phase.
instance.canonical.currentProps = newProps;
}
const node = instance.node;
let clone;
if (keepChildren) {
if (updatePayload !== null) {
clone = cloneNodeWithNewProps(node, updatePayload);
} else {
clone = cloneNode(node);
if (diffInCommitPhase) {
// No changes
return instance;
} else {
clone = cloneNode(node);
}
}
} else {
if (updatePayload !== null) {

11
packages/react-noop-renderer/src/createReactNoop.js vendored
View File

@ -88,7 +88,6 @@ if (__DEV__) {
function createReactNoop(reconciler: Function, useMutation: boolean) {
let instanceCounter = 0;
let hostDiffCounter = 0;
let hostUpdateCounter = 0;
let hostCloneCounter = 0;
@ -458,16 +457,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
oldProps: Props,
newProps: Props,
): null | {...} {
if (type === 'errorInCompletePhase') {
throw new Error('Error in host config.');
}
if (oldProps === null) {
throw new Error('Should have old props');
}
if (newProps === null) {
throw new Error('Should have new props');
}
hostDiffCounter++;
return UPDATE_SIGNAL;
},
@ -1186,30 +1181,24 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
},
startTrackingHostCounters(): void {
hostDiffCounter = 0;
hostUpdateCounter = 0;
hostCloneCounter = 0;
},
stopTrackingHostCounters():
| {
hostDiffCounter: number,
hostUpdateCounter: number,
}
| {
hostDiffCounter: number,
hostCloneCounter: number,
} {
const result = useMutation
? {
hostDiffCounter,
hostUpdateCounter,
}
: {
hostDiffCounter,
hostCloneCounter,
};
hostDiffCounter = 0;
hostUpdateCounter = 0;
hostCloneCounter = 0;

3
packages/react-reconciler/src/ReactFiberCommitWork.js vendored
View File

@ -54,6 +54,7 @@ import {
enableFloat,
enableLegacyHidden,
enableHostSingletons,
diffInCommitPhase,
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
@ -2774,7 +2775,7 @@ function commitMutationEffectsOnFiber(
const updatePayload: null | UpdatePayload =
(finishedWork.updateQueue: any);
finishedWork.updateQueue = null;
if (updatePayload !== null) {
if (updatePayload !== null || diffInCommitPhase) {
try {
commitUpdate(
instance,

84
packages/react-reconciler/src/ReactFiberCompleteWork.js vendored
View File

@ -38,6 +38,7 @@ import {
enableCache,
enableTransitionTracing,
enableFloat,
diffInCommitPhase,
} from 'shared/ReactFeatureFlags';
import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource';
@ -432,29 +433,32 @@ function updateHostComponent(
return;
}
// If we get updated because one of our children updated, we don't
// have newProps so we'll have to reuse them.
// TODO: Split the update API as separate for the props vs. children.
// Even better would be if children weren't special cased at all tho.
const instance: Instance = workInProgress.stateNode;
const currentHostContext = getHostContext();
// TODO: Experiencing an error where oldProps is null. Suggests a host
// component is hitting the resume path. Figure out why. Possibly
// related to `hidden`.
const updatePayload = prepareUpdate(
instance,
type,
oldProps,
newProps,
currentHostContext,
);
// TODO: Type this specific to this type of component.
workInProgress.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
// is a new ref we mark this as an update. All the work is done in commitWork.
if (updatePayload) {
if (diffInCommitPhase) {
markUpdate(workInProgress);
} else {
// If we get updated because one of our children updated, we don't
// have newProps so we'll have to reuse them.
// TODO: Split the update API as separate for the props vs. children.
// Even better would be if children weren't special cased at all tho.
const instance: Instance = workInProgress.stateNode;
// TODO: Experiencing an error where oldProps is null. Suggests a host
// component is hitting the resume path. Figure out why. Possibly
// related to `hidden`.
const currentHostContext = getHostContext();
const updatePayload = prepareUpdate(
instance,
type,
oldProps,
newProps,
currentHostContext,
);
// TODO: Type this specific to this type of component.
workInProgress.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
// is a new ref we mark this as an update. All the work is done in commitWork.
if (updatePayload) {
markUpdate(workInProgress);
}
}
} else if (supportsPersistence) {
const currentInstance = current.stateNode;
@ -471,20 +475,22 @@ function updateHostComponent(
const recyclableInstance: Instance = workInProgress.stateNode;
const currentHostContext = getHostContext();
let updatePayload = null;
if (oldProps !== newProps) {
updatePayload = prepareUpdate(
recyclableInstance,
type,
oldProps,
newProps,
currentHostContext,
);
}
if (childrenUnchanged && updatePayload === null) {
// No changes, just reuse the existing instance.
// Note that this might release a previous clone.
workInProgress.stateNode = currentInstance;
return;
if (!diffInCommitPhase) {
if (oldProps !== newProps) {
updatePayload = prepareUpdate(
recyclableInstance,
type,
oldProps,
newProps,
currentHostContext,
);
}
if (childrenUnchanged && updatePayload === null) {
// No changes, just reuse the existing instance.
// Note that this might release a previous clone.
workInProgress.stateNode = currentInstance;
return;
}
}
const newInstance = cloneInstance(
currentInstance,
@ -496,6 +502,12 @@ function updateHostComponent(
childrenUnchanged,
recyclableInstance,
);
if (diffInCommitPhase && newInstance === currentInstance) {
// No changes, just reuse the existing instance.
// Note that this might release a previous clone.
workInProgress.stateNode = currentInstance;
return;
}
if (
finalizeInitialChildren(newInstance, type, newProps, currentHostContext)

14
packages/react-reconciler/src/ReactFiberHydrationContext.js vendored
View File

@ -39,6 +39,7 @@ import {
enableHostSingletons,
enableFloat,
enableClientRenderFallbackOnTextMismatch,
diffInCommitPhase,
} from 'shared/ReactFeatureFlags';
import {
@ -687,12 +688,15 @@ function prepareToHydrateHostInstance(
fiber,
shouldWarnIfMismatchDev,
);
// TODO: Type this specific to this type of component.
fiber.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
// is a new ref we mark this as an update.
if (updatePayload !== null) {
return true;
if (!diffInCommitPhase) {
fiber.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
// is a new ref we mark this as an update.
if (updatePayload !== null) {
return true;
}
}
return false;
}

10
packages/react-reconciler/src/__tests__/ReactIncrementalUpdatesMinimalism-test.js vendored
View File

@ -35,14 +35,12 @@ describe('ReactIncrementalUpdatesMinimalism', () => {
ReactNoop.startTrackingHostCounters();
await act(() => ReactNoop.render(<Parent />));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostUpdateCounter: 0,
});
ReactNoop.startTrackingHostCounters();
await act(() => ReactNoop.render(<Parent />));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 1,
hostUpdateCounter: 1,
});
});
@ -75,14 +73,12 @@ describe('ReactIncrementalUpdatesMinimalism', () => {
ReactNoop.startTrackingHostCounters();
await act(() => ReactNoop.render(<Parent />));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostUpdateCounter: 0,
});
ReactNoop.startTrackingHostCounters();
await act(() => ReactNoop.render(<Parent />));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostUpdateCounter: 0,
});
});
@ -128,17 +124,12 @@ describe('ReactIncrementalUpdatesMinimalism', () => {
ReactNoop.startTrackingHostCounters();
await act(() => ReactNoop.render(<Parent />));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostUpdateCounter: 0,
});
ReactNoop.startTrackingHostCounters();
await act(() => childInst.setState({name: 'Robin'}));
expect(ReactNoop.stopTrackingHostCounters()).toEqual({
// Child > div
// Child > Leaf > span
// Child > Leaf > span > b
hostDiffCounter: 3,
// Child > div
// Child > Leaf > span
// Child > Leaf > span > b
@ -159,7 +150,6 @@ describe('ReactIncrementalUpdatesMinimalism', () => {
// Parent > section > div > hr
// Parent > section > div > Leaf > span
// Parent > section > div > Leaf > span > b
hostDiffCounter: 10,
hostUpdateCounter: 10,
});
});

10
packages/react-reconciler/src/__tests__/ReactPersistentUpdatesMinimalism-test.js vendored
View File

@ -34,14 +34,12 @@ describe('ReactPersistentUpdatesMinimalism', () => {
ReactNoopPersistent.startTrackingHostCounters();
await act(() => ReactNoopPersistent.render(<Parent />));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostCloneCounter: 0,
});
ReactNoopPersistent.startTrackingHostCounters();
await act(() => ReactNoopPersistent.render(<Parent />));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 1,
hostCloneCounter: 1,
});
});
@ -74,14 +72,12 @@ describe('ReactPersistentUpdatesMinimalism', () => {
ReactNoopPersistent.startTrackingHostCounters();
await act(() => ReactNoopPersistent.render(<Parent />));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostCloneCounter: 0,
});
ReactNoopPersistent.startTrackingHostCounters();
await act(() => ReactNoopPersistent.render(<Parent />));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostCloneCounter: 0,
});
});
@ -127,17 +123,12 @@ describe('ReactPersistentUpdatesMinimalism', () => {
ReactNoopPersistent.startTrackingHostCounters();
await act(() => ReactNoopPersistent.render(<Parent />));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
hostDiffCounter: 0,
hostCloneCounter: 0,
});
ReactNoopPersistent.startTrackingHostCounters();
await act(() => childInst.setState({name: 'Robin'}));
expect(ReactNoopPersistent.stopTrackingHostCounters()).toEqual({
// section > div > Child > div
// section > div > Child > Leaf > span
// section > div > Child > Leaf > span > b
hostDiffCounter: 3,
// section
// section > div
// section > div > Child > div
@ -159,7 +150,6 @@ describe('ReactPersistentUpdatesMinimalism', () => {
// Parent > section > div > hr
// Parent > section > div > Leaf > span
// Parent > section > div > Leaf > span > b
hostDiffCounter: 10,
hostCloneCounter: 10,
});
});

2
packages/react-test-renderer/src/ReactFiberConfigTestHost.js vendored
View File

@ -230,7 +230,7 @@ export const supportsMutation = true;
export function commitUpdate(
instance: Instance,
updatePayload: {...},
updatePayload: null | {...},
type: string,
oldProps: Props,
newProps: Props,

3
packages/shared/ReactFeatureFlags.js
View File

@ -122,6 +122,9 @@ export const enableUseEffectEventHook = __EXPERIMENTAL__;
// (handled with an MutationObserver) instead of inline-scripts
export const enableFizzExternalRuntime = true;
// Performance related test
export const diffInCommitPhase = __EXPERIMENTAL__;
// -----------------------------------------------------------------------------
// Chopping Block
//

2
packages/shared/forks/ReactFeatureFlags.native-fb.js
View File

@ -82,5 +82,7 @@ export const enableHostSingletons = true;
export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const diffInCommitPhase = true;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

2
packages/shared/forks/ReactFeatureFlags.native-oss.js
View File

@ -72,5 +72,7 @@ export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;
export const diffInCommitPhase = true;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

2
packages/shared/forks/ReactFeatureFlags.test-renderer.js
View File

@ -72,5 +72,7 @@ export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;
export const diffInCommitPhase = true;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

2
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
View File

@ -69,5 +69,7 @@ export const enableHostSingletons = true;
export const useModernStrictMode = false;
export const enableDeferRootSchedulingToMicrotask = true;
export const diffInCommitPhase = true;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

2
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View File

@ -74,5 +74,7 @@ export const useModernStrictMode = false;
export const enableFizzExternalRuntime = false;
export const enableDeferRootSchedulingToMicrotask = true;
export const diffInCommitPhase = true;
// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

1
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
View File

@ -25,6 +25,7 @@ export const enableUnifiedSyncLane = __VARIANT__;
export const enableTransitionTracing = __VARIANT__;
export const enableCustomElementPropertySupport = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
export const diffInCommitPhase = __VARIANT__;
// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.

1
packages/shared/forks/ReactFeatureFlags.www.js
View File

@ -29,6 +29,7 @@ export const {
enableTransitionTracing,
enableCustomElementPropertySupport,
enableDeferRootSchedulingToMicrotask,
diffInCommitPhase,
} = dynamicFeatureFlags;
// On WWW, __EXPERIMENTAL__ is used for a new modern build.