Skip to content

Commit 13965b4

Browse files
bvaughnbvaughn
authored
Interaction tracking ref-counting bug fixes (WIP) (#13590)
* Added new (failing) suspense+interaction tests * Add new tracing+suspense test harness fixture * Refactored interaction tracing to fix ref counting bug
1 parent 17e703c commit 13965b4

7 files changed

Lines changed: 925 additions & 225 deletions

File tree

‎fixtures/tracing/test.html‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!DOCTYPE html>
2+
<html style="width: 100%; height: 100%;">
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Test tracing UMD</title>
6+
<style>
7+
body {
8+
font-family: sans-serif;
9+
}
10+
</style>
11+
</head>
12+
<body>
13+
<div id="root"></div>
14+
<!-- Load the tracing API before react to test that it's lazily evaluated -->
15+
<script src="../../build/node_modules/scheduler/umd/scheduler.development.js"></script>
16+
<script src="../../build/node_modules/scheduler/umd/scheduler-tracing.development.js"></script>
17+
<script src="../../build/node_modules/react/umd/react.development.js"></script>
18+
<script src="../../build/node_modules/react-dom/umd/react-dom.development.js"></script>
19+
<script src="./test.js"></script>
20+
</body>
21+
</html>

‎fixtures/tracing/test.js‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
const {createElement, Component, Placeholder} = React;
2+
const {unstable_createRoot: createRoot} = ReactDOM;
3+
const {
4+
unstable_subscribe: subscribe,
5+
unstable_trace: trace,
6+
unstable_wrap: wrap,
7+
} = SchedulerTracing;
8+
9+
const createLogger = (backgroundColor, color, enabled) => (
10+
message,
11+
...args
12+
) => {
13+
if (enabled === false) return;
14+
console.groupCollapsed(
15+
`%c${message}`,
16+
`background-color: ${backgroundColor}; color: ${color}; padding: 2px 4px;`,
17+
...args
18+
);
19+
console.log(
20+
new Error('stack').stack
21+
.split('\n')
22+
.slice(2)
23+
.join('\n')
24+
);
25+
console.groupEnd();
26+
};
27+
28+
window.log = {
29+
app: createLogger('#37474f', '#fff'),
30+
interaction: createLogger('#6a1b9a', '#fff'),
31+
react: createLogger('#ff5722', '#fff'),
32+
tracing: createLogger('#2962ff', '#fff'),
33+
work: createLogger('#e1bee7', '#000'),
34+
};
35+
36+
// Fake suspense
37+
const resolvedValues = {};
38+
const read = key => {
39+
if (!resolvedValues[key]) {
40+
log.app(`Suspending for "${key}" ...`);
41+
throw new Promise(
42+
wrap(resolve => {
43+
setTimeout(
44+
wrap(() => {
45+
log.app(`Loaded "${key}" ...`);
46+
resolvedValues[key] = true;
47+
resolve(key);
48+
}),
49+
1000
50+
);
51+
})
52+
);
53+
}
54+
return key;
55+
};
56+
57+
const TestApp = () =>
58+
createElement(
59+
Placeholder,
60+
{delayMs: 100, fallback: createElement(PlaceholderText)},
61+
createElement(SuspendingChild, {text: 'foo'}),
62+
createElement(SuspendingChild, {text: 'bar'}),
63+
createElement(SuspendingChild, {text: 'baz'})
64+
);
65+
66+
const PlaceholderText = () => 'Loading ...';
67+
68+
const SuspendingChild = ({text}) => {
69+
const resolvedValue = read(text);
70+
return resolvedValue;
71+
};
72+
73+
subscribe({
74+
onInteractionScheduledWorkCompleted: interaction =>
75+
log.interaction(
76+
'onInteractionScheduledWorkCompleted',
77+
JSON.stringify(interaction)
78+
),
79+
onInteractionTraced: interaction =>
80+
log.interaction('onInteractionTraced', JSON.stringify(interaction)),
81+
onWorkCanceled: interactions =>
82+
log.work('onWorkCanceled', JSON.stringify(Array.from(interactions))),
83+
onWorkScheduled: interactions =>
84+
log.work('onWorkScheduled', JSON.stringify(Array.from(interactions))),
85+
onWorkStarted: interactions =>
86+
log.work('onWorkStarted', JSON.stringify(Array.from(interactions))),
87+
onWorkStopped: interactions =>
88+
log.work('onWorkStopped', JSON.stringify(Array.from(interactions))),
89+
});
90+
91+
const element = document.getElementById('root');
92+
trace('initial_render', performance.now(), () => {
93+
const root = createRoot(element);
94+
const batch = root.createBatch();
95+
log.app('batch.render()');
96+
batch.render(createElement(TestApp));
97+
batch.then(
98+
wrap(() => {
99+
log.app('batch.commit()');
100+
batch.commit();
101+
})
102+
);
103+
});

‎packages/react-reconciler/src/ReactFiberBeginWork.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,15 @@ import {
4141
Update,
4242
Ref,
4343
} from 'shared/ReactSideEffectTags';
44+
import {captureWillSyncRenderPlaceholder} from './ReactFiberScheduler';
4445
import ReactSharedInternals from 'shared/ReactSharedInternals';
4546
import {
4647
enableGetDerivedStateFromCatch,
4748
enableSuspense,
4849
debugRenderPhaseSideEffects,
4950
debugRenderPhaseSideEffectsForStrictMode,
5051
enableProfilerTimer,
52+
enableSchedulerTracing,
5153
} from 'shared/ReactFeatureFlags';
5254
import invariant from 'shared/invariant';
5355
import getComponentName from 'shared/getComponentName';
@@ -825,6 +827,13 @@ function updatePlaceholderComponent(
825827

826828
let nextDidTimeout;
827829
if (current !== null && workInProgress.updateQueue !== null) {
830+
if (enableSchedulerTracing) {
831+
// Handle special case of rendering a Placeholder for a sync, suspended tree.
832+
// We flag this to properly trace and count interactions.
833+
// Otherwise interaction pending count will be decremented too many times.
834+
captureWillSyncRenderPlaceholder();
835+
}
836+
828837
// We're outside strict mode. Something inside this Placeholder boundary
829838
// suspended during the last commit. Switch to the placholder.
830839
workInProgress.updateQueue = null;

0 commit comments

Comments
 (0)
⚡