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.

Sign up for GitHub

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

Jump to bottom

Cleanup enableUseRefAccessWarning flag #28699

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Cleanup enableUseRefAccessWarning flag #28699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 3 additions & 83 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
enableDebugTracing,
enableSchedulingProfiler,
enableCache,
enableUseRefAccessWarning,
enableLazyContextPropagation,
enableTransitionTracing,
enableUseMemoCacheHook,
Expand Down Expand Up @@ -2330,90 +2329,11 @@ function createEffectInstance(): EffectInstance {
return {destroy: undefined};
}

let stackContainsErrorMessage: boolean | null = null;

function getCallerStackFrame(): string {
// eslint-disable-next-line react-internal/prod-error-codes
const stackFrames = new Error('Error message').stack.split('\n');

// Some browsers (e.g. Chrome) include the error message in the stack
// but others (e.g. Firefox) do not.
if (stackContainsErrorMessage === null) {
stackContainsErrorMessage = stackFrames[0].includes('Error message');
}

return stackContainsErrorMessage
? stackFrames.slice(3, 4).join('\n')
: stackFrames.slice(2, 3).join('\n');
}

function mountRef<T>(initialValue: T): {current: T} {
const hook = mountWorkInProgressHook();
if (enableUseRefAccessWarning) {
if (__DEV__) {
// Support lazy initialization pattern shown in docs.
// We need to store the caller stack frame so that we don't warn on subsequent renders.
let hasBeenInitialized = initialValue != null;
let lazyInitGetterStack = null;
let didCheckForLazyInit = false;

// Only warn once per component+hook.
let didWarnAboutRead = false;
let didWarnAboutWrite = false;

let current = initialValue;
const ref = {
get current() {
if (!hasBeenInitialized) {
didCheckForLazyInit = true;
lazyInitGetterStack = getCallerStackFrame();
} else if (currentlyRenderingFiber !== null && !didWarnAboutRead) {
if (
lazyInitGetterStack === null ||
lazyInitGetterStack !== getCallerStackFrame()
) {
didWarnAboutRead = true;
console.warn(
'%s: Unsafe read of a mutable value during render.\n\n' +
'Reading from a ref during render is only safe if:\n' +
'1. The ref value has not been updated, or\n' +
'2. The ref holds a lazily-initialized value that is only set once.\n',
getComponentNameFromFiber(currentlyRenderingFiber) || 'Unknown',
);
}
}
return current;
},
set current(value: any) {
if (currentlyRenderingFiber !== null && !didWarnAboutWrite) {
if (hasBeenInitialized || !didCheckForLazyInit) {
didWarnAboutWrite = true;
console.warn(
'%s: Unsafe write of a mutable value during render.\n\n' +
'Writing to a ref during render is only safe if the ref holds ' +
'a lazily-initialized value that is only set once.\n',
getComponentNameFromFiber(currentlyRenderingFiber) || 'Unknown',
);
}
}

hasBeenInitialized = true;
current = value;
},
};
Object.seal(ref);
hook.memoizedState = ref;
return ref;
} else {
const ref = {current: initialValue};
hook.memoizedState = ref;
return ref;
}
} else {
const ref = {current: initialValue};
hook.memoizedState = ref;
return ref;
}
const ref = {current: initialValue};
hook.memoizedState = ref;
return ref;
}

function updateRef<T>(initialValue: T): {current: T} {
Expand Down
125 changes: 0 additions & 125 deletions packages/react-reconciler/src/__tests__/useRef-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,24 +160,6 @@ describe('useRef', () => {
});
});

// @gate enableUseRefAccessWarning
it('should warn about reads during render', async () => {
function Example() {
const ref = useRef(123);
let value;
expect(() => {
value = ref.current;
}).toWarnDev([
'Example: Unsafe read of a mutable value during render.',
]);
return value;
}

await act(() => {
ReactNoop.render(<Example />);
});
});

it('should not warn about lazy init during render', async () => {
function Example() {
const ref1 = useRef(null);
Expand Down Expand Up @@ -221,113 +203,6 @@ describe('useRef', () => {
});
});

// @gate enableUseRefAccessWarning
it('should warn about unconditional lazy init during render', async () => {
function Example() {
const ref1 = useRef(null);
const ref2 = useRef(undefined);

if (shouldExpectWarning) {
expect(() => {
ref1.current = 123;
}).toWarnDev([
'Example: Unsafe write of a mutable value during render',
]);
expect(() => {
ref2.current = 123;
}).toWarnDev([
'Example: Unsafe write of a mutable value during render',
]);
} else {
ref1.current = 123;
ref1.current = 123;
}

// But only warn once
ref1.current = 345;
ref1.current = 345;

return null;
}

let shouldExpectWarning = true;
await act(() => {
ReactNoop.render(<Example />);
});

// Should not warn again on update.
shouldExpectWarning = false;
await act(() => {
ReactNoop.render(<Example />);
});
});

// @gate enableUseRefAccessWarning
it('should warn about reads to ref after lazy init pattern', async () => {
function Example() {
const ref1 = useRef(null);
const ref2 = useRef(undefined);

// Read 1: safe because lazy init:
if (ref1.current === null) {
ref1.current = 123;
}
if (ref2.current === undefined) {
ref2.current = 123;
}

let value;
expect(() => {
value = ref1.current;
}).toWarnDev(['Example: Unsafe read of a mutable value during render']);
expect(() => {
value = ref2.current;
}).toWarnDev(['Example: Unsafe read of a mutable value during render']);

// But it should only warn once.
value = ref1.current;
value = ref2.current;

return value;
}

await act(() => {
ReactNoop.render(<Example />);
});
});

// @gate enableUseRefAccessWarning
it('should warn about writes to ref after lazy init pattern', async () => {
function Example() {
const ref1 = useRef(null);
const ref2 = useRef(undefined);
// Read: safe because lazy init:
if (ref1.current === null) {
ref1.current = 123;
}
if (ref2.current === undefined) {
ref2.current = 123;
}

expect(() => {
ref1.current = 456;
}).toWarnDev([
'Example: Unsafe write of a mutable value during render',
]);
expect(() => {
ref2.current = 456;
}).toWarnDev([
'Example: Unsafe write of a mutable value during render',
]);

return null;
}

await act(() => {
ReactNoop.render(<Example />);
});
});

it('should not warn about reads or writes within effect', async () => {
function Example() {
const ref = useRef(123);
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/ReactFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ export const enableRenderableContext = true;
// when we plan to enable them.
// -----------------------------------------------------------------------------

export const enableUseRefAccessWarning = false;

// Enables time slicing for updates that aren't wrapped in startTransition.
export const forceConcurrentByDefaultForTesting = false;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,5 @@ export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
export const enableInfiniteRenderLoopDetection = __VARIANT__;
export const enableRenderableContext = __VARIANT__;
export const enableUnifiedSyncLane = __VARIANT__;
export const enableUseRefAccessWarning = __VARIANT__;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
export const useModernStrictMode = __VARIANT__;
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export const {
enableInfiniteRenderLoopDetection,
enableRenderableContext,
enableUnifiedSyncLane,
enableUseRefAccessWarning,
passChildrenWhenCloningPersistedNodes,
useModernStrictMode,
} = dynamicFlags;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export const enableRetryLaneExpiration = false;
export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
export const transitionLaneExpirationMs = 5000;
export const enableUseRefAccessWarning = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
export const transitionLaneExpirationMs = 5000;

export const enableUseRefAccessWarning = false;

export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export const enableCPUSuspense = true;
export const enableUseMemoCacheHook = true;
export const enableUseEffectEventHook = false;
export const favorSafetyOverHydrationPerf = true;
export const enableUseRefAccessWarning = false;
export const enableInfiniteRenderLoopDetection = false;
export const enableRenderableContext = false;

Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
export const transitionLaneExpirationMs = 5000;

export const enableUseRefAccessWarning = false;

export const disableSchedulerTimeoutInWorkLoop = false;
export const enableLazyContextPropagation = false;
export const enableLegacyHidden = false;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// with the __VARIANT__ set to `true`, and once set to `false`.

export const disableIEWorkarounds = __VARIANT__;
export const enableUseRefAccessWarning = __VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableLazyContextPropagation = __VARIANT__;
export const forceConcurrentByDefaultForTesting = __VARIANT__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const {
disableIEWorkarounds,
enableTrustedTypesIntegration,
enableDebugTracing,
enableUseRefAccessWarning,
enableLazyContextPropagation,
enableUnifiedSyncLane,
enableRetryLaneExpiration,
Expand Down
1 change: 0 additions & 1 deletion packages/use-sync-external-store/src/__tests__/useSyncExternalStoreNative-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ describe('useSyncExternalStore (userspace shim, server rendering)', () => {
expect(root).toMatchRenderedOutput('client');
});

// @gate !(enableUseRefAccessWarning && __DEV__)
test('Using isEqual to bailout', async () => {
const store = createExternalStore({a: 0, b: 0});

Expand Down
Loading