Skip to content

Commit

Permalink
Cleanup enableUseRefAccessWarning flag (#28699)
Browse files Browse the repository at this point in the history 
Cleanup enableUseRefAccessWarning flag

I don't think this flag has a path forward in the current
implementation. The detection by stack trace is too brittle to detect
the lazy initialization pattern reliably (see e.g. some internal tests
that expect the warning because they use lazy intialization, but a
slightly different pattern then the expected pattern.

I think a new version of this could be to fully ban ref access during
render with an alternative API for the exceptional cases that today
require ref access during render.

DiffTrain build for [20e710a](20e710a)
  • Loading branch information
@kassens
kassens committed Apr 3, 2024
1 parent a491103 commit ce3ba22
Show file tree
Hide file tree
Showing 18 changed files with 1,298 additions and 1,852 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3761acb42bf9979314fff130d4d9505408bcb651
20e710aeab3e03809c82d134171986ea270026a0
95 changes: 6 additions & 89 deletions compiled/facebook-www/ReactART-dev.classic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "19.0.0-www-classic-f095b08b";
var ReactVersion = "19.0.0-www-classic-fce3a147";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -164,7 +164,6 @@ if (__DEV__) {
var dynamicFeatureFlags = require("ReactFeatureFlags");

var enableDebugTracing = dynamicFeatureFlags.enableDebugTracing,
enableUseRefAccessWarning = dynamicFeatureFlags.enableUseRefAccessWarning,
enableLazyContextPropagation =
dynamicFeatureFlags.enableLazyContextPropagation,
enableUnifiedSyncLane = dynamicFeatureFlags.enableUnifiedSyncLane,
Expand Down Expand Up @@ -10874,95 +10873,13 @@ if (__DEV__) {
};
}

var stackContainsErrorMessage = null;

function getCallerStackFrame() {
// eslint-disable-next-line react-internal/prod-error-codes
var 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(initialValue) {
var hook = mountWorkInProgressHook();

if (enableUseRefAccessWarning) {
{
// Support lazy initialization pattern shown in docs.
// We need to store the caller stack frame so that we don't warn on subsequent renders.
var hasBeenInitialized = initialValue != null;
var lazyInitGetterStack = null;
var didCheckForLazyInit = false; // Only warn once per component+hook.

var didWarnAboutRead = false;
var didWarnAboutWrite = false;
var current = initialValue;
var ref = {
get current() {
if (!hasBeenInitialized) {
didCheckForLazyInit = true;
lazyInitGetterStack = getCallerStackFrame();
} else if (
currentlyRenderingFiber$1 !== null &&
!didWarnAboutRead
) {
if (
lazyInitGetterStack === null ||
lazyInitGetterStack !== getCallerStackFrame()
) {
didWarnAboutRead = true;

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$1) ||
"Unknown"
);
}
}

return current;
},

set current(value) {
if (currentlyRenderingFiber$1 !== null && !didWarnAboutWrite) {
if (hasBeenInitialized || !didCheckForLazyInit) {
didWarnAboutWrite = true;

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$1) ||
"Unknown"
);
}
}

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

function updateRef(initialValue) {
Expand Down
95 changes: 6 additions & 89 deletions compiled/facebook-www/ReactART-dev.modern.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "19.0.0-www-modern-5c16ebb5";
var ReactVersion = "19.0.0-www-modern-8336910b";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -164,7 +164,6 @@ if (__DEV__) {
var dynamicFeatureFlags = require("ReactFeatureFlags");

var enableDebugTracing = dynamicFeatureFlags.enableDebugTracing,
enableUseRefAccessWarning = dynamicFeatureFlags.enableUseRefAccessWarning,
enableLazyContextPropagation =
dynamicFeatureFlags.enableLazyContextPropagation,
enableUnifiedSyncLane = dynamicFeatureFlags.enableUnifiedSyncLane,
Expand Down Expand Up @@ -10625,95 +10624,13 @@ if (__DEV__) {
};
}

var stackContainsErrorMessage = null;

function getCallerStackFrame() {
// eslint-disable-next-line react-internal/prod-error-codes
var 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(initialValue) {
var hook = mountWorkInProgressHook();

if (enableUseRefAccessWarning) {
{
// Support lazy initialization pattern shown in docs.
// We need to store the caller stack frame so that we don't warn on subsequent renders.
var hasBeenInitialized = initialValue != null;
var lazyInitGetterStack = null;
var didCheckForLazyInit = false; // Only warn once per component+hook.

var didWarnAboutRead = false;
var didWarnAboutWrite = false;
var current = initialValue;
var ref = {
get current() {
if (!hasBeenInitialized) {
didCheckForLazyInit = true;
lazyInitGetterStack = getCallerStackFrame();
} else if (
currentlyRenderingFiber$1 !== null &&
!didWarnAboutRead
) {
if (
lazyInitGetterStack === null ||
lazyInitGetterStack !== getCallerStackFrame()
) {
didWarnAboutRead = true;

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$1) ||
"Unknown"
);
}
}

return current;
},

set current(value) {
if (currentlyRenderingFiber$1 !== null && !didWarnAboutWrite) {
if (hasBeenInitialized || !didCheckForLazyInit) {
didWarnAboutWrite = true;

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$1) ||
"Unknown"
);
}
}

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

function updateRef(initialValue) {
Expand Down
Loading

0 comments on commit ce3ba22

Please sign in to comment.