Skip to content

Commit

Permalink
Improve format of mounting logs (facebook#49016)
Browse files Browse the repository at this point in the history
Summary:

Changelog: [internal]

This replaces the existing string-based logs with something more structured, and increases the coverage to properly log all operations.

As part of this work I had to refactor how we record mutations so they would be done while applying the mutations, and not before/after where necessary metadata might not be available yet/anymore.

Reviewed By: sammy-SC

Differential Revision: D67549201
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jan 29, 2025
1 parent 5d1af6f commit edfee73
Show file tree
Hide file tree
Showing 6 changed files with 204 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('focus view command', () => {
Fantom.runTask(() => {
root.render(
<TextInput
nativeID="text-input"
ref={node => {
if (node) {
node.focus();
Expand All @@ -35,13 +36,12 @@ describe('focus view command', () => {
);
});

const mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(2);
expect(mountingLogs[0]).toBe('create view type: `AndroidTextInput`');
expect(mountingLogs[1]).toBe(
'dispatch command `focus` on component `AndroidTextInput`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Update {type: "RootView", nativeID: (root)}',
'Create {type: "AndroidTextInput", nativeID: "text-input"}',
'Insert {type: "AndroidTextInput", parentNativeID: (root), index: 0, nativeID: "text-input"}',
'Command {type: "AndroidTextInput", nativeID: "text-input", name: "focus"}',
]);
});

it('creates view before dispatching view command from useLayoutEffect', () => {
Expand All @@ -56,19 +56,18 @@ describe('focus view command', () => {
textInputRef.current?.focus();
});

return <TextInput ref={textInputRef} />;
return <TextInput ref={textInputRef} nativeID="text-input" />;
}
Fantom.runTask(() => {
root.render(<Component />);
});

const mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(2);
expect(mountingLogs[0]).toBe('create view type: `AndroidTextInput`');
expect(mountingLogs[1]).toBe(
'dispatch command `focus` on component `AndroidTextInput`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Update {type: "RootView", nativeID: (root)}',
'Create {type: "AndroidTextInput", nativeID: "text-input"}',
'Insert {type: "AndroidTextInput", parentNativeID: (root), index: 0, nativeID: "text-input"}',
'Command {type: "AndroidTextInput", nativeID: "text-input", name: "focus"}',
]);
});

it('creates view before dispatching view command from useEffect', () => {
Expand All @@ -83,19 +82,19 @@ describe('focus view command', () => {
textInputRef.current?.focus();
});

return <TextInput ref={textInputRef} />;
return <TextInput ref={textInputRef} nativeID="text-input" />;
}

Fantom.runTask(() => {
root.render(<Component />);
});

const mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(2);
expect(mountingLogs[0]).toBe('create view type: `AndroidTextInput`');
expect(mountingLogs[1]).toBe(
'dispatch command `focus` on component `AndroidTextInput`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Update {type: "RootView", nativeID: (root)}',
'Create {type: "AndroidTextInput", nativeID: "text-input"}',
'Insert {type: "AndroidTextInput", parentNativeID: (root), index: 0, nativeID: "text-input"}',
'Command {type: "AndroidTextInput", nativeID: "text-input", name: "focus"}',
]);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function Square(props: {squareId: SquareId}) {
if (data == null) {
data = use(fetchData(props.squareId));
}
return <View key={data.color} nativeID={'square with data: ' + data.color} />;
return <View key={data.color} nativeID={`square-with-data-${data.color}`} />;
}

function GreenSquare() {
Expand All @@ -104,7 +104,7 @@ function RedSquare() {
}

function Fallback() {
return <View nativeID="suspense fallback" />;
return <View nativeID="suspense-fallback" />;
}

describe('Suspense', () => {
Expand All @@ -120,25 +120,24 @@ describe('Suspense', () => {
);
});

let mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `suspense fallback`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Update {type: "RootView", nativeID: (root)}',
'Create {type: "View", nativeID: "suspense-fallback"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "suspense-fallback"}',
]);

expect(resolveFunction).not.toBeNull();
Fantom.runTask(() => {
resolveFunction?.();
resolveFunction = null;
});

mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `square with data: green`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: "suspense-fallback"}',
'Delete {type: "View", nativeID: "suspense-fallback"}',
'Create {type: "View", nativeID: "square-with-data-green"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-green"}',
]);

Fantom.runTask(() => {
root.render(
Expand All @@ -148,25 +147,25 @@ describe('Suspense', () => {
);
});

mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `suspense fallback`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-green"}',
'Delete {type: "View", nativeID: "square-with-data-green"}',
'Create {type: "View", nativeID: "suspense-fallback"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "suspense-fallback"}',
]);

expect(resolveFunction).not.toBeNull();
Fantom.runTask(() => {
resolveFunction?.();
resolveFunction = null;
});

mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `square with data: red`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: "suspense-fallback"}',
'Delete {type: "View", nativeID: "suspense-fallback"}',
'Create {type: "View", nativeID: "square-with-data-red"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-red"}',
]);

Fantom.runTask(() => {
root.render(
Expand All @@ -176,12 +175,12 @@ describe('Suspense', () => {
);
});

mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `square with data: green`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-red"}',
'Delete {type: "View", nativeID: "square-with-data-red"}',
'Create {type: "View", nativeID: "square-with-data-green"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-green"}',
]);

expect(resolveFunction).toBeNull();
});
Expand All @@ -206,12 +205,11 @@ describe('Suspense', () => {
root.render(<App color="green" />);
});

let mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `square with data: green`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Update {type: "RootView", nativeID: (root)}',
'Create {type: "View", nativeID: "square-with-data-green"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-green"}',
]);

expect(resolveFunction).toBeNull();
Fantom.runTask(() => {
Expand All @@ -220,22 +218,20 @@ describe('Suspense', () => {
});
});

mountingLogs = root.takeMountingManagerLogs();

// Green square is still mounted. Fallback is not shown to the user.
expect(mountingLogs.length).toBe(0);
expect(root.takeMountingManagerLogs()).toEqual([]);

expect(resolveFunction).not.toBeNull();
Fantom.runTask(() => {
resolveFunction?.();
resolveFunction = null;
});

mountingLogs = root.takeMountingManagerLogs();

expect(mountingLogs.length).toBe(1);
expect(mountingLogs[0]).toBe(
'create view type: `View` nativeId: `square with data: red`',
);
expect(root.takeMountingManagerLogs()).toEqual([
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-green"}',
'Delete {type: "View", nativeID: "square-with-data-green"}',
'Create {type: "View", nativeID: "square-with-data-red"}',
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: "square-with-data-red"}',
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ StubView::operator ShadowView() const {
shadowView.eventEmitter = eventEmitter;
shadowView.layoutMetrics = layoutMetrics;
shadowView.state = state;
shadowView.traits = traits;
return shadowView;
}

Expand All @@ -35,6 +36,7 @@ void StubView::update(const ShadowView& shadowView) {
eventEmitter = shadowView.eventEmitter;
layoutMetrics = shadowView.layoutMetrics;
state = shadowView.state;
traits = shadowView.traits;
}

bool operator==(const StubView& lhs, const StubView& rhs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class StubView final {
ComponentHandle componentHandle;
SurfaceId surfaceId;
Tag tag;
ShadowNodeTraits traits{};
Props::Shared props;
SharedEventEmitter eventEmitter;
LayoutMetrics layoutMetrics;
Expand Down
Loading

0 comments on commit edfee73

Please sign in to comment.