diff --git a/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload.js b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload.js index 5d9388e3a07..567ac6fab6e 100644 --- a/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload.js +++ b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/ReactNativeAttributePayload.js @@ -8,7 +8,7 @@ * @format */ -import type {AttributeConfiguration} from '../../Renderer/shims/ReactNativeTypes'; +import type {PartialAttributeConfiguration as AttributeConfiguration} from '../../Renderer/shims/ReactNativeTypes'; import flattenStyle from '../../StyleSheet/flattenStyle'; import deepDiffer from '../../Utilities/differ/deepDiffer'; @@ -61,7 +61,7 @@ function restoreDeletedValuesInNestedArray( } else if (node && removedKeyCount > 0) { const obj = node; for (const propKey in removedKeys) { - // $FlowFixMe[incompatible-use] found when upgrading Flow + // $FlowFixMe[incompatible-use] removedKeys is always non-null if (!removedKeys[propKey]) { continue; } @@ -133,12 +133,12 @@ function diffNestedArrayProperty( ); } for (; i < nextArray.length; i++) { - // Add all remaining properties. - updatePayload = addNestedProperty( - updatePayload, - nextArray[i], - validAttributes, - ); + // Add all remaining properties + const nextProp = nextArray[i]; + if (!nextProp) { + continue; + } + updatePayload = addNestedProperty(updatePayload, nextProp, validAttributes); } return updatePayload; } @@ -183,9 +183,7 @@ function diffNestedProperty( if (Array.isArray(prevProp)) { return diffProperties( updatePayload, - // $FlowFixMe - We know that this is always an object when the input is. flattenStyle(prevProp), - // $FlowFixMe - We know that this isn't an array because of above flow. nextProp, validAttributes, ); @@ -194,43 +192,11 @@ function diffNestedProperty( return diffProperties( updatePayload, prevProp, - // $FlowFixMe - We know that this is always an object when the input is. flattenStyle(nextProp), validAttributes, ); } -/** - * addNestedProperty takes a single set of props and valid attribute - * attribute configurations. It processes each prop and adds it to the - * updatePayload. - */ -function addNestedProperty( - updatePayload: null | Object, - nextProp: NestedNode, - validAttributes: AttributeConfiguration, -): $FlowFixMe { - if (!nextProp) { - return updatePayload; - } - - if (!Array.isArray(nextProp)) { - // Add each property of the leaf. - return addProperties(updatePayload, nextProp, validAttributes); - } - - for (let i = 0; i < nextProp.length; i++) { - // Add all the properties of the array. - updatePayload = addNestedProperty( - updatePayload, - nextProp[i], - validAttributes, - ); - } - - return updatePayload; -} - /** * clearNestedProperty takes a single set of props and valid attributes. It * adds a null sentinel to the updatePayload, for each prop key. @@ -285,14 +251,19 @@ function diffProperties( prevProp = prevProps[propKey]; nextProp = nextProps[propKey]; - // functions are converted to booleans as markers that the associated - // events should be sent from native. if (typeof nextProp === 'function') { - nextProp = (true: any); - // If nextProp is not a function, then don't bother changing prevProp - // since nextProp will win and go into the updatePayload regardless. - if (typeof prevProp === 'function') { - prevProp = (true: any); + const attributeConfigHasProcess = + typeof attributeConfig === 'object' && + typeof attributeConfig.process === 'function'; + if (!attributeConfigHasProcess) { + // functions are converted to booleans as markers that the associated + // events should be sent from native. + nextProp = (true: any); + // If nextProp is not a function, then don't bother changing prevProp + // since nextProp will win and go into the updatePayload regardless. + if (typeof prevProp === 'function') { + prevProp = (true: any); + } } } @@ -442,16 +413,69 @@ function diffProperties( return updatePayload; } -/** - * addProperties adds all the valid props to the payload after being processed. - */ -function addProperties( - updatePayload: null | Object, +function addNestedProperty( + payload: null | Object, props: Object, validAttributes: AttributeConfiguration, ): null | Object { - // TODO: Fast path - return diffProperties(updatePayload, emptyObject, props, validAttributes); + // Flatten nested style props. + if (Array.isArray(props)) { + for (let i = 0; i < props.length; i++) { + payload = addNestedProperty(payload, props[i], validAttributes); + } + return payload; + } + + for (const propKey in props) { + const prop = props[propKey]; + + const attributeConfig = ((validAttributes[ + propKey + ]: any): AttributeConfiguration); + + if (attributeConfig == null) { + continue; + } + + let newValue; + + if (prop === undefined) { + // Discard the prop if it was previously defined. + if (payload && payload[propKey] !== undefined) { + newValue = null; + } else { + continue; + } + } else if (typeof attributeConfig === 'object') { + if (typeof attributeConfig.process === 'function') { + // An atomic prop with custom processing. + newValue = attributeConfig.process(prop); + } else if (typeof attributeConfig.diff === 'function') { + // An atomic prop with custom diffing. We don't need to do diffing when adding props. + newValue = prop; + } + } else { + if (typeof prop === 'function') { + // A function prop. It represents an event handler. Pass it to native as 'true'. + newValue = true; + } else { + // An atomic prop. Doesn't need to be flattened. + newValue = prop; + } + } + + if (newValue !== undefined) { + if (!payload) { + payload = ({}: {[string]: $FlowFixMe}); + } + payload[propKey] = newValue; + continue; + } + + payload = addNestedProperty(payload, prop, attributeConfig); + } + + return payload; } /** @@ -463,7 +487,6 @@ function clearProperties( prevProps: Object, validAttributes: AttributeConfiguration, ): null | Object { - // TODO: Fast path return diffProperties(updatePayload, prevProps, emptyObject, validAttributes); } @@ -471,11 +494,7 @@ export function create( props: Object, validAttributes: AttributeConfiguration, ): null | Object { - return addProperties( - null, // updatePayload - props, - validAttributes, - ); + return addNestedProperty(null, props, validAttributes); } export function diff( diff --git a/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/__tests__/ReactNativeAttributePayload-test.js b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/__tests__/ReactNativeAttributePayload-test.js new file mode 100644 index 00000000000..a09ac112351 --- /dev/null +++ b/packages/react-native/Libraries/ReactNative/ReactFabricPublicInstance/__tests__/ReactNativeAttributePayload-test.js @@ -0,0 +1,495 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + * @format + */ + +'use strict'; + +import type {PartialAttributeConfiguration as AttributeConfiguration} from '../../../Renderer/shims/ReactNativeTypes'; + +const {create, diff} = require('../ReactNativeAttributePayload'); + +describe('ReactNativeAttributePayload.create', () => { + it('should work with simple example', () => { + expect(create({b: 2, c: 3}, {a: true, b: true})).toEqual({ + b: 2, + }); + }); + + it('should work with complex example', () => { + const validAttributes: AttributeConfiguration = { + style: { + position: true, + zIndex: true, + flexGrow: true, + flexShrink: true, + flexDirection: true, + overflow: true, + backgroundColor: true, + }, + }; + + expect( + create( + { + style: [ + { + flexGrow: 1, + flexShrink: 1, + flexDirection: 'row', + overflow: 'scroll', + }, + [ + {position: 'relative', zIndex: 2}, + {flexGrow: 0}, + {backgroundColor: 'red'}, + ], + ], + }, + validAttributes, + ), + ).toEqual({ + flexGrow: 0, + flexShrink: 1, + flexDirection: 'row', + overflow: 'scroll', + position: 'relative', + zIndex: 2, + backgroundColor: 'red', + }); + }); + + it('should nullify previously defined style prop that is subsequently set to null or undefined', () => { + expect( + create({style: [{a: 0}, {a: undefined}]}, {style: {a: true}}), + ).toEqual({a: null}); + expect(create({style: [{a: 0}, {a: null}]}, {style: {a: true}})).toEqual({ + a: null, + }); + }); + + it('should ignore non-style fields that are set to undefined', () => { + expect(create({}, {a: true})).toEqual(null); + expect(create({a: undefined}, {a: true})).toEqual(null); + expect(create({a: undefined, b: undefined}, {a: true, b: true})).toEqual( + null, + ); + expect( + create({a: undefined, b: undefined, c: 1}, {a: true, b: true}), + ).toEqual(null); + expect( + create({a: undefined, b: undefined, c: 1}, {a: true, b: true, c: true}), + ).toEqual({c: 1}); + expect( + create({a: 1, b: undefined, c: 2}, {a: true, b: true, c: true}), + ).toEqual({a: 1, c: 2}); + }); + + it('should ignore invalid fields', () => { + expect(create({b: 2}, {})).toEqual(null); + }); + + it('should not use the diff attribute', () => { + const diffA = jest.fn(); + expect(create({a: [2]}, {a: {diff: diffA}})).toEqual({a: [2]}); + expect(diffA).not.toBeCalled(); + }); + + it('should use the process attribute', () => { + const processA = jest.fn(a => a + 1); + expect(create({a: 2}, {a: {process: processA}})).toEqual({a: 3}); + expect(processA).toBeCalledWith(2); + }); + + it('should use the process attribute for functions as well', () => { + const process = (x: Object) => x; + const nextFunction = () => {}; + expect(create({a: nextFunction}, {a: {process}})).toEqual({ + a: nextFunction, + }); + }); + + it('should work with undefined styles', () => { + expect(create({style: undefined}, {style: {b: true}})).toEqual(null); + expect(create({style: {a: '#ffffff', b: 1}}, {style: {b: true}})).toEqual({ + b: 1, + }); + }); + + it('should flatten nested styles and predefined styles', () => { + const validStyleAttribute: AttributeConfiguration = { + style: {foo: true, bar: true}, + }; + expect(create({style: [{foo: 1}, {bar: 2}]}, validStyleAttribute)).toEqual({ + foo: 1, + bar: 2, + }); + expect(create({}, validStyleAttribute)).toEqual(null); + const barStyle = { + bar: 3, + }; + expect( + create({style: [[{foo: 1}, {foo: 2}], barStyle]}, validStyleAttribute), + ).toEqual({foo: 2, bar: 3}); + }); + + it('should not flatten nested props if attribute config is a primitive or only has diff/process', () => { + expect(create({a: {foo: 1, bar: 2}}, {a: true})).toEqual({ + a: {foo: 1, bar: 2}, + }); + expect(create({a: [{foo: 1}, {bar: 2}]}, {a: true})).toEqual({ + a: [{foo: 1}, {bar: 2}], + }); + expect(create({a: {foo: 1, bar: 2}}, {a: {diff: a => a}})).toEqual({ + a: {foo: 1, bar: 2}, + }); + expect( + create({a: [{foo: 1}, {bar: 2}]}, {a: {diff: a => a, process: a => a}}), + ).toEqual({a: [{foo: 1}, {bar: 2}]}); + }); + + it('handles attributes defined multiple times', () => { + const validAttributes: AttributeConfiguration = { + foo: true, + style: {foo: true}, + }; + expect(create({foo: 4, style: {foo: 2}}, validAttributes)).toEqual({ + foo: 2, + }); + expect(create({style: {foo: 2}}, validAttributes)).toEqual({ + foo: 2, + }); + expect(create({style: {foo: 2}, foo: 4}, validAttributes)).toEqual({ + foo: 4, + }); + expect(create({foo: 4, style: {foo: null}}, validAttributes)).toEqual({ + foo: null, // this should ideally be null. + }); + expect( + create({foo: 4, style: [{foo: null}, {foo: 5}]}, validAttributes), + ).toEqual({ + foo: 5, + }); + }); + + // Function properties are just markers to native that events should be sent. + it('should convert functions to booleans', () => { + expect( + create( + { + a: function () { + return 9; + }, + b: function () { + return 3; + }, + }, + {a: true, b: true}, + ), + ).toEqual({a: true, b: true}); + }); +}); + +describe('ReactNativeAttributePayload.diff', () => { + it('should work with simple example', () => { + expect(diff({a: 1, c: 3}, {b: 2, c: 3}, {a: true, b: true})).toEqual({ + a: null, + b: 2, + }); + }); + + it('should skip fields that are equal', () => { + expect( + diff( + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0}, + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0}, + {a: true, b: true, c: true, d: true, e: true, f: true}, + ), + ).toEqual(null); + }); + + it('should remove fields', () => { + expect(diff({a: 1}, {}, {a: true})).toEqual({a: null}); + }); + + it('should remove fields that are set to undefined', () => { + expect(diff({a: 1}, {a: undefined}, {a: true})).toEqual({a: null}); + }); + + it('should ignore invalid fields', () => { + expect(diff({a: 1}, {b: 2}, {})).toEqual(null); + }); + + it('should use the diff attribute', () => { + const diffA = jest.fn((a, b) => true); + const diffB = jest.fn((a, b) => false); + expect( + diff( + {a: [1], b: [3]}, + {a: [2], b: [4]}, + {a: {diff: diffA}, b: {diff: diffB}}, + ), + ).toEqual({a: [2]}); + expect(diffA).toBeCalledWith([1], [2]); + expect(diffB).toBeCalledWith([3], [4]); + }); + + it('should not use the diff attribute on addition/removal', () => { + const diffA = jest.fn(); + const diffB = jest.fn(); + expect( + diff({a: [1]}, {b: [2]}, {a: {diff: diffA}, b: {diff: diffB}}), + ).toEqual({a: null, b: [2]}); + expect(diffA).not.toBeCalled(); + expect(diffB).not.toBeCalled(); + }); + + it('should do deep diffs of Objects by default', () => { + expect( + diff( + {a: [1], b: {k: [3, 4]}, c: {k: [4, 4]}}, + {a: [2], b: {k: [3, 4]}, c: {k: [4, 5]}}, + {a: true, b: true, c: true}, + ), + ).toEqual({a: [2], c: {k: [4, 5]}}); + }); + + it('should work with undefined styles', () => { + expect( + diff( + {style: {a: '#ffffff', b: 1}}, + {style: undefined}, + {style: {b: true}}, + ), + ).toEqual({b: null}); + expect( + diff( + {style: undefined}, + {style: {a: '#ffffff', b: 1}}, + {style: {b: true}}, + ), + ).toEqual({b: 1}); + expect( + diff({style: undefined}, {style: undefined}, {style: {b: true}}), + ).toEqual(null); + }); + + it('should work with empty styles', () => { + const validAttributes: AttributeConfiguration = {a: true, b: true}; + expect(diff({a: 1, c: 3}, {}, validAttributes)).toEqual({a: null}); + expect(diff({}, {a: 1, c: 3}, validAttributes)).toEqual({a: 1}); + expect(diff({}, {}, validAttributes)).toEqual(null); + }); + + it('should flatten nested styles and predefined styles', () => { + const validStyleAttribute: AttributeConfiguration = { + style: {foo: true, bar: true}, + }; + + expect( + diff({}, {style: [{foo: 1}, {bar: 2}]}, validStyleAttribute), + ).toEqual({foo: 1, bar: 2}); + + expect( + diff({style: [{foo: 1}, {bar: 2}]}, {}, validStyleAttribute), + ).toEqual({foo: null, bar: null}); + + const barStyle = { + bar: 3, + }; + + expect( + diff({}, {style: [[{foo: 1}, {foo: 2}], barStyle]}, validStyleAttribute), + ).toEqual({foo: 2, bar: 3}); + }); + + it('should reset a value to a previous if it is removed', () => { + const validStyleAttribute: AttributeConfiguration = { + style: {foo: true, bar: true}, + }; + + expect( + diff( + {style: [{foo: 1}, {foo: 3}]}, + {style: [{foo: 1}, {bar: 2}]}, + validStyleAttribute, + ), + ).toEqual({foo: 1, bar: 2}); + }); + + it('should not clear removed props if they are still in another slot', () => { + const validStyleAttribute: AttributeConfiguration = { + style: {foo: true, bar: true}, + }; + + expect( + diff( + {style: [{}, {foo: 3, bar: 2}]}, + {style: [{foo: 3}, {bar: 2}]}, + validStyleAttribute, + ), + ).toEqual({foo: 3}); // this should ideally be null. heuristic tradeoff. + + expect( + diff( + {style: [{}, {foo: 3, bar: 2}]}, + {style: [{foo: 1, bar: 1}, {bar: 2}]}, + validStyleAttribute, + ), + ).toEqual({bar: 2, foo: 1}); + }); + + it('should clear a prop if a later style is explicit null/undefined', () => { + const validStyleAttribute: AttributeConfiguration = { + style: {foo: true, bar: true}, + }; + expect( + diff( + {style: [{}, {foo: 3, bar: 2}]}, + {style: [{foo: 1}, {bar: 2, foo: null}]}, + validStyleAttribute, + ), + ).toEqual({foo: null}); + + expect( + diff( + {style: [{foo: 3}, {foo: null, bar: 2}]}, + {style: [{foo: null}, {bar: 2}]}, + validStyleAttribute, + ), + ).toEqual({foo: null}); + + expect( + diff( + {style: [{foo: 1}, {foo: null}]}, + {style: [{foo: 2}, {foo: null}]}, + validStyleAttribute, + ), + ).toEqual({foo: null}); // this should ideally be null. heuristic. + + // Test the same case with object equality because an early bailout doesn't + // work in this case. + const fooObj = {foo: 3}; + expect( + diff( + {style: [{foo: 1}, fooObj]}, + {style: [{foo: 2}, fooObj]}, + validStyleAttribute, + ), + ).toEqual({foo: 3}); // this should ideally be null. heuristic. + + expect( + diff( + {style: [{foo: 1}, {foo: 3}]}, + {style: [{foo: 2}, {foo: undefined}]}, + validStyleAttribute, + ), + ).toEqual({foo: null}); // this should ideally be null. heuristic. + }); + + it('handles attributes defined multiple times', () => { + const validAttributes: AttributeConfiguration = { + foo: true, + style: {foo: true}, + }; + expect(diff({}, {foo: 4, style: {foo: 2}}, validAttributes)).toEqual({ + foo: 2, + }); + expect(diff({foo: 4}, {style: {foo: 2}}, validAttributes)).toEqual({ + foo: 2, + }); + expect(diff({style: {foo: 2}}, {foo: 4}, validAttributes)).toEqual({ + foo: 4, + }); + }); + + // Function properties are just markers to native that events should be sent. + it('should convert functions to booleans', () => { + // Note that if the property changes from one function to another, we don't + // need to send an update. + expect( + diff( + { + a: function () { + return 1; + }, + b: function () { + return 2; + }, + c: 3, + }, + { + b: function () { + return 9; + }, + c: function () { + return 3; + }, + }, + {a: true, b: true, c: true}, + ), + ).toEqual({a: null, c: true}); + }); + + it('should skip changed functions', () => { + expect( + diff( + { + a: function () { + return 1; + }, + }, + { + a: function () { + return 9; + }, + }, + {a: true}, + ), + ).toEqual(null); + }); + + it('should skip deeply-nested changed functions', () => { + expect( + diff( + { + wrapper: { + a: function () { + return 1; + }, + }, + }, + { + wrapper: { + a: function () { + return 9; + }, + }, + }, + {wrapper: true}, + ), + ).toEqual(null); + }); + + it('should use the process function config when prop is a function', () => { + const process = jest.fn(a => a); + const nextFunction = function () {}; + expect( + diff( + { + a: function () {}, + }, + { + a: nextFunction, + }, + {a: {process}}, + ), + ).toEqual({a: nextFunction}); + expect(process).toBeCalled(); + }); +});