Skip to content

Commit 911dbd9

Browse files
authored
feat(ReactNative): prioritize attribute config process function to allow processing function props (#32119)
## Summary In react-native props that are passed as function get converted to a boolean (`true`). This is the default pattern for event handlers in react-native. However, there are reasons for why you might want to opt-out of this behavior, and instead, pass along the actual function as the prop. Right now, there is no way to do this, and props that are functions always get set to `true`. The `ViewConfig` attributes already have the API for a `process` function. I simply moved the check for the process function up, so if a ViewConfig's prop attribute configured a process function this is always called first. This provides an API to opt out of the default behavior. This is the accompanied PR for react-native: - facebook/react-native#48777 ## How did you test this change? <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> I modified the code manually in a template react-native app and confirmed its working. This is a code path you only need in very special cases, thus it's a bit hard to provide a test for this. I recorded a video where you can see that the changes are active and the prop is being passed as native value. For this I created a custom native component with a view config that looked like this: ```js const viewConfig = { uiViewClassName: 'CustomView', bubblingEventTypes: {}, directEventTypes: {}, validAttributes: { nativeProp: { process: (nativeProp) => { // Identity function that simply returns the prop function callback // to opt out of this prop being set to `true` as its a function return nativeProp }, }, }, } ``` https://github.com/user-attachments/assets/493534b2-a508-4142-a760-0b1b24419e19 Additionally I made sure that this doesn't conflict with any existing view configs in react native. In general, this shouldn't be a breaking change, as for existing view configs it didn't made a difference if you simply set `myProp: true` or `myProp: { process: () => {...} }` because as soon as it was detected that the prop is a function the config wouldn't be used (which is what this PR fixes). Probably everyone, including the react-native core components use `myProp: true` for callback props, so this change should be fine.
1 parent c0b5a0c commit 911dbd9

File tree

2 files changed

+51
-19
lines changed

2 files changed

+51
-19
lines changed

packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,17 @@ function diffProperties(
254254
prevProp = prevProps[propKey];
255255
nextProp = nextProps[propKey];
256256

257-
// functions are converted to booleans as markers that the associated
258-
// events should be sent from native.
259257
if (typeof nextProp === 'function') {
260-
nextProp = (true: any);
261-
// If nextProp is not a function, then don't bother changing prevProp
262-
// since nextProp will win and go into the updatePayload regardless.
263-
if (typeof prevProp === 'function') {
264-
prevProp = (true: any);
258+
const attributeConfigHasProcess = typeof attributeConfig === 'object' && typeof attributeConfig.process === 'function';
259+
if (!attributeConfigHasProcess) {
260+
// functions are converted to booleans as markers that the associated
261+
// events should be sent from native.
262+
nextProp = (true: any);
263+
// If nextProp is not a function, then don't bother changing prevProp
264+
// since nextProp will win and go into the updatePayload regardless.
265+
if (typeof prevProp === 'function') {
266+
prevProp = (true: any);
267+
}
265268
}
266269
}
267270

@@ -444,18 +447,22 @@ function addNestedProperty(
444447
} else {
445448
continue;
446449
}
447-
} else if (typeof prop === 'function') {
448-
// A function prop. It represents an event handler. Pass it to native as 'true'.
449-
newValue = true;
450-
} else if (typeof attributeConfig !== 'object') {
451-
// An atomic prop. Doesn't need to be flattened.
452-
newValue = prop;
453-
} else if (typeof attributeConfig.process === 'function') {
454-
// An atomic prop with custom processing.
455-
newValue = attributeConfig.process(prop);
456-
} else if (typeof attributeConfig.diff === 'function') {
457-
// An atomic prop with custom diffing. We don't need to do diffing when adding props.
458-
newValue = prop;
450+
} else if (typeof attributeConfig === 'object') {
451+
if (typeof attributeConfig.process === 'function') {
452+
// An atomic prop with custom processing.
453+
newValue = attributeConfig.process(prop);
454+
} else if (typeof attributeConfig.diff === 'function') {
455+
// An atomic prop with custom diffing. We don't need to do diffing when adding props.
456+
newValue = prop;
457+
}
458+
} else {
459+
if (typeof prop === 'function') {
460+
// A function prop. It represents an event handler. Pass it to native as 'true'.
461+
newValue = true;
462+
} else {
463+
// An atomic prop. Doesn't need to be flattened.
464+
newValue = prop;
465+
}
459466
}
460467

461468
if (newValue !== undefined) {

packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ describe('ReactNativeAttributePayloadFabric.create', () => {
102102
expect(processA).toBeCalledWith(2);
103103
});
104104

105+
it('should use the process attribute for functions as well', () => {
106+
const process = x => x;
107+
const nextFunction = () => {};
108+
expect(create({a: nextFunction}, {a: {process}})).toEqual({
109+
a: nextFunction,
110+
});
111+
});
112+
105113
it('should work with undefined styles', () => {
106114
expect(create({style: undefined}, {style: {b: true}})).toEqual(null);
107115
expect(create({style: {a: '#ffffff', b: 1}}, {style: {b: true}})).toEqual({
@@ -452,4 +460,21 @@ describe('ReactNativeAttributePayloadFabric.diff', () => {
452460
),
453461
).toEqual(null);
454462
});
463+
464+
it('should use the process function config when prop is a function', () => {
465+
const process = jest.fn(a => a);
466+
const nextFunction = function () {};
467+
expect(
468+
diff(
469+
{
470+
a: function () {},
471+
},
472+
{
473+
a: nextFunction,
474+
},
475+
{a: {process}},
476+
),
477+
).toEqual({a: nextFunction});
478+
expect(process).toBeCalled();
479+
});
455480
});

0 commit comments

Comments
 (0)