Skip to content

Commit a083344

Browse files
authored
[DevTools] Compute environment names for the timeline (facebook#34892)
Stacked on facebook#34885. This refactors the timeline to store not just an id but a complex object for each step. This will later represent a group of boundaries. Each timeline step is assigned an environment name. We pick the last environment name (assumed to have resolved last) from the union of the parent and child environment names. I.e. a child step is considered to be blocked by the parent so if a child isn't blocked on any environment name it still gets marked as the parent's environment name. In a follow up, I'd like to reorder the document order timeline based on environment names to favor loading everything in one environment before the next.
1 parent 423c44b commit a083344

File tree

6 files changed

+155
-77
lines changed

6 files changed

+155
-77
lines changed

packages/react-devtools-shared/src/devtools/store.js

Lines changed: 77 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
shallowDiffers,
3535
utfDecodeStringWithRanges,
3636
parseElementDisplayNameFromBackend,
37+
unionOfTwoArrays,
3738
} from '../utils';
3839
import {localStorageGetItem, localStorageSetItem} from '../storage';
3940
import {__DEBUG__} from '../constants';
@@ -51,6 +52,7 @@ import type {
5152
ComponentFilter,
5253
ElementType,
5354
SuspenseNode,
55+
SuspenseTimelineStep,
5456
Rect,
5557
} from 'react-devtools-shared/src/frontend/types';
5658
import type {
@@ -895,13 +897,10 @@ export default class Store extends EventEmitter<{
895897
*/
896898
getSuspendableDocumentOrderSuspense(
897899
uniqueSuspendersOnly: boolean,
898-
): $ReadOnlyArray<SuspenseNode['id']> {
900+
): $ReadOnlyArray<SuspenseTimelineStep> {
901+
const target: Array<SuspenseTimelineStep> = [];
899902
const roots = this.roots;
900-
if (roots.length === 0) {
901-
return [];
902-
}
903-
904-
const list: SuspenseNode['id'][] = [];
903+
let rootStep: null | SuspenseTimelineStep = null;
905904
for (let i = 0; i < roots.length; i++) {
906905
const rootID = roots[i];
907906
const root = this.getElementByID(rootID);
@@ -912,44 +911,76 @@ export default class Store extends EventEmitter<{
912911

913912
const suspense = this.getSuspenseByID(rootID);
914913
if (suspense !== null) {
915-
if (list.length === 0) {
916-
// start with an arbitrary root that will allow inspection of the Screen
917-
list.push(suspense.id);
918-
}
919-
920-
const stack = [suspense];
921-
while (stack.length > 0) {
922-
const current = stack.pop();
923-
if (current === undefined) {
924-
continue;
925-
}
926-
// Ignore any suspense boundaries that has no visual representation as this is not
927-
// part of the visible loading sequence.
928-
// TODO: Consider making visible meta data and other side-effects get virtual rects.
929-
const hasRects =
930-
current.rects !== null &&
931-
current.rects.length > 0 &&
932-
current.rects.some(isNonZeroRect);
933-
if (
934-
hasRects &&
935-
(!uniqueSuspendersOnly || current.hasUniqueSuspenders) &&
936-
// Roots are already included as part of the Screen
937-
current.id !== rootID
938-
) {
939-
list.push(current.id);
940-
}
941-
// Add children in reverse order to maintain document order
942-
for (let j = current.children.length - 1; j >= 0; j--) {
943-
const childSuspense = this.getSuspenseByID(current.children[j]);
944-
if (childSuspense !== null) {
945-
stack.push(childSuspense);
946-
}
947-
}
914+
const environments = suspense.environments;
915+
const environmentName =
916+
environments.length > 0
917+
? environments[environments.length - 1]
918+
: null;
919+
if (rootStep === null) {
920+
// Arbitrarily use the first root as the root step id.
921+
rootStep = {
922+
id: suspense.id,
923+
environment: environmentName,
924+
};
925+
target.push(rootStep);
926+
} else if (rootStep.environment === null) {
927+
// If any root has an environment name, then let's use it.
928+
rootStep.environment = environmentName;
948929
}
930+
this.pushTimelineStepsInDocumentOrder(
931+
suspense.children,
932+
target,
933+
uniqueSuspendersOnly,
934+
environments,
935+
);
949936
}
950937
}
951938

952-
return list;
939+
return target;
940+
}
941+
942+
pushTimelineStepsInDocumentOrder(
943+
children: Array<SuspenseNode['id']>,
944+
target: Array<SuspenseTimelineStep>,
945+
uniqueSuspendersOnly: boolean,
946+
parentEnvironments: Array<string>,
947+
): void {
948+
for (let i = 0; i < children.length; i++) {
949+
const child = this.getSuspenseByID(children[i]);
950+
if (child === null) {
951+
continue;
952+
}
953+
// Ignore any suspense boundaries that has no visual representation as this is not
954+
// part of the visible loading sequence.
955+
// TODO: Consider making visible meta data and other side-effects get virtual rects.
956+
const hasRects =
957+
child.rects !== null &&
958+
child.rects.length > 0 &&
959+
child.rects.some(isNonZeroRect);
960+
const childEnvironments = child.environments;
961+
// Since children are blocked on the parent, they're also blocked by the parent environments.
962+
// Only if we discover a novel environment do we add that and it becomes the name we use.
963+
const unionEnvironments = unionOfTwoArrays(
964+
parentEnvironments,
965+
childEnvironments,
966+
);
967+
const environmentName =
968+
unionEnvironments.length > 0
969+
? unionEnvironments[unionEnvironments.length - 1]
970+
: null;
971+
if (hasRects && (!uniqueSuspendersOnly || child.hasUniqueSuspenders)) {
972+
target.push({
973+
id: child.id,
974+
environment: environmentName,
975+
});
976+
}
977+
this.pushTimelineStepsInDocumentOrder(
978+
child.children,
979+
target,
980+
uniqueSuspendersOnly,
981+
unionEnvironments,
982+
);
983+
}
953984
}
954985

955986
getRendererIDForElement(id: number): number | null {
@@ -1627,6 +1658,7 @@ export default class Store extends EventEmitter<{
16271658
rects,
16281659
hasUniqueSuspenders: false,
16291660
isSuspended: isSuspended,
1661+
environments: [],
16301662
});
16311663

16321664
hasSuspenseTreeChanged = true;
@@ -1812,7 +1844,10 @@ export default class Store extends EventEmitter<{
18121844
envIndex++
18131845
) {
18141846
const environmentNameStringID = operations[i++];
1815-
environmentNames.push(stringTable[environmentNameStringID]);
1847+
const environmentName = stringTable[environmentNameStringID];
1848+
if (environmentName != null) {
1849+
environmentNames.push(environmentName);
1850+
}
18161851
}
18171852
const suspense = this._idToSuspense.get(id);
18181853

@@ -1836,7 +1871,7 @@ export default class Store extends EventEmitter<{
18361871

18371872
suspense.hasUniqueSuspenders = hasUniqueSuspenders;
18381873
suspense.isSuspended = isSuspended;
1839-
// TODO: Recompute the environment names.
1874+
suspense.environments = environmentNames;
18401875
}
18411876

18421877
hasSuspenseTreeChanged = true;

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ function SuspenseRects({
154154
const selected = inspectedElementID === suspenseID;
155155

156156
const hovered =
157-
hoveredTimelineIndex > -1 && timeline[hoveredTimelineIndex] === suspenseID;
157+
hoveredTimelineIndex > -1 &&
158+
timeline[hoveredTimelineIndex].id === suspenseID;
158159

159160
const boundingBox = getBoundingBox(suspense.rects);
160161

packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ function SuspenseTimelineInput() {
3434
const max = timeline.length > 0 ? timeline.length - 1 : 0;
3535

3636
function switchSuspenseNode(nextTimelineIndex: number) {
37-
const nextSelectedSuspenseID = timeline[nextTimelineIndex];
37+
const nextSelectedSuspenseID = timeline[nextTimelineIndex].id;
3838
treeDispatch({
3939
type: 'SELECT_ELEMENT_BY_ID',
4040
payload: nextSelectedSuspenseID,
@@ -54,7 +54,7 @@ function SuspenseTimelineInput() {
5454
}
5555

5656
function handleHoverSegment(hoveredIndex: number) {
57-
const nextSelectedSuspenseID = timeline[hoveredIndex];
57+
const nextSelectedSuspenseID = timeline[hoveredIndex].id;
5858
suspenseTreeDispatch({
5959
type: 'HOVER_TIMELINE_FOR_ID',
6060
payload: nextSelectedSuspenseID,
@@ -68,7 +68,7 @@ function SuspenseTimelineInput() {
6868
}
6969

7070
function skipPrevious() {
71-
const nextSelectedSuspenseID = timeline[timelineIndex - 1];
71+
const nextSelectedSuspenseID = timeline[timelineIndex - 1].id;
7272
treeDispatch({
7373
type: 'SELECT_ELEMENT_BY_ID',
7474
payload: nextSelectedSuspenseID,
@@ -80,7 +80,7 @@ function SuspenseTimelineInput() {
8080
}
8181

8282
function skipForward() {
83-
const nextSelectedSuspenseID = timeline[timelineIndex + 1];
83+
const nextSelectedSuspenseID = timeline[timelineIndex + 1].id;
8484
treeDispatch({
8585
type: 'SELECT_ELEMENT_BY_ID',
8686
payload: nextSelectedSuspenseID,
@@ -106,7 +106,7 @@ function SuspenseTimelineInput() {
106106
// anything suspended in the root. The step after that should have one less
107107
// thing suspended. I.e. the first suspense boundary should be unsuspended
108108
// when it's selected. This also lets you show everything in the last step.
109-
const suspendedSet = timeline.slice(timelineIndex + 1);
109+
const suspendedSet = timeline.slice(timelineIndex + 1).map(step => step.id);
110110
bridge.send('overrideSuspenseMilestone', {
111111
suspendedSet,
112112
});

0 commit comments

Comments
 (0)