Skip to content

Commit

Permalink
Fix fastAddProperties to properly nullify style props (#30334)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes the `fastAddProperties` function. Now it nullifies a prop
if it was defined in one of the items of a style array, but then set to
`undefined` or `null` in one of the subsequent items. E.g. `style:
[{top: 0}, {top: undefined}]` should evaluate to `{top: null}`. Also
added a test case for that.

## How did you test this change?
```
yarn test packages/react-native-renderer -r=xplat --variant=false
yarn test packages/react-native-renderer -r=xplat --variant=true
yarn flow native
```
  • Loading branch information
dmytrorykun authored Jul 15, 2024
1 parent fc1371f commit f510ece
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -464,10 +464,6 @@ function fastAddProperties(
for (const propKey in props) {
const prop = props[propKey];

if (prop === undefined) {
continue;
}

const attributeConfig = ((validAttributes[
propKey
]: any): AttributeConfiguration);
Expand All @@ -478,7 +474,14 @@ function fastAddProperties(

let newValue;

if (typeof prop === 'function') {
if (prop === undefined) {
// Discard the prop if it was previously defined.
if (payload && payload[propKey] !== undefined) {
newValue = null;
} else {
continue;
}
} else if (typeof prop === 'function') {
// A function prop. It represents an event handler. Pass it to native as 'true'.
newValue = true;
} else if (typeof attributeConfig !== 'object') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,16 @@ describe('ReactNativeAttributePayloadFabric.create', () => {
});
});

it('should ignore fields that are set to undefined', () => {
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(
Expand Down

0 comments on commit f510ece

Please sign in to comment.