Skip to content

Commit

Permalink
Use const [] for 0-arg prop children so that JS props.children valu…
Browse files Browse the repository at this point in the history
…es are identical
  • Loading branch information
greglittlefield-wf committed Feb 25, 2020
1 parent d0beec3 commit ca2ad9c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 14 deletions.
5 changes: 4 additions & 1 deletion lib/src/component_declaration/component_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,10 @@ abstract class UiProps extends MapBase
// Use `identical` since it compiles down to `===` in dart2js instead of calling equality helper functions,
// and we don't want to allow any object overriding `operator==` to claim it's equal to `_notSpecified`.
if (identical(c1, notSpecified)) {
childArguments = [];
// Use a const list so that empty children prop values are always identical
// in the JS props, resulting in JS libraries (e.g., react-redux) and Dart code alike
// not marking props as having changed as a result of rerendering the ReactElement with a new list.
childArguments = const [];
} else if (identical(c2, notSpecified)) {
childArguments = [c1];
} else if (identical(c3, notSpecified)) {
Expand Down
53 changes: 40 additions & 13 deletions test/over_react/component_declaration/component_base_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,35 @@ main() {
});
}

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

test('1', () {
final instance = builder(1);
expect(getJsChildren(instance), equals(1));
});
final instance2 = builder();
expect(getJsChildren(instance2), same(getJsChildren(instance)),
reason: 'zero arg children should always be the same List instance for perf reasons');
});

test('1', () {
final instance = builder(1);
expect(getJsChildren(instance), [1]);
});
} else {
test('0', () {
final instance = builder();
expect(getJsChildren(instance), isNull);
});

test('1', () {
final instance = builder(1);
expect(getJsChildren(instance), equals(1));
});
}

const firstGeneralCaseVariadicChildCount = 2;
const maxSupportedVariadicChildCount = 40;
Expand Down Expand Up @@ -137,9 +153,22 @@ main() {
stopRecordingValidationWarnings();
});

group('renders a DOM component with the correct children when', () {
_commonVariadicChildrenTests(Dom.div());
group('creates a ReactElement with the correct children:', () {
group('DomProps:', () {
_commonVariadicChildrenTests(Dom.div());
});

// Need to test these separately because they have different JS factory proxies
group('Dart Component:', () {
_commonVariadicChildrenTests(TestComponent());
});

group('Dart Component2:', () {
_commonVariadicChildrenTests(TestComponent2(), alwaysExpectList: true);
});
});

group('renders a DOM component with the correct children when', () {
test('no children are passed in', () {
var renderedNode = renderAndGetDom(Dom.div()());

Expand Down Expand Up @@ -201,8 +230,6 @@ main() {
}, testOn: '!js');

group('renders a composite Dart component with the correct children when', () {
_commonVariadicChildrenTests(TestComponent());

test('no children are passed in', () {

var renderedInstance = render(TestComponent()());
Expand Down

0 comments on commit ca2ad9c

Please sign in to comment.