Skip to content

Commit bc8bd24

Browse files
authored
Run persistent mode tests in CI (#15029)
* Add command to run tests in persistent mode * Convert Suspense fuzz tester to use noop renderer So we can run it in persistent mode, too. * Don't mutate stateNode in appendAllChildren We can't mutate the stateNode in appendAllChildren because the children could be current. This is a bit weird because now the child that we append is different from the one on the fiber stateNode. I think this makes conceptual sense, but I suspect this likely breaks an assumption in Fabric. With this approach, we no longer need to clone to unhide the children, so I removed those host config methods. Fixes bug surfaced by fuzz tester. (The test case that failed was the one that's already hard coded.) * In persistent mode, disable test that reads a ref Refs behave differently in persistent mode. I added a TODO to write a persistent mode version of this test. * Run persistent mode tests in CI * test-persistent should skip files without noop If a file doesn't reference react-noop-renderer, we shouldn't bother running it in persistent mode, since the results will be identical to the normal test run. * Remove module constructor from placeholder tests We don't need this now that we have the ability to run any test file in either mutation or persistent mode. * Revert "test-persistent should skip files without noop" Seb objected to adding shelljs as a dep and I'm too lazy to worry about Windows support so whatever I'll just revert this. * Delete duplicate file
1 parent 3f4852f commit bc8bd24

12 files changed

+611
-699
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js",
101101
"debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/.bin/jest --config ./scripts/jest/config.source.js --runInBand",
102102
"test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
103+
"test-persistent": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-persistent.js",
103104
"test-fire": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-fire.js",
104105
"test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js",
105106
"test-fire-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source-fire.js",

packages/react-native-renderer/src/ReactFabricHostConfig.js

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -387,25 +387,6 @@ export function cloneHiddenInstance(
387387
};
388388
}
389389

390-
export function cloneUnhiddenInstance(
391-
instance: Instance,
392-
type: string,
393-
props: Props,
394-
internalInstanceHandle: Object,
395-
): Instance {
396-
const viewConfig = instance.canonical.viewConfig;
397-
const node = instance.node;
398-
const updatePayload = diff(
399-
{...props, style: [props.style, {display: 'none'}]},
400-
props,
401-
viewConfig.validAttributes,
402-
);
403-
return {
404-
node: cloneNodeWithNewProps(node, updatePayload),
405-
canonical: instance.canonical,
406-
};
407-
}
408-
409390
export function cloneHiddenTextInstance(
410391
instance: Instance,
411392
text: string,
@@ -414,14 +395,6 @@ export function cloneHiddenTextInstance(
414395
throw new Error('Not yet implemented.');
415396
}
416397

417-
export function cloneUnhiddenTextInstance(
418-
instance: Instance,
419-
text: string,
420-
internalInstanceHandle: Object,
421-
): TextInstance {
422-
throw new Error('Not yet implemented.');
423-
}
424-
425398
export function createContainerChildSet(container: Container): ChildSet {
426399
return createChildNodeSet(container);
427400
}

packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -486,26 +486,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
486486
return clone;
487487
},
488488

489-
cloneUnhiddenInstance(
490-
instance: Instance,
491-
type: string,
492-
props: Props,
493-
internalInstanceHandle: Object,
494-
): Instance {
495-
const clone = cloneInstance(
496-
instance,
497-
null,
498-
type,
499-
props,
500-
props,
501-
internalInstanceHandle,
502-
true,
503-
null,
504-
);
505-
clone.hidden = props.hidden === true;
506-
return clone;
507-
},
508-
509489
cloneHiddenTextInstance(
510490
instance: TextInstance,
511491
text: string,
@@ -528,29 +508,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
528508
});
529509
return clone;
530510
},
531-
532-
cloneUnhiddenTextInstance(
533-
instance: TextInstance,
534-
text: string,
535-
internalInstanceHandle: Object,
536-
): TextInstance {
537-
const clone = {
538-
text: instance.text,
539-
id: instanceCounter++,
540-
hidden: false,
541-
context: instance.context,
542-
};
543-
// Hide from unit tests
544-
Object.defineProperty(clone, 'id', {
545-
value: clone.id,
546-
enumerable: false,
547-
});
548-
Object.defineProperty(clone, 'context', {
549-
value: clone.context,
550-
enumerable: false,
551-
});
552-
return clone;
553-
},
554511
};
555512

556513
const NoopRenderer = reconciler(hostConfig);

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 59 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ import {
5959
supportsPersistence,
6060
cloneInstance,
6161
cloneHiddenInstance,
62-
cloneUnhiddenInstance,
6362
cloneHiddenTextInstance,
64-
cloneUnhiddenTextInstance,
6563
createContainerChildSet,
6664
appendChildToContainerChildSet,
6765
finalizeContainerChildren,
@@ -209,31 +207,19 @@ if (supportsMutation) {
209207
// eslint-disable-next-line no-labels
210208
branches: if (node.tag === HostComponent) {
211209
let instance = node.stateNode;
212-
if (needsVisibilityToggle) {
210+
if (needsVisibilityToggle && isHidden) {
211+
// This child is inside a timed out tree. Hide it.
213212
const props = node.memoizedProps;
214213
const type = node.type;
215-
if (isHidden) {
216-
// This child is inside a timed out tree. Hide it.
217-
instance = cloneHiddenInstance(instance, type, props, node);
218-
} else {
219-
// This child was previously inside a timed out tree. If it was not
220-
// updated during this render, it may need to be unhidden. Clone
221-
// again to be sure.
222-
instance = cloneUnhiddenInstance(instance, type, props, node);
223-
}
224-
node.stateNode = instance;
214+
instance = cloneHiddenInstance(instance, type, props, node);
225215
}
226216
appendInitialChild(parent, instance);
227217
} else if (node.tag === HostText) {
228218
let instance = node.stateNode;
229-
if (needsVisibilityToggle) {
219+
if (needsVisibilityToggle && isHidden) {
220+
// This child is inside a timed out tree. Hide it.
230221
const text = node.memoizedProps;
231-
if (isHidden) {
232-
instance = cloneHiddenTextInstance(instance, text, node);
233-
} else {
234-
instance = cloneUnhiddenTextInstance(instance, text, node);
235-
}
236-
node.stateNode = instance;
222+
instance = cloneHiddenTextInstance(instance, text, node);
237223
}
238224
appendInitialChild(parent, instance);
239225
} else if (node.tag === HostPortal) {
@@ -247,15 +233,22 @@ if (supportsMutation) {
247233
if (newIsHidden) {
248234
const primaryChildParent = node.child;
249235
if (primaryChildParent !== null) {
250-
appendAllChildren(parent, primaryChildParent, true, newIsHidden);
251-
node = primaryChildParent.sibling;
252-
continue;
236+
if (primaryChildParent.child !== null) {
237+
primaryChildParent.child.return = primaryChildParent;
238+
appendAllChildren(
239+
parent,
240+
primaryChildParent,
241+
true,
242+
newIsHidden,
243+
);
244+
}
245+
const fallbackChildParent = primaryChildParent.sibling;
246+
if (fallbackChildParent !== null) {
247+
fallbackChildParent.return = node;
248+
node = fallbackChildParent;
249+
continue;
250+
}
253251
}
254-
} else {
255-
const primaryChildParent = node;
256-
appendAllChildren(parent, primaryChildParent, true, newIsHidden);
257-
// eslint-disable-next-line no-labels
258-
break branches;
259252
}
260253
}
261254
if (node.child !== null) {
@@ -299,31 +292,19 @@ if (supportsMutation) {
299292
// eslint-disable-next-line no-labels
300293
branches: if (node.tag === HostComponent) {
301294
let instance = node.stateNode;
302-
if (needsVisibilityToggle) {
295+
if (needsVisibilityToggle && isHidden) {
296+
// This child is inside a timed out tree. Hide it.
303297
const props = node.memoizedProps;
304298
const type = node.type;
305-
if (isHidden) {
306-
// This child is inside a timed out tree. Hide it.
307-
instance = cloneHiddenInstance(instance, type, props, node);
308-
} else {
309-
// This child was previously inside a timed out tree. If it was not
310-
// updated during this render, it may need to be unhidden. Clone
311-
// again to be sure.
312-
instance = cloneUnhiddenInstance(instance, type, props, node);
313-
}
314-
node.stateNode = instance;
299+
instance = cloneHiddenInstance(instance, type, props, node);
315300
}
316301
appendChildToContainerChildSet(containerChildSet, instance);
317302
} else if (node.tag === HostText) {
318303
let instance = node.stateNode;
319-
if (needsVisibilityToggle) {
304+
if (needsVisibilityToggle && isHidden) {
305+
// This child is inside a timed out tree. Hide it.
320306
const text = node.memoizedProps;
321-
if (isHidden) {
322-
instance = cloneHiddenTextInstance(instance, text, node);
323-
} else {
324-
instance = cloneUnhiddenTextInstance(instance, text, node);
325-
}
326-
node.stateNode = instance;
307+
instance = cloneHiddenTextInstance(instance, text, node);
327308
}
328309
appendChildToContainerChildSet(containerChildSet, instance);
329310
} else if (node.tag === HostPortal) {
@@ -337,25 +318,22 @@ if (supportsMutation) {
337318
if (newIsHidden) {
338319
const primaryChildParent = node.child;
339320
if (primaryChildParent !== null) {
340-
appendAllChildrenToContainer(
341-
containerChildSet,
342-
primaryChildParent,
343-
true,
344-
newIsHidden,
345-
);
346-
node = primaryChildParent.sibling;
347-
continue;
321+
if (primaryChildParent.child !== null) {
322+
primaryChildParent.child.return = primaryChildParent;
323+
appendAllChildrenToContainer(
324+
containerChildSet,
325+
primaryChildParent,
326+
true,
327+
newIsHidden,
328+
);
329+
}
330+
const fallbackChildParent = primaryChildParent.sibling;
331+
if (fallbackChildParent !== null) {
332+
fallbackChildParent.return = node;
333+
node = fallbackChildParent;
334+
continue;
335+
}
348336
}
349-
} else {
350-
const primaryChildParent = node;
351-
appendAllChildrenToContainer(
352-
containerChildSet,
353-
primaryChildParent,
354-
true,
355-
newIsHidden,
356-
);
357-
// eslint-disable-next-line no-labels
358-
break branches;
359337
}
360338
}
361339
if (node.child !== null) {
@@ -714,11 +692,23 @@ function completeWork(
714692
}
715693
}
716694

717-
if (nextDidTimeout || prevDidTimeout) {
718-
// If the children are hidden, or if they were previous hidden, schedule
719-
// an effect to toggle their visibility. This is also used to attach a
720-
// retry listener to the promise.
721-
workInProgress.effectTag |= Update;
695+
if (supportsPersistence) {
696+
if (nextDidTimeout) {
697+
// If this boundary just timed out, schedule an effect to attach a
698+
// retry listener to the proimse. This flag is also used to hide the
699+
// primary children.
700+
workInProgress.effectTag |= Update;
701+
}
702+
}
703+
if (supportsMutation) {
704+
if (nextDidTimeout || prevDidTimeout) {
705+
// If this boundary just timed out, schedule an effect to attach a
706+
// retry listener to the proimse. This flag is also used to hide the
707+
// primary children. In mutation mode, we also need the flag to
708+
// *unhide* children that were previously hidden, so check if the
709+
// is currently timed out, too.
710+
workInProgress.effectTag |= Update;
711+
}
722712
}
723713
break;
724714
}

packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
let React;
22
let Suspense;
3-
let ReactTestRenderer;
3+
let ReactNoop;
44
let Scheduler;
55
let ReactFeatureFlags;
66
let Random;
@@ -26,7 +26,7 @@ describe('ReactSuspenseFuzz', () => {
2626
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
2727
React = require('react');
2828
Suspense = React.Suspense;
29-
ReactTestRenderer = require('react-test-renderer');
29+
ReactNoop = require('react-noop-renderer');
3030
Scheduler = require('scheduler');
3131
Random = require('random-seed');
3232
});
@@ -143,54 +143,49 @@ describe('ReactSuspenseFuzz', () => {
143143
return resolvedText;
144144
}
145145

146-
function renderToRoot(
147-
root,
148-
children,
149-
{shouldSuspend} = {shouldSuspend: true},
150-
) {
151-
root.update(
152-
<ShouldSuspendContext.Provider value={shouldSuspend}>
153-
{children}
154-
</ShouldSuspendContext.Provider>,
155-
);
146+
function resolveAllTasks() {
156147
Scheduler.unstable_flushWithoutYielding();
157-
158148
let elapsedTime = 0;
159149
while (pendingTasks && pendingTasks.size > 0) {
160150
if ((elapsedTime += 1000) > 1000000) {
161151
throw new Error('Something did not resolve properly.');
162152
}
163-
ReactTestRenderer.act(() => jest.advanceTimersByTime(1000));
153+
ReactNoop.act(() => jest.advanceTimersByTime(1000));
164154
Scheduler.unstable_flushWithoutYielding();
165155
}
166-
167-
return root.toJSON();
168156
}
169157

170158
function testResolvedOutput(unwrappedChildren) {
171159
const children = (
172160
<Suspense fallback="Loading...">{unwrappedChildren}</Suspense>
173161
);
174162

175-
const expectedRoot = ReactTestRenderer.create(null);
176-
const expectedOutput = renderToRoot(expectedRoot, children, {
177-
shouldSuspend: false,
178-
});
179-
expectedRoot.unmount();
163+
resetCache();
164+
ReactNoop.renderToRootWithID(
165+
<ShouldSuspendContext.Provider value={false}>
166+
{children}
167+
</ShouldSuspendContext.Provider>,
168+
'expected',
169+
);
170+
resolveAllTasks();
171+
const expectedOutput = ReactNoop.getChildrenAsJSX('expected');
172+
ReactNoop.renderToRootWithID(null, 'expected');
173+
Scheduler.unstable_flushWithoutYielding();
180174

181175
resetCache();
182-
const syncRoot = ReactTestRenderer.create(null);
183-
const syncOutput = renderToRoot(syncRoot, children);
176+
ReactNoop.renderLegacySyncRoot(children);
177+
resolveAllTasks();
178+
const syncOutput = ReactNoop.getChildrenAsJSX();
184179
expect(syncOutput).toEqual(expectedOutput);
185-
syncRoot.unmount();
180+
ReactNoop.renderLegacySyncRoot(null);
186181

187182
resetCache();
188-
const concurrentRoot = ReactTestRenderer.create(null, {
189-
unstable_isConcurrent: true,
190-
});
191-
const concurrentOutput = renderToRoot(concurrentRoot, children);
183+
ReactNoop.renderToRootWithID(children, 'concurrent');
184+
Scheduler.unstable_flushWithoutYielding();
185+
resolveAllTasks();
186+
const concurrentOutput = ReactNoop.getChildrenAsJSX('concurrent');
192187
expect(concurrentOutput).toEqual(expectedOutput);
193-
concurrentRoot.unmount();
188+
ReactNoop.renderToRootWithID(null, 'concurrent');
194189
Scheduler.unstable_flushWithoutYielding();
195190
}
196191

0 commit comments

Comments
 (0)