Skip to content

Commit ca2ad9c

Browse files
Use const [] for 0-arg prop children so that JS props.children values are identical
1 parent d0beec3 commit ca2ad9c

File tree

2 files changed

+44
-14
lines changed

2 files changed

+44
-14
lines changed

lib/src/component_declaration/component_base.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,10 @@ abstract class UiProps extends MapBase
525525
// Use `identical` since it compiles down to `===` in dart2js instead of calling equality helper functions,
526526
// and we don't want to allow any object overriding `operator==` to claim it's equal to `_notSpecified`.
527527
if (identical(c1, notSpecified)) {
528-
childArguments = [];
528+
// Use a const list so that empty children prop values are always identical
529+
// in the JS props, resulting in JS libraries (e.g., react-redux) and Dart code alike
530+
// not marking props as having changed as a result of rerendering the ReactElement with a new list.
531+
childArguments = const [];
529532
} else if (identical(c2, notSpecified)) {
530533
childArguments = [c1];
531534
} else if (identical(c3, notSpecified)) {

test/over_react/component_declaration/component_base_test.dart

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,19 +86,35 @@ main() {
8686
});
8787
}
8888

89-
void _commonVariadicChildrenTests(UiProps builder) {
89+
void _commonVariadicChildrenTests(UiProps builder, {bool alwaysExpectList = false}) {
9090
// There are different code paths for 0, 1, 2, 3, 4, 5, 6, and 6+ arguments.
9191
// Test all of them.
9292
group('a number of variadic children:', () {
93-
test('0', () {
94-
final instance = builder();
95-
expect(getJsChildren(instance), isNull);
96-
});
93+
if (alwaysExpectList) {
94+
test('0', () {
95+
final instance = builder();
96+
expect(getJsChildren(instance), []);
9797

98-
test('1', () {
99-
final instance = builder(1);
100-
expect(getJsChildren(instance), equals(1));
101-
});
98+
final instance2 = builder();
99+
expect(getJsChildren(instance2), same(getJsChildren(instance)),
100+
reason: 'zero arg children should always be the same List instance for perf reasons');
101+
});
102+
103+
test('1', () {
104+
final instance = builder(1);
105+
expect(getJsChildren(instance), [1]);
106+
});
107+
} else {
108+
test('0', () {
109+
final instance = builder();
110+
expect(getJsChildren(instance), isNull);
111+
});
112+
113+
test('1', () {
114+
final instance = builder(1);
115+
expect(getJsChildren(instance), equals(1));
116+
});
117+
}
102118

103119
const firstGeneralCaseVariadicChildCount = 2;
104120
const maxSupportedVariadicChildCount = 40;
@@ -137,9 +153,22 @@ main() {
137153
stopRecordingValidationWarnings();
138154
});
139155

140-
group('renders a DOM component with the correct children when', () {
141-
_commonVariadicChildrenTests(Dom.div());
156+
group('creates a ReactElement with the correct children:', () {
157+
group('DomProps:', () {
158+
_commonVariadicChildrenTests(Dom.div());
159+
});
142160

161+
// Need to test these separately because they have different JS factory proxies
162+
group('Dart Component:', () {
163+
_commonVariadicChildrenTests(TestComponent());
164+
});
165+
166+
group('Dart Component2:', () {
167+
_commonVariadicChildrenTests(TestComponent2(), alwaysExpectList: true);
168+
});
169+
});
170+
171+
group('renders a DOM component with the correct children when', () {
143172
test('no children are passed in', () {
144173
var renderedNode = renderAndGetDom(Dom.div()());
145174

@@ -201,8 +230,6 @@ main() {
201230
}, testOn: '!js');
202231

203232
group('renders a composite Dart component with the correct children when', () {
204-
_commonVariadicChildrenTests(TestComponent());
205-
206233
test('no children are passed in', () {
207234

208235
var renderedInstance = render(TestComponent()());

0 commit comments

Comments
 (0)