@samitouri / QOS-React / commits / 51dd09631a

Add tests for ReactNativeAttributePayloadFabric.js (#29608)

## Summary This PR add tests for `ReactNativeAttributePayloadFabric.js`. It introduces `ReactNativeAttributePayloadFabric-test.internal.js`, which is a copy-paste of `ReactNativeAttributePayload-test.internal.js`. On top of that, there is a bunch of new test cases for the `ReactNativeAttributePayloadFabric.create` function. ## How did you test this change? ``` yarn test packages/react-native-renderer ```

Dmytro Rykun committed May 30, 2024 at 10:24 UTC 51dd09631ac0c7824fec55f38462846b6fe41d06
2 files changed +464 -26
packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js
+18 -26
@@ -449,17 +449,24 @@ function fastAddProperties(
449 props: Object,
450 validAttributes: AttributeConfiguration,
451 ): null | Object {
452 - let attributeConfig;
453 - let prop;
452 + // Flatten nested style props.
453 + if (isArray(props)) {
454 + for (let i = 0; i < props.length; i++) {
455 + payload = fastAddProperties(payload, props[i], validAttributes);
456 + }
457 + return payload;
458 + }
459
460 for (const propKey in props) {
456 - prop = props[propKey];
461 + const prop = props[propKey];
462
463 if (prop === undefined) {
464 continue;
465 }
466
462 - attributeConfig = ((validAttributes[propKey]: any): AttributeConfiguration);
467 + const attributeConfig = ((validAttributes[
468 + propKey
469 + ]: any): AttributeConfiguration);
470
471 if (attributeConfig == null) {
472 continue;
@@ -477,7 +484,7 @@ function fastAddProperties(
484 // An atomic prop with custom processing.
485 newValue = attributeConfig.process(prop);
486 } else if (typeof attributeConfig.diff === 'function') {
480 - // An atomic prop with custom diffing. We don't do diffing here.
487 + // An atomic prop with custom diffing. We don't need to do diffing when adding props.
488 newValue = prop;
489 }
490
@@ -489,17 +496,6 @@ function fastAddProperties(
496 continue;
497 }
498
492 - // Not-atomic prop that needs to be flattened. Likely it's the 'style' prop.
493 -
494 - // It can be an array.
495 - if (isArray(prop)) {
496 - for (let i = 0; i < prop.length; i++) {
497 - payload = fastAddProperties(payload, prop[i], attributeConfig);
498 - }
499 - continue;
500 - }
501 -
502 - // Or it can be an object.
499 payload = fastAddProperties(payload, prop, attributeConfig);
500 }
501
@@ -514,11 +510,7 @@ function addProperties(
510 props: Object,
511 validAttributes: AttributeConfiguration,
512 ): null | Object {
517 - if (enableAddPropertiesFastPath) {
518 - return fastAddProperties(updatePayload, props, validAttributes);
519 - } else {
520 - return diffProperties(updatePayload, emptyObject, props, validAttributes);
521 - }
513 + return diffProperties(updatePayload, emptyObject, props, validAttributes);
514 }
515
516 /**
@@ -538,11 +530,11 @@ export function create(
530 props: Object,
531 validAttributes: AttributeConfiguration,
532 ): null | Object {
541 - return addProperties(
542 - null, // updatePayload
543 - props,
544 - validAttributes,
545 - );
533 + if (enableAddPropertiesFastPath) {
534 + return fastAddProperties(null, props, validAttributes);
535 + } else {
536 + return addProperties(null, props, validAttributes);
537 + }
538 }
539
540 export function diff(
packages/react-native-renderer/src/__tests__/ReactNativeAttributePayloadFabric-test.internal.js new
+446
@@ -0,0 +1,446 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @jest-environment node
8 + */
9 +'use strict';
10 +
11 +const {diff, create} = require('../ReactNativeAttributePayloadFabric');
12 +
13 +describe('ReactNativeAttributePayload.create', () => {
14 + it('should work with simple example', () => {
15 + expect(create({b: 2, c: 3}, {a: true, b: true})).toEqual({
16 + b: 2,
17 + });
18 + });
19 +
20 + it('should work with complex example', () => {
21 + const validAttributes = {
22 + style: {
23 + position: true,
24 + zIndex: true,
25 + flexGrow: true,
26 + flexShrink: true,
27 + flexDirection: true,
28 + overflow: true,
29 + backgroundColor: true,
30 + },
31 + };
32 +
33 + expect(
34 + create(
35 + {
36 + style: [
37 + {
38 + flexGrow: 1,
39 + flexShrink: 1,
40 + flexDirection: 'row',
41 + overflow: 'scroll',
42 + },
43 + [
44 + {position: 'relative', zIndex: 2},
45 + {flexGrow: 0},
46 + {backgroundColor: 'red'},
47 + ],
48 + ],
49 + },
50 + validAttributes,
51 + ),
52 + ).toEqual({
53 + flexGrow: 0,
54 + flexShrink: 1,
55 + flexDirection: 'row',
56 + overflow: 'scroll',
57 + position: 'relative',
58 + zIndex: 2,
59 + backgroundColor: 'red',
60 + });
61 + });
62 +
63 + it('should ignore fields that are set to undefined', () => {
64 + expect(create({}, {a: true})).toEqual(null);
65 + expect(create({a: undefined}, {a: true})).toEqual(null);
66 + expect(create({a: undefined, b: undefined}, {a: true, b: true})).toEqual(
67 + null,
68 + );
69 + expect(
70 + create({a: undefined, b: undefined, c: 1}, {a: true, b: true}),
71 + ).toEqual(null);
72 + expect(
73 + create({a: undefined, b: undefined, c: 1}, {a: true, b: true, c: true}),
74 + ).toEqual({c: 1});
75 + expect(
76 + create({a: 1, b: undefined, c: 2}, {a: true, b: true, c: true}),
77 + ).toEqual({a: 1, c: 2});
78 + });
79 +
80 + it('should ignore invalid fields', () => {
81 + expect(create({b: 2}, {})).toEqual(null);
82 + });
83 +
84 + it('should not use the diff attribute', () => {
85 + const diffA = jest.fn();
86 + expect(create({a: [2]}, {a: {diff: diffA}})).toEqual({a: [2]});
87 + expect(diffA).not.toBeCalled();
88 + });
89 +
90 + it('should use the process attribute', () => {
91 + const processA = jest.fn(a => a + 1);
92 + expect(create({a: 2}, {a: {process: processA}})).toEqual({a: 3});
93 + expect(processA).toBeCalledWith(2);
94 + });
95 +
96 + it('should work with undefined styles', () => {
97 + expect(create({style: undefined}, {style: {b: true}})).toEqual(null);
98 + expect(create({style: {a: '#ffffff', b: 1}}, {style: {b: true}})).toEqual({
99 + b: 1,
100 + });
101 + });
102 +
103 + it('should flatten nested styles and predefined styles', () => {
104 + const validStyleAttribute = {someStyle: {foo: true, bar: true}};
105 + expect(
106 + create({someStyle: [{foo: 1}, {bar: 2}]}, validStyleAttribute),
107 + ).toEqual({foo: 1, bar: 2});
108 + expect(create({}, validStyleAttribute)).toEqual(null);
109 + const barStyle = {
110 + bar: 3,
111 + };
112 + expect(
113 + create(
114 + {someStyle: [[{foo: 1}, {foo: 2}], barStyle]},
115 + validStyleAttribute,
116 + ),
117 + ).toEqual({foo: 2, bar: 3});
118 + });
119 +
120 + it('should not flatten nested props if attribute config is a primitive or only has diff/process', () => {
121 + expect(create({a: {foo: 1, bar: 2}}, {a: true})).toEqual({
122 + a: {foo: 1, bar: 2},
123 + });
124 + expect(create({a: [{foo: 1}, {bar: 2}]}, {a: true})).toEqual({
125 + a: [{foo: 1}, {bar: 2}],
126 + });
127 + expect(create({a: {foo: 1, bar: 2}}, {a: {diff: a => a}})).toEqual({
128 + a: {foo: 1, bar: 2},
129 + });
130 + expect(
131 + create({a: [{foo: 1}, {bar: 2}]}, {a: {diff: a => a, process: a => a}}),
132 + ).toEqual({a: [{foo: 1}, {bar: 2}]});
133 + });
134 +
135 + it('handles attributes defined multiple times', () => {
136 + const validAttributes = {foo: true, style: {foo: true}};
137 + expect(create({foo: 4, style: {foo: 2}}, validAttributes)).toEqual({
138 + foo: 2,
139 + });
140 + expect(create({style: {foo: 2}}, validAttributes)).toEqual({
141 + foo: 2,
142 + });
143 + expect(create({style: {foo: 2}, foo: 4}, validAttributes)).toEqual({
144 + foo: 4,
145 + });
146 + expect(create({foo: 4, style: {foo: null}}, validAttributes)).toEqual({
147 + foo: null, // this should ideally be null.
148 + });
149 + expect(
150 + create({foo: 4, style: [{foo: null}, {foo: 5}]}, validAttributes),
151 + ).toEqual({
152 + foo: 5,
153 + });
154 + });
155 +
156 + // Function properties are just markers to native that events should be sent.
157 + it('should convert functions to booleans', () => {
158 + expect(
159 + create(
160 + {
161 + a: function () {
162 + return 9;
163 + },
164 + b: function () {
165 + return 3;
166 + },
167 + },
168 + {a: true, b: true},
169 + ),
170 + ).toEqual({a: true, b: true});
171 + });
172 +});
173 +
174 +describe('ReactNativeAttributePayload.diff', () => {
175 + it('should work with simple example', () => {
176 + expect(diff({a: 1, c: 3}, {b: 2, c: 3}, {a: true, b: true})).toEqual({
177 + a: null,
178 + b: 2,
179 + });
180 + });
181 +
182 + it('should skip fields that are equal', () => {
183 + expect(
184 + diff(
185 + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0},
186 + {a: 1, b: 'two', c: true, d: false, e: undefined, f: 0},
187 + {a: true, b: true, c: true, d: true, e: true, f: true},
188 + ),
189 + ).toEqual(null);
190 + });
191 +
192 + it('should remove fields', () => {
193 + expect(diff({a: 1}, {}, {a: true})).toEqual({a: null});
194 + });
195 +
196 + it('should remove fields that are set to undefined', () => {
197 + expect(diff({a: 1}, {a: undefined}, {a: true})).toEqual({a: null});
198 + });
199 +
200 + it('should ignore invalid fields', () => {
201 + expect(diff({a: 1}, {b: 2}, {})).toEqual(null);
202 + });
203 +
204 + it('should use the diff attribute', () => {
205 + const diffA = jest.fn((a, b) => true);
206 + const diffB = jest.fn((a, b) => false);
207 + expect(
208 + diff(
209 + {a: [1], b: [3]},
210 + {a: [2], b: [4]},
211 + {a: {diff: diffA}, b: {diff: diffB}},
212 + ),
213 + ).toEqual({a: [2]});
214 + expect(diffA).toBeCalledWith([1], [2]);
215 + expect(diffB).toBeCalledWith([3], [4]);
216 + });
217 +
218 + it('should not use the diff attribute on addition/removal', () => {
219 + const diffA = jest.fn();
220 + const diffB = jest.fn();
221 + expect(
222 + diff({a: [1]}, {b: [2]}, {a: {diff: diffA}, b: {diff: diffB}}),
223 + ).toEqual({a: null, b: [2]});
224 + expect(diffA).not.toBeCalled();
225 + expect(diffB).not.toBeCalled();
226 + });
227 +
228 + it('should do deep diffs of Objects by default', () => {
229 + expect(
230 + diff(
231 + {a: [1], b: {k: [3, 4]}, c: {k: [4, 4]}},
232 + {a: [2], b: {k: [3, 4]}, c: {k: [4, 5]}},
233 + {a: true, b: true, c: true},
234 + ),
235 + ).toEqual({a: [2], c: {k: [4, 5]}});
236 + });
237 +
238 + it('should work with undefined styles', () => {
239 + expect(
240 + diff(
241 + {style: {a: '#ffffff', b: 1}},
242 + {style: undefined},
243 + {style: {b: true}},
244 + ),
245 + ).toEqual({b: null});
246 + expect(
247 + diff(
248 + {style: undefined},
249 + {style: {a: '#ffffff', b: 1}},
250 + {style: {b: true}},
251 + ),
252 + ).toEqual({b: 1});
253 + expect(
254 + diff({style: undefined}, {style: undefined}, {style: {b: true}}),
255 + ).toEqual(null);
256 + });
257 +
258 + it('should work with empty styles', () => {
259 + expect(diff({a: 1, c: 3}, {}, {a: true, b: true})).toEqual({a: null});
260 + expect(diff({}, {a: 1, c: 3}, {a: true, b: true})).toEqual({a: 1});
261 + expect(diff({}, {}, {a: true, b: true})).toEqual(null);
262 + });
263 +
264 + it('should flatten nested styles and predefined styles', () => {
265 + const validStyleAttribute = {someStyle: {foo: true, bar: true}};
266 +
267 + expect(
268 + diff({}, {someStyle: [{foo: 1}, {bar: 2}]}, validStyleAttribute),
269 + ).toEqual({foo: 1, bar: 2});
270 +
271 + expect(
272 + diff({someStyle: [{foo: 1}, {bar: 2}]}, {}, validStyleAttribute),
273 + ).toEqual({foo: null, bar: null});
274 +
275 + const barStyle = {
276 + bar: 3,
277 + };
278 +
279 + expect(
280 + diff(
281 + {},
282 + {someStyle: [[{foo: 1}, {foo: 2}], barStyle]},
283 + validStyleAttribute,
284 + ),
285 + ).toEqual({foo: 2, bar: 3});
286 + });
287 +
288 + it('should reset a value to a previous if it is removed', () => {
289 + const validStyleAttribute = {someStyle: {foo: true, bar: true}};
290 +
291 + expect(
292 + diff(
293 + {someStyle: [{foo: 1}, {foo: 3}]},
294 + {someStyle: [{foo: 1}, {bar: 2}]},
295 + validStyleAttribute,
296 + ),
297 + ).toEqual({foo: 1, bar: 2});
298 + });
299 +
300 + it('should not clear removed props if they are still in another slot', () => {
301 + const validStyleAttribute = {someStyle: {foo: true, bar: true}};
302 +
303 + expect(
304 + diff(
305 + {someStyle: [{}, {foo: 3, bar: 2}]},
306 + {someStyle: [{foo: 3}, {bar: 2}]},
307 + validStyleAttribute,
308 + ),
309 + ).toEqual({foo: 3}); // this should ideally be null. heuristic tradeoff.
310 +
311 + expect(
312 + diff(
313 + {someStyle: [{}, {foo: 3, bar: 2}]},
314 + {someStyle: [{foo: 1, bar: 1}, {bar: 2}]},
315 + validStyleAttribute,
316 + ),
317 + ).toEqual({bar: 2, foo: 1});
318 + });
319 +
320 + it('should clear a prop if a later style is explicit null/undefined', () => {
321 + const validStyleAttribute = {someStyle: {foo: true, bar: true}};
322 + expect(
323 + diff(
324 + {someStyle: [{}, {foo: 3, bar: 2}]},
325 + {someStyle: [{foo: 1}, {bar: 2, foo: null}]},
326 + validStyleAttribute,
327 + ),
328 + ).toEqual({foo: null});
329 +
330 + expect(
331 + diff(
332 + {someStyle: [{foo: 3}, {foo: null, bar: 2}]},
333 + {someStyle: [{foo: null}, {bar: 2}]},
334 + validStyleAttribute,
335 + ),
336 + ).toEqual({foo: null});
337 +
338 + expect(
339 + diff(
340 + {someStyle: [{foo: 1}, {foo: null}]},
341 + {someStyle: [{foo: 2}, {foo: null}]},
342 + validStyleAttribute,
343 + ),
344 + ).toEqual({foo: null}); // this should ideally be null. heuristic.
345 +
346 + // Test the same case with object equality because an early bailout doesn't
347 + // work in this case.
348 + const fooObj = {foo: 3};
349 + expect(
350 + diff(
351 + {someStyle: [{foo: 1}, fooObj]},
352 + {someStyle: [{foo: 2}, fooObj]},
353 + validStyleAttribute,
354 + ),
355 + ).toEqual({foo: 3}); // this should ideally be null. heuristic.
356 +
357 + expect(
358 + diff(
359 + {someStyle: [{foo: 1}, {foo: 3}]},
360 + {someStyle: [{foo: 2}, {foo: undefined}]},
361 + validStyleAttribute,
362 + ),
363 + ).toEqual({foo: null}); // this should ideally be null. heuristic.
364 + });
365 +
366 + it('handles attributes defined multiple times', () => {
367 + const validAttributes = {foo: true, style: {foo: true}};
368 + expect(diff({}, {foo: 4, style: {foo: 2}}, validAttributes)).toEqual({
369 + foo: 2,
370 + });
371 + expect(diff({foo: 4}, {style: {foo: 2}}, validAttributes)).toEqual({
372 + foo: 2,
373 + });
374 + expect(diff({style: {foo: 2}}, {foo: 4}, validAttributes)).toEqual({
375 + foo: 4,
376 + });
377 + });
378 +
379 + // Function properties are just markers to native that events should be sent.
380 + it('should convert functions to booleans', () => {
381 + // Note that if the property changes from one function to another, we don't
382 + // need to send an update.
383 + expect(
384 + diff(
385 + {
386 + a: function () {
387 + return 1;
388 + },
389 + b: function () {
390 + return 2;
391 + },
392 + c: 3,
393 + },
394 + {
395 + b: function () {
396 + return 9;
397 + },
398 + c: function () {
399 + return 3;
400 + },
401 + },
402 + {a: true, b: true, c: true},
403 + ),
404 + ).toEqual({a: null, c: true});
405 + });
406 +
407 + it('should skip changed functions', () => {
408 + expect(
409 + diff(
410 + {
411 + a: function () {
412 + return 1;
413 + },
414 + },
415 + {
416 + a: function () {
417 + return 9;
418 + },
419 + },
420 + {a: true},
421 + ),
422 + ).toEqual(null);
423 + });
424 +
425 + it('should skip deeply-nested changed functions', () => {
426 + expect(
427 + diff(
428 + {
429 + wrapper: {
430 + a: function () {
431 + return 1;
432 + },
433 + },
434 + },
435 + {
436 + wrapper: {
437 + a: function () {
438 + return 9;
439 + },
440 + },
441 + },
442 + {wrapper: true},
443 + ),
444 + ).toEqual(null);
445 + });
446 +});