Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add back root override for strict mode #21428

Merged
merged 3 commits into from May 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Expand Up @@ -102,6 +102,20 @@ describe('ReactTestUtils.act()', () => {
root.render(<App />);
Scheduler.unstable_flushAll();
});

// @gate experimental
it('warns in concurrent mode if root is strict', () => {
expect(() => {
const root = ReactDOM.unstable_createRoot(
document.createElement('div'),
{unstable_strictMode: true},
);
root.render(<App />);
Scheduler.unstable_flushAll();
}).toErrorDev([
'An update to App ran an effect, but was not wrapped in act(...)',
]);
});
});
});

Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/client/ReactDOMRoot.js
Expand Up @@ -27,6 +27,7 @@ export type RootOptions = {
mutableSources?: Array<MutableSource<any>>,
...
},
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
...
};
Expand Down Expand Up @@ -122,6 +123,7 @@ function createRootImpl(
options.hydrationOptions != null &&
options.hydrationOptions.mutableSources) ||
null;
const isStrictMode = options != null && options.unstable_strictMode === true;

let concurrentUpdatesByDefaultOverride = null;
if (allowConcurrentByDefault) {
Expand All @@ -136,6 +138,7 @@ function createRootImpl(
tag,
hydrate,
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
);
markContainerAsRoot(root.current, container);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-renderer/src/ReactFabric.js
Expand Up @@ -207,7 +207,7 @@ function render(
if (!root) {
// TODO (bvaughn): If we decide to keep the wrapper component,
// We could create a wrapper for containerTag as well to reduce special casing.
root = createContainer(containerTag, LegacyRoot, false, null, null);
root = createContainer(containerTag, LegacyRoot, false, null, false, null);
roots.set(containerTag, root);
}
updateContainer(element, root, null, callback);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
Expand Up @@ -203,7 +203,7 @@ function render(
if (!root) {
// TODO (bvaughn): If we decide to keep the wrapper component,
// We could create a wrapper for containerTag as well to reduce special casing.
root = createContainer(containerTag, LegacyRoot, false, null, null);
root = createContainer(containerTag, LegacyRoot, false, null, false, null);
roots.set(containerTag, root);
}
updateContainer(element, root, null, callback);
Expand Down
4 changes: 3 additions & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Expand Up @@ -722,7 +722,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (!root) {
const container = {rootID: rootID, pendingChildren: [], children: []};
rootContainers.set(rootID, container);
root = NoopRenderer.createContainer(container, tag, false, null);
root = NoopRenderer.createContainer(container, tag, false, null, null);
roots.set(rootID, root);
}
return root.current.stateNode.containerInfo;
Expand All @@ -740,6 +740,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
ConcurrentRoot,
false,
null,
null,
);
return {
_Scheduler: Scheduler,
Expand All @@ -766,6 +767,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
LegacyRoot,
false,
null,
null,
);
return {
_Scheduler: Scheduler,
Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiber.new.js
Expand Up @@ -422,12 +422,19 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {

export function createHostRootFiber(
tag: RootTag,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): Fiber {
let mode;
if (tag === ConcurrentRoot) {
mode = ConcurrentMode;
if (enableStrictEffects && createRootStrictEffectsByDefault) {
if (isStrictMode === true) {
mode |= StrictLegacyMode;

if (enableStrictEffects) {
mode |= StrictEffectsMode;
}
} else if (enableStrictEffects && createRootStrictEffectsByDefault) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete this after some internal clean ups.

mode |= StrictLegacyMode | StrictEffectsMode;
}
if (
Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/ReactFiber.old.js
Expand Up @@ -422,12 +422,19 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {

export function createHostRootFiber(
tag: RootTag,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): Fiber {
let mode;
if (tag === ConcurrentRoot) {
mode = ConcurrentMode;
if (enableStrictEffects && createRootStrictEffectsByDefault) {
if (isStrictMode === true) {
mode |= StrictLegacyMode;

if (enableStrictEffects) {
mode |= StrictEffectsMode;
}
} else if (enableStrictEffects && createRootStrictEffectsByDefault) {
mode |= StrictLegacyMode | StrictEffectsMode;
}
if (
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
Expand Up @@ -248,13 +248,15 @@ export function createContainer(
tag: RootTag,
hydrate: boolean,
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
tag,
hydrate,
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.old.js
Expand Up @@ -248,13 +248,15 @@ export function createContainer(
tag: RootTag,
hydrate: boolean,
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
tag,
hydrate,
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberRoot.new.js
Expand Up @@ -98,6 +98,7 @@ export function createFiberRoot(
tag: RootTag,
hydrate: boolean,
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): FiberRoot {
const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any);
Expand All @@ -109,6 +110,7 @@ export function createFiberRoot(
// stateNode is any.
const uninitializedFiber = createHostRootFiber(
tag,
isStrictMode,
concurrentUpdatesByDefaultOverride,
);
root.current = uninitializedFiber;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberRoot.old.js
Expand Up @@ -98,6 +98,7 @@ export function createFiberRoot(
tag: RootTag,
hydrate: boolean,
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
): FiberRoot {
const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any);
Expand All @@ -109,6 +110,7 @@ export function createFiberRoot(
// stateNode is any.
const uninitializedFiber = createHostRootFiber(
tag,
isStrictMode,
concurrentUpdatesByDefaultOverride,
);
root.current = uninitializedFiber;
Expand Down
6 changes: 6 additions & 0 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Expand Up @@ -58,6 +58,7 @@ const {IsSomeRendererActing} = ReactSharedInternals;
type TestRendererOptions = {
createNodeMock: (element: React$Element<any>) => any,
unstable_isConcurrent: boolean,
unstable_strictMode: boolean,
unstable_concurrentUpdatesByDefault: boolean,
...
};
Expand Down Expand Up @@ -436,6 +437,7 @@ function propsMatch(props: Object, filter: Object): boolean {
function create(element: React$Element<any>, options: TestRendererOptions) {
let createNodeMock = defaultTestOptions.createNodeMock;
let isConcurrent = false;
let isStrictMode = false;
let concurrentUpdatesByDefault = null;
if (typeof options === 'object' && options !== null) {
if (typeof options.createNodeMock === 'function') {
Expand All @@ -444,6 +446,9 @@ function create(element: React$Element<any>, options: TestRendererOptions) {
if (options.unstable_isConcurrent === true) {
isConcurrent = true;
}
if (options.unstable_strictMode === true) {
isStrictMode = true;
}
if (allowConcurrentByDefault) {
if (options.unstable_concurrentUpdatesByDefault !== undefined) {
concurrentUpdatesByDefault =
Expand All @@ -461,6 +466,7 @@ function create(element: React$Element<any>, options: TestRendererOptions) {
isConcurrent ? ConcurrentRoot : LegacyRoot,
false,
null,
isStrictMode,
concurrentUpdatesByDefault,
);
invariant(root != null, 'something went wrong');
Expand Down
22 changes: 22 additions & 0 deletions packages/react/src/__tests__/ReactStrictMode-test.internal.js
Expand Up @@ -65,6 +65,28 @@ describe('ReactStrictMode', () => {
});

if (__DEV__) {
// @gate experimental
it('should support enabling strict mode via createRoot option', () => {
act(() => {
const container = document.createElement('div');
const root = ReactDOM.createRoot(container, {
unstable_strictMode: true,
});
root.render(<Component label="A" />);
});

expect(log).toEqual([
'A: render',
'A: render',
'A: useLayoutEffect mount',
'A: useEffect mount',
'A: useLayoutEffect unmount',
'A: useEffect unmount',
'A: useLayoutEffect mount',
'A: useEffect mount',
]);
});

// @gate experimental
it('should include legacy + strict effects mode', () => {
act(() => {
Expand Down