Mr#36364
Closed
modeysamuel-lab wants to merge 846 commits intofacebook:pr33760from
Closed
Conversation
…ook#35284) Fixes an edge case where a function expression would fail to take a dependency if it referenced a hoisted `const` inferred as a primitive value. We were incorrectly skipping primitve-typed operands when determing scopes for merging in InferReactiveScopeVariables. This was super tricky to debug, for posterity the trick is that Context variables (StoreContext etc) are modeled just like a mutable object, where assignment to the variable is equivalent to `object.value = ...` and reading the variable is equivalent to `object.value` property access. Comparing to an equivalent version of the repro case replaced with an object and property read/writes showed that everything was exactly right, except that InferReactiveScopeVariables wasn't merging the scopes of the function and the context variable, which led me right to the problematic line. Closes facebook#35122
facebook#35285) Alternative approach to facebook#35282 for validating effect deps in the compiler that builds on the machinery in ValidateExhaustiveDependencies. Key changes to that pass: * Refactor to track the dependencies of array expressions as temporaries so we can look them up later if they appear as effect deps. * Instead of not storing temporaries for LoadLocals of locally created variables, we store the temporary but also propagate the local-ness through. This allows us to record deps at the top level, necessary for effect deps. Previously the pass was only ever concerned with tracking deps within function expressions. * Refactor the bulk of the dependency-checking logic from `onFinishMemoize()` into a standalone helper to use it for the new `onEffect()` helper as well. * Add a new ErrorCategory for effect deps, use it for errors on effects * Put the effect dep validation behind a feature flag * Adjust the error reason for effect errors --------- Co-authored-by: Jack Pope <jackpope1@gmail.com>
…user/agent (facebook#35306) The current `validateNoSetStateInEffects` error has potential false positives because we cannot fully statically detect patterns where calling setState in an effect is actually valid. This flag `enableVerboseNoSetStateInEffect` adds a verbose error mode that presents multiple possible use-cases, allowing an agent to reason about which fix is appropriate before acting: 1. Non-local derived data - suggests restructuring state ownership 2. Derived event pattern - suggests requesting an event callback from parent 3. Force update / external sync - suggests using `useSyncExternalStore` This gives agents the context needed to make informed decisions rather than blindly applying a fix that may not be correct for the specific situation.
Was bumped to a canary in facebook#34499 which got never released as stable. Presumeably to use `Activity` which only made it into Activity in later Next.js releases. However, `Activity` never ended up being used due to incompatibilities with Monaco Editor. Downgrading should be safe. Downgrading to fix GHSA-9qr9-h5gf-34mp. This will allow new deploys since Vercel is currently blocking new deploys of unsafe version --------- Co-authored-by: Eugene Choi <4eugenechoi@gmail.com>
Speculative fix to facebook#35336 written by Claude. I have verified that applying a similar patch locally to the repro from facebook#35336 does fix the crash. I'm not familiar enough with the underlying APIs to tell whether the fix is correct or sufficient.
…facebook#35338) Temporarily enables these 2 flags for internal testing.
## Summary Add keyboard shortcuts (Cmd/Ctrl + Left/Right arrow keys) to navigate between commits in the Profiler's snapshot view. Moved `filteredCommitIndices` management and commit navigation logic (`selectNextCommitIndex`, `selectPrevCommitIndex`) from `SnapshotSelector` into `useCommitFilteringAndNavigation` used by `ProfilerContext` to enable keyboard shortcuts from the top-level Profiler component. ## How did you test this change? - New tests in ProfilerContext-tests - Built browser extension: `yarn build:<browser name>` - tested in browser: `yarn run test:<browser name>` - Manually verified Left/Right arrow navigation cycles through commits - Verified navigation respects commit duration filter - Verified reload-and-profile button unaffected Chrome: https://github.com/user-attachments/assets/01d2a749-13dc-4d08-8bcb-3d4d45a5f97c Edge with duration filter: https://github.com/user-attachments/assets/a7f76ff7-2a0b-4b9c-a0ce-d4449373308b firefox mixing hotkey with clicking arrow buttons: https://github.com/user-attachments/assets/48912d68-7c75-40f2-a203-5e6d7e6b2d99
…acebook#35341) Continue attaching `internalInstanceKey` to DOM nodes in DEV. This prevents breaking some internal dev tooling while we experiment with the broader change. Note that this does not reference the DOM handle within the flag, just attaches it and deletes it. Internals tracking is still done through the private map.
…ed (facebook#35294) Follow-up to facebook#34653. React Native doesn't implement `getClientRect`, since this is applicable to CSS box, which is not a concept for Native (maybe yet). I am loosening the condition that gates `showOverlay()` call to pass if `getClientRect` is not implemented. Conceptually, everything that is inside `react-devtools-shared/backend` should be Host-agnostic, because both on Web and Native it is installed inside the Host JavaScript runtime, be it main frame of the page, or RN instance. Since overlay & highlighting logic also lives there, it should also follow these principles.
…35293) `Error.prepareStackTrace` is non-standard feature and not all JavaScript runtimes implement the methods that we are using in React DevTools backend. This PR adds additional checks for the presence of the methods that we are using.
…eference (facebook#35343) Follow-up to facebook#35296. We can get `ReferenceError` if this is unavailable. Using `typeof` check instead for safety.
### What Fixes source locations for VariableDeclarator in the generated AST. Fixes a number of the errors in the snapshot I added yesterday in the source loc validator PR facebook#35109 I'm not entirely sure why, but a side effect of the fix has resulted in a ton of snaps needing updating, with some empty lines no longer present in the generated output. I broke the change up into 2 separate commits. The [first commit](facebook@f4e4dc0) has the core change and the update to the missing source locations test expectation, and the [second commit](facebook@cd4d9e9) has the rest of the snapshot updates. ### How - Add location for variable declarators in ast codegen. - We don't actually have the location preserved in HIR, since when we lower the declarations we pass through the location for the VariableDeclaration. Since VariableDeclarator is just a container for each of the assignments, the start of the `id` and end of the `init` can be used to accurately reconstruct it when generating the AST. - Add source locations for object/array patterns for destructuring assignment source location support
…ook#35345) Server Functions can be stringified (sometimes implicitly) when passed as data. This adds an override to hide the source code in that case - just in case someone puts sensitive information in there. Note that this still preserves the `name` field but this is also available on the export but in practice is likely minified anyway. There's nothing else on these referenes we'd consider unsafe unless you explicitly expose expandos which are part of the `"use server"` export. This adds a safety check to ensure you don't encode cyclic Promises. This isn't a parser bug per se. Promises do have a safety mechanism that avoids them infinite looping. However, since we use custom Thenables, what can happen is that every time a native Promise awaits it, another Promise wrapper is created around the Thenable which foils the ECMAScript Promise cycle detection which can lead to an infinite loop. This also ensures that embedded `ReadableStream` and `AsyncIterable` streams are properly closed if the source stream closes early both on the Server and Client. This doesn't cause an infinite loop but just to make sure resource clean up can proceed properly. We're also adding some more explicit clear errors for invalid payloads since we no longer need to obfuscate the original issue.
…5129)" (facebook#35346) This broke main.
In case we get into loops.
Putting up facebook#35129 again Reverted in facebook#35346 after breaking main before security patch This change impacts output formatting in a lot of snaps, so is very sensitive to additions in main to the fixtures resulting in broken tests after merging, so we should try merge quickly after rebasing or do a fast follow to the merge with a snap update.
…nt' (facebook#35216) Summary: These validations are not essential for compilation, with this we only run that logic when outputMode is 'lint' Test Plan: Update fixtures and run tests
Upgrading due to CVE-2025-55183 and CVE-2025-67779
<!-- 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). --> ## Summary Currently, every second console log is dimmed, receiving a special style that indicates to user that it was raising because of [React Strict Mode](https://react.dev/reference/react/StrictMode) second rendering. This introduces a setting to disable this. ## How did you test this change? Test in console-test.js https://github.com/user-attachments/assets/af6663ac-f79b-4824-95c0-d46b0c8dec12 Browser extension react devtools https://github.com/user-attachments/assets/7e2ecb7a-fbdf-4c72-ab45-7e3a1c6e5e44 React native dev tools: https://github.com/user-attachments/assets/d875b3ac-1f27-43f8-8d9d-12b2d65fa6e6 --------- Co-authored-by: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com>
…d temporarily enable (facebook#35365) `react-hooks/exhaustive-effect-dependencies` from `ValidateExhaustiveDeps` reports errors for both missing and extra effect deps. We already have `react-hooks/exhaustive-deps` that errors on missing dependencies. In the future we'd like to consolidate this all to the compiler based error, but for now there's a lot of overlap. Let's enable testing the extra dep warning by splitting out reporting modes. This PR - Creates `on`, `off`, `missing-only`, and `extra-only` reporting modes for the effect dep validation flag - Temporarily enables the new rule with `extra-only` in `eslint-plugin-react-hooks` - Adds additional null checking to `manualMemoLoc` to fix a bug found when running against the fixture
…#35369) ## Summary This PR improves cyclic thenable detection in `ReactFlightReplyServer.js`. Fixes facebook#35368. The previous fix only detected direct self-references (`inspectedValue === chunk`) and relied on the `cycleProtection` counter to eventually bail out of longer cycles. This change keeps the existing MAX_THENABLE_CYCLE_DEPTH ($1000$) `cycleProtection` cap as a hard guardrail and adds a visited set so that we can detect self-cycles and multi-node cycles as soon as any `ReactPromise` is revisited and while still bounding the amount of work we do for deep acyclic chains via `cycleProtection`. ## How did you test this change? - Ran the existing test suite for the server renderer: ```bash yarn test react-server yarn test --prod react-server yarn flow dom-node yarn linc ``` --------- Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
…ok#35380) When the Fizz runtime runs a view-transition we apply `view-transition-name` and `view-transition-class` to the `style`. These can be observed by Fiber when hydrating which incorrectly leads to hydration errors. More over, even after we remove them, the `style` attribute has now been normalized which we are unable to diff because we diff against the SSR generated `style` attribute string and not the normalized form. So if there are other inline styles defined, we have to skip diffing them in this scenario.
…ok#35347) We already had tests for cyclic objects, but not for cyclic arrays.
facebook#35457) DevTools has ~45 test files which don't distribute well across 10 shards, causing shard 3 to run 2x slower than others (104s vs ~50s). This moves DevTools build tests to a separate job with 3 shards for better load balancing.
[ci] Increase DevTools test shards and bump timeout - Increase DevTools test shards from 3 to 5 - Bump timeout to 20s --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35459). * facebook#35458 * __->__ facebook#35459
…default param (facebook#36107) ## Summary Fixes facebook#36101 When a component function has a destructured prop with a `NewExpression` default value (e.g. `{ value = new Number() }`), the React Compiler bails out during HIR construction when trying to lower the default value via `lowerReorderableExpression`. This causes `validateNoSetStateInEffects` to never run, silently suppressing the `set-state-in-effect` diagnostic. **Root cause:** `isReorderableExpression` did not have a case for `NewExpression`, so it fell through to the `default: return false` branch. `lowerReorderableExpression` then recorded a `Todo` error and aborted compilation of the function before any validation passes ran. **Fix:** Add a `NewExpression` case to `isReorderableExpression` that mirrors the existing `CallExpression` case — the expression is safe to reorder when the callee and all arguments are themselves reorderable (e.g. global identifiers and literals). ## How did you test this change? Added a new compiler fixture `invalid-setState-in-useEffect-new-expression-default-param` that reproduces the bug from the issue. The fixture verifies that the `EffectSetState` diagnostic is correctly emitted for a component with a `NewExpression` default prop value. All 1720 compiler snapshot tests pass.
We use FB_WWW bundle to inject internal feature flag values, but need to use NODE guard type because this is a node script -- __DEV__ is breaking internal builds Follow up to facebook#35951
…ld (facebook#36243) PR facebook#35951 added FB_WWW_DEV builds for eslint-plugin-react-hooks to get www-specific feature flag values. However, the FB_WWW build uses the full ReactFeatureFlags.www.js fork, which contains: const dynamicFeatureFlags = require('ReactFeatureFlags'); This is a www Haste module that only exists in the www runtime. Rollup can't tree-shake CJS require() calls (they're assumed side-effectful), so the bare require('ReactFeatureFlags') survives in the build output even though the eslint plugin only uses the static eprh_* exports. When the built artifact is synced to www at scripts/lint/eslint/rules/eslint-plugin-react-hooks/index.js, Node.js fails with "Cannot find module 'ReactFeatureFlags'" because Haste modules aren't available in the Node.js lint environment. Create a dedicated fork (ReactFeatureFlags.eslint-plugin.www.js) that exports only the static eprh_* flags with www values, without the require('ReactFeatureFlags') dependency. Wire it up in forks.js for the eslint-plugin-react-hooks entry point. <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> Co-authored-by: Eugene Choi <eugenechoi@meta.com>
…initeRenderLoopDetection (facebook#36195) My change in facebook#35999 did not cover all possible scenarios for emitting a warning, instead of throwing. The instrumentation not only enables the identification for the infinite loop via execution context checks, but also adds the check to more lifecycle methods, like `markRootPinged` and `markRootUpdated`. See the newly added test to understand a potential scenario. Before the fix, the error would be thrown: <img width="1192" height="424" alt="Screenshot 2026-04-08 at 17 21 51" src="https://github.com/user-attachments/assets/ba8ea379-0271-4938-ae45-e37ee75e1963" /> With the current changes, the warning is logged with `console.error`.
…#36253) ## Summary Set up the experiment to migrate event dispatching in the React Native renderer to be based on the native EventTarget API. Behind the `enableNativeEventTargetEventDispatching` flag, events are dispatched through `dispatchTrustedEvent` instead of the legacy plugin system. Regular event handler props are NOT registered via addEventListener at commit time. Instead, a hook on EventTarget (`EVENT_TARGET_GET_DECLARATIVE_LISTENER_KEY`) extracts handlers from `canonical.currentProps` at dispatch time, shifting cost from every render to only when events fire. The hook is overridden in ReactNativeElement to look up the prop name via a reverse mapping from event names (built lazily from the view config registry). Responder events bypass EventTarget entirely. `negotiateResponder` walks the fiber tree directly (capture then bubble phase), calling handlers from `canonical.currentProps` and checking return values inline. Lifecycle events (`responderGrant`, `responderMove`, etc.) call handlers directly from props and inspect return values — `onResponderGrant` returning `true` blocks native responder, `onResponderTerminationRequest` returning `false` refuses termination. This eliminates all commit-time cost for responder events (no wrappers, no addEventListener, no `responderWrappers` on canonical). ## How did you test this change? Flow Tested e2e in RN using Fantom tests (that will land after this).
## Summary We found a bug in the logic in facebook#36253 and we realized it's very inconvenient to iterate on the implementation when it's in this repository, as we're forced to then synchronize it to RN to test changes. This moves the entire implementation to RN for simplicity and also to simplify some clean ups in the future (like removing `top` prefixes from native event types). ## How did you test this change? The changes are gated. Will test e2e in RN.
## Summary
The Paper renderer is no longer used in React Native. This commit
removes all remaining Paper source code, tests, build system references,
and Paper backward-compatibility branches in shared code.
Deleted Paper-only source files:
- ReactNativeRenderer.js, ReactNativeInjection.js,
ReactFiberConfigNative.js
- ReactNativeComponentTree.js, ReactNativeEventEmitter.js
- ReactNativeFiberHostComponent.js, ReactNativeGlobalResponderHandler.js
- ReactNativeAttributePayload.js, NativeMethodsMixinUtils.js
- ReactFiberConfig.native.js (reconciler fork)
- index.js (Paper entry point)
Cleaned up shared files:
- ReactNativePublicCompat.js: removed _nativeTag checks, UIManager/
legacySendAccessibilityEvent Paper fallbacks
- ReactNativeFiberInspector.js: removed getInspectorDataForViewTag,
UIManager.measure fallback, Paper branch in
getInspectorDataForViewAtPoint
- ReactFiberConfigFabric.js: removed _nativeTag backward compat in
getPublicInstance, removed getInspectorDataForViewTag from devtools
config
- ReactNativeTypes.js: removed ReactNativeType (Paper API type)
Cleaned up build system:
- inlinedHostConfigs.js: removed shortName 'native' config
- forks.js: removed dead 'react-native-renderer' case
- Deleted ReactNative.js shim and Paper-only test mocks
## How did you test this change?
Manually synced the renderer to RN and passed all Fantom tests.
Manually verified the differences in the generated `ReactFabric-dev.js`
file. Only Paper compat logic has been removed.
<details>
<summary>diff</summary>
```diff
--- /tmp/react-fabric-baseline/ReactFabric-dev.js 2026-04-16 16:42:42
+++ build/react-native/implementations/ReactFabric-dev.js 2026-04-16 18:08:43
@@ -30,43 +30,19 @@
: emptyObject;
}
function createHierarchy(fiberHierarchy) {
- return fiberHierarchy.map(function (fiber$jscomp$0) {
+ return fiberHierarchy.map(function (fiber) {
return {
- name: getComponentNameFromType(fiber$jscomp$0.type),
+ name: getComponentNameFromType(fiber.type),
getInspectorData: function () {
return {
- props: getHostProps(fiber$jscomp$0),
+ props: getHostProps(fiber),
measure: function (callback) {
- var hostFiber = findCurrentHostFiber(fiber$jscomp$0);
- if (
- (hostFiber =
- null != hostFiber &&
- null !== hostFiber.stateNode &&
- hostFiber.stateNode.node)
- )
+ var hostFiber = findCurrentHostFiber(fiber);
+ (hostFiber =
+ null != hostFiber &&
+ null !== hostFiber.stateNode &&
+ hostFiber.stateNode.node) &&
nativeFabricUIManager.measure(hostFiber, callback);
- else {
- hostFiber = ReactNativePrivateInterface.UIManager;
- var JSCompiler_temp_const = hostFiber.measure,
- JSCompiler_inline_result;
- a: {
- for (var fiber = fiber$jscomp$0; fiber; ) {
- null !== fiber.stateNode &&
- 5 === fiber.tag &&
- (JSCompiler_inline_result = findNodeHandle(
- fiber.stateNode
- ));
- if (JSCompiler_inline_result) break a;
- fiber = fiber.child;
- }
- JSCompiler_inline_result = null;
- }
- return JSCompiler_temp_const.call(
- hostFiber,
- JSCompiler_inline_result,
- callback
- );
- }
}
};
}
@@ -1805,18 +1781,6 @@
}
return null;
}
- function doesFiberContain(parentFiber, childFiber) {
- for (
- var parentFiberAlternate = parentFiber.alternate;
- null !== childFiber;
-
- ) {
- if (childFiber === parentFiber || childFiber === parentFiberAlternate)
- return !0;
- childFiber = childFiber.return;
- }
- return !1;
- }
function traverseVisibleHostChildren(
child,
searchWithinHosts,
@@ -16986,44 +16950,6 @@
function getCurrentFiberForDevTools() {
return current;
}
- function findNodeHandle(componentOrHandle) {
- var owner = current;
- null !== owner &&
- isRendering &&
- null !== owner.stateNode &&
- (owner.stateNode._warnedAboutRefsInRender ||
- console.error(
- "%s is accessing findNodeHandle inside its render(). render() should be a pure function of props and state. It should never access something that requires stale data from the previous render, such as refs. Move this logic to componentDidMount and componentDidUpdate instead.",
- getComponentNameFromType(owner.type) || "A component"
- ),
- (owner.stateNode._warnedAboutRefsInRender = !0));
- if (null == componentOrHandle) return null;
- if ("number" === typeof componentOrHandle) return componentOrHandle;
- if (componentOrHandle._nativeTag) return componentOrHandle._nativeTag;
- if (
- null != componentOrHandle.canonical &&
- null != componentOrHandle.canonical.nativeTag
- )
- return componentOrHandle.canonical.nativeTag;
- if (
- (owner =
- ReactNativePrivateInterface.getNativeTagFromPublicInstance(
- componentOrHandle
- ))
- )
- return owner;
- componentOrHandle = findHostInstanceWithWarning(
- componentOrHandle,
- "findNodeHandle"
- );
- return null == componentOrHandle
- ? componentOrHandle
- : null != componentOrHandle._nativeTag
- ? componentOrHandle._nativeTag
- : ReactNativePrivateInterface.getNativeTagFromPublicInstance(
- componentOrHandle
- );
- }
function getNodeFromInternalInstanceHandle(internalInstanceHandle) {
return (
internalInstanceHandle &&
@@ -17134,12 +17060,9 @@
}
return instance.canonical.publicInstance;
}
- return null != instance.containerInfo &&
- null != instance.containerInfo.publicInstance
+ return null != instance.containerInfo
? instance.containerInfo.publicInstance
- : null != instance._nativeTag
- ? instance
- : null;
+ : null;
}
function getPublicInstanceFromHostFiber(fiber) {
fiber = getPublicInstance(fiber.stateNode);
@@ -18017,7 +17940,6 @@
DefaultEventPriority = 32,
IdleEventPriority = 268435456,
searchTarget = null,
- instanceCache = new Map(),
bind = Function.prototype.bind,
valueStack = [];
var fiberStack = [];
@@ -20041,24 +19963,19 @@
_nativeFabricUIManage.unstable_getCurrentEventPriority,
extraDevToolsConfig = {
getInspectorDataForInstance: getInspectorDataForInstance,
- getInspectorDataForViewTag: function (viewTag) {
- viewTag = instanceCache.get(viewTag) || null;
- return getInspectorDataForInstance(viewTag);
- },
getInspectorDataForViewAtPoint: function (
inspectedView,
locationX,
locationY,
callback
) {
- var closestInstance = null,
- fabricNode =
- ReactNativePrivateInterface.getNodeFromPublicInstance(
- inspectedView
- );
- fabricNode
+ var closestInstance = null;
+ (inspectedView =
+ ReactNativePrivateInterface.getNodeFromPublicInstance(
+ inspectedView
+ ))
? nativeFabricUIManager.findNodeAtPoint(
- fabricNode,
+ inspectedView,
locationX,
locationY,
function (internalInstanceHandle) {
@@ -20109,32 +20026,9 @@
}
}
)
- : null != inspectedView._internalFiberInstanceHandleDEV
- ? ReactNativePrivateInterface.UIManager.findSubviewIn(
- findNodeHandle(inspectedView),
- [locationX, locationY],
- function (nativeViewTag, left, top, width, height) {
- var inspectorData = getInspectorDataForInstance(
- instanceCache.get(nativeViewTag) || null
- );
- callback(
- assign({}, inspectorData, {
- pointerY: locationY,
- frame: {
- left: left,
- top: top,
- width: width,
- height: height
- },
- touchedViewTag: nativeViewTag,
- closestPublicInstance: nativeViewTag
- })
- );
- }
- )
- : console.error(
- "getInspectorDataForViewAtPoint expects to receive a host component"
- );
+ : console.error(
+ "getInspectorDataForViewAtPoint expects to receive a host component"
+ );
}
},
getViewConfigForType =
@@ -20368,23 +20262,12 @@
);
};
exports.dispatchCommand = function (handle, command, args) {
- var nativeTag =
- null != handle._nativeTag
- ? handle._nativeTag
- : ReactNativePrivateInterface.getNativeTagFromPublicInstance(handle);
- null == nativeTag
- ? console.error(
+ handle = ReactNativePrivateInterface.getNodeFromPublicInstance(handle);
+ null != handle
+ ? nativeFabricUIManager.dispatchCommand(handle, command, args)
+ : console.error(
"dispatchCommand was called with a ref that isn't a native component. Use React.forwardRef to get access to the underlying native component"
- )
- : ((handle =
- ReactNativePrivateInterface.getNodeFromPublicInstance(handle)),
- null != handle
- ? nativeFabricUIManager.dispatchCommand(handle, command, args)
- : ReactNativePrivateInterface.UIManager.dispatchViewManagerCommand(
- nativeTag,
- command,
- args
- ));
+ );
};
exports.findHostInstance_DEPRECATED = function (componentOrHandle) {
var owner = current;
@@ -20402,14 +20285,46 @@
: componentOrHandle.canonical &&
componentOrHandle.canonical.publicInstance
? componentOrHandle.canonical.publicInstance
- : componentOrHandle._nativeTag
- ? componentOrHandle
- : findHostInstanceWithWarning(
- componentOrHandle,
- "findHostInstance_DEPRECATED"
- );
+ : findHostInstanceWithWarning(
+ componentOrHandle,
+ "findHostInstance_DEPRECATED"
+ );
};
- exports.findNodeHandle = findNodeHandle;
+ exports.findNodeHandle = function (componentOrHandle) {
+ var owner = current;
+ null !== owner &&
+ isRendering &&
+ null !== owner.stateNode &&
+ (owner.stateNode._warnedAboutRefsInRender ||
+ console.error(
+ "%s is accessing findNodeHandle inside its render(). render() should be a pure function of props and state. It should never access something that requires stale data from the previous render, such as refs. Move this logic to componentDidMount and componentDidUpdate instead.",
+ getComponentNameFromType(owner.type) || "A component"
+ ),
+ (owner.stateNode._warnedAboutRefsInRender = !0));
+ if (null == componentOrHandle) return null;
+ if ("number" === typeof componentOrHandle) return componentOrHandle;
+ if (
+ null != componentOrHandle.canonical &&
+ null != componentOrHandle.canonical.nativeTag
+ )
+ return componentOrHandle.canonical.nativeTag;
+ if (
+ (owner =
+ ReactNativePrivateInterface.getNativeTagFromPublicInstance(
+ componentOrHandle
+ ))
+ )
+ return owner;
+ componentOrHandle = findHostInstanceWithWarning(
+ componentOrHandle,
+ "findNodeHandle"
+ );
+ return null == componentOrHandle
+ ? componentOrHandle
+ : ReactNativePrivateInterface.getNativeTagFromPublicInstance(
+ componentOrHandle
+ );
+ };
exports.getNodeFromInternalInstanceHandle =
getNodeFromInternalInstanceHandle;
exports.getPublicInstanceFromInternalInstanceHandle = function (
@@ -20433,14 +20348,6 @@
: null;
};
exports.isChildPublicInstance = function (parentInstance, childInstance) {
- if (
- parentInstance._internalFiberInstanceHandleDEV &&
- childInstance._internalFiberInstanceHandleDEV
- )
- return doesFiberContain(
- parentInstance._internalFiberInstanceHandleDEV,
- childInstance._internalFiberInstanceHandleDEV
- );
parentInstance =
ReactNativePrivateInterface.getInternalInstanceHandleFromPublicInstance(
parentInstance
@@ -20449,9 +20356,27 @@
ReactNativePrivateInterface.getInternalInstanceHandleFromPublicInstance(
childInstance
);
- return null != parentInstance && null != childInstance
- ? doesFiberContain(parentInstance, childInstance)
- : !1;
+ if (null != parentInstance && null != childInstance) {
+ a: {
+ for (
+ var parentFiberAlternate = parentInstance.alternate;
+ null !== childInstance;
+
+ ) {
+ if (
+ childInstance === parentInstance ||
+ childInstance === parentFiberAlternate
+ ) {
+ parentInstance = !0;
+ break a;
+ }
+ childInstance = childInstance.return;
+ }
+ parentInstance = !1;
+ }
+ return parentInstance;
+ }
+ return !1;
};
exports.render = function (
element,
@@ -20521,22 +20446,12 @@
return element;
};
exports.sendAccessibilityEvent = function (handle, eventType) {
- var nativeTag =
- null != handle._nativeTag
- ? handle._nativeTag
- : ReactNativePrivateInterface.getNativeTagFromPublicInstance(handle);
- null == nativeTag
- ? console.error(
+ handle = ReactNativePrivateInterface.getNodeFromPublicInstance(handle);
+ null != handle
+ ? nativeFabricUIManager.sendAccessibilityEvent(handle, eventType)
+ : console.error(
"sendAccessibilityEvent was called with a ref that isn't a native component. Use React.forwardRef to get access to the underlying native component"
- )
- : ((handle =
- ReactNativePrivateInterface.getNodeFromPublicInstance(handle)),
- null != handle
- ? nativeFabricUIManager.sendAccessibilityEvent(handle, eventType)
- : ReactNativePrivateInterface.legacySendAccessibilityEvent(
- nativeTag,
- eventType
- ));
+ );
};
exports.stopSurface = function (containerTag) {
var root = roots.get(containerTag);
```
</details>
…le times (facebook#36287) It is possible for the fallback tasks from a Suspense boundary to trigger an early `completeAll` call which is later repeated due to `finishedTask` reentrancy. For node.js in particular this might be problematic since we invoke a callback on each `completeAll` call but in general it just isn't the right semantics since the call is running slightly earlier than the completion of the last `finishedTask` invocation. This change ensures that any reentrant `finishedTask` calls (due to soft aborting fallback tasks) omit the `completeAll` call by temporarily incrementing the total pending tasks.
## Summary - Imports `startViewTransitionReadyFinished` from `nativeFabricUIManager` in `ReactFiberConfigFabricWithViewTransition` - Calls `fabricStartViewTransitionReadyFinished()` when the view transition `ready` promise resolves This is not a config function, but it's helpful to have it notify fabric ViewTransition runtime when ready callback is done. Right now we're testing animation kicked off from view transition event handlers, this is signal to know when animations that belong to a transition have all started. ## Test plan - Existing Fabric renderer tests should continue to pass - View transition ready callback now notifies the native module when finished
…n in Fabric (facebook#36196) ## Summary - Wires up the native `fabricCreateViewTransitionInstance` call in `createViewTransitionInstance` which will create a ShadowNode for old pseudo element - Extracts tag allocation logic into a shared `allocateTag()` function exported from `ReactFiberConfigFabric` - Imports `allocateTag` in `ReactFiberConfigFabricWithViewTransition` - Reuses `allocateTag()` in `createInstance` and `createTextInstance` instead of inline tag incrementing - Wires up native `fabricSuspendOnActiveViewTransition` call in `suspendOnActiveViewTransition` which suspends another view transition when the previous one is not yet finished ## Test plan - Existing Fabric renderer tests should continue to pass - ViewTransition instance creation now properly allocates a tag and calls the native module
The prior fix for finishedTask reentrancy solved an observed failure. This change adds a bit of defensive bookeeping to protect against other theoretical reentrant task finishing that might fail in simlar ways but where we don't have a clear demonstration of the bug.
…ow (facebook#36297) ## Summary PR facebook#36285 deleted the Paper (legacy) renderer, including the shim file `scripts/rollup/shims/react-native/ReactNative.js`. However, the `runtime_commit_artifacts` workflow still tries to `rm` this file after moving build artifacts into `compiled-rn/`. Since the file no longer exists in the build output, `rm` (without `-f`) fails and kills the entire step. This has caused **every run of the Commit Artifacts workflow to fail since facebook#36285 landed on April 16**, blocking both `builds/facebook-www` and `builds/facebook-fbsource` branches from receiving new build artifacts. This in turn blocks DiffTrain from syncing React changes into Meta's internal monorepo.
…cebook#36307) The `component-hook-factories` rule was removed in facebook#35825 as part of a feature flag cleanup, but was listed in the README as part of the manual config example. This broke users who used a manual config (copied from the old README) in eslint-plugin-react-hooks 7.1.0. This adds back a deprecated no-op rule as a fix. facebook#35825 removed other rules (`automatic-effect-dependencies` and `fire`), but these were for experimental features that did not ship. These were also not referenced in the README.
…cebook#36310) ## Summary - Adds a null check before calling `fabricSuspendOnActiveViewTransition()` in the Fabric renderer's `suspendOnActiveViewTransition` export - Prevents crashes on hosts where `nativeFabricUIManager` does not yet implement `suspendOnActiveViewTransition` ## Test plan - Verified the change compiles correctly - Hosts with `suspendOnActiveViewTransition` implemented continue to work as before - Hosts without `suspendOnActiveViewTransition` no longer crash when view transitions are active
Updated the changelog to reflect the correct contributor for the ESLint v10 support addition.
…ebook#36148) ## Summary The `credentialless` attribute is a boolean HTML attribute for `<iframe>` elements that loads the iframe in a new, ephemeral context without access to the parent's credentials (cookies, client certificates, etc.). This change adds it to all boolean attribute switch/case lists in React DOM so it is properly handled as a boolean (set when true, removed when false) rather than being treated as an unknown string attribute. Per the [Anonymous iframe spec (WICG)](https://wicg.github.io/anonymous-iframe/): > The credentialless attribute enables loading documents hosted by the iframe with a new and ephemeral storage partition. It is a boolean value. The default is false. ``` partial interface HTMLIFrameElement { attribute boolean credentialless; }; ``` Changes: - ReactDOMComponent.js: Added to both `setProp` and `diffHydratedGenericElement` - ReactFizzConfigDOM.js: Added to `pushAttribute` for server-side rendering - ReactDOMUnknownPropertyHook.js: Added to both validation switch/case lists ## Test plan - Added unit test in DOMPropertyOperations-test.js verifying `credentialless={true}` sets the attribute to `''` and `credentialless={false}` removes it - All tests pass in source and www channels (590 tests each) - Flow type checking passes (dom-node renderer) - Prettier and lint pass
…facebook#36322) ## Summary Follow-up to facebook#36148 (which added credentialless as a recognized boolean attribute for iframes). Adds credentialless to possibleStandardNames so React's dev warning can suggest the correct casing when users write it as Credentialless (or another incorrect case). Includes an SSR test asserting the "Did you mean credentialless?" warning fires. ## Test plan - yarn test ReactDOMComponent passes, including the new should warn about incorrect casing on the credentialless property (ssr) case
…re options per DOM spec (facebook#36047) ## Summary `FragmentInstance.addEventListener` and `removeEventListener` fail to cross-match listeners when the `capture` option is passed as a **boolean** in one call and an **options object** in the other. This violates the [DOM Living Standard](https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener), which states that `addEventListener(type, fn, true)` and `addEventListener(type, fn, {capture: true})` are identical. ### Root Cause In `ReactFiberConfigDOM.js`, the `normalizeListenerOptions` function generates a listener key string for deduplication. The boolean branch generates a **different format** than the object branch: ```js // Boolean branch (old) — produces "c=1" return `c=${opts ? '1' : '0'}`; // Object branch — produces "c=1&o=0&p=0" return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`; ``` Because the keys differ, `indexOfEventListener` cannot match them — so `removeEventListener('click', fn, {capture: true})` silently fails to remove a listener registered with `addEventListener('click', fn, true)`, and vice versa. This causes a **memory leak and event listener accumulation** on all Fragment child DOM nodes. ### Fix Normalize the boolean branch to produce the same full key format: ```js // Boolean branch (fixed) — now produces "c=1&o=0&p=0" (matches object branch) return `c=${opts ? '1' : '0'}&o=0&p=0`; ``` This makes both forms produce an identical key, matching the DOM spec behavior. ### When Was This Introduced This bug has been present since `FragmentInstance` event listener tracking was first added. It became reachable in production as of [facebook#36026](facebook#36026) which enabled `enableFragmentRefs` + `enableFragmentRefsInstanceHandles` across all builds (merged 3 days ago). ### Tests Added two new regression tests to `ReactDOMFragmentRefs-test.js`: 1. `removes a capture listener registered with boolean when removed with options object` 2. `removes a capture listener registered with options object when removed with boolean` Both tests were failing before this fix and pass after. ## How did you test this change? Added two new automated tests covering both cross-form removal directions. Existing tests continue to pass. ## Changelog ### React DOM - **Fixed** `FragmentInstance.removeEventListener()` not removing capture-phase listeners when the `capture` option form (boolean vs options object) differs between `add` and `remove` calls.
…ook#36309) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> - Adds a new react-flight-server-fb package providing RSC Flight bindings for Meta's internal bundler stack - Unlike webpack/turbopack integrations, this uses no manifest. Module metadata is self-contained in ClientReference objects and sent over the wire as-is - Registers dom-browser-fb and dom-node-fb host configs for Rollup builds targeting FB_WWW_DEV and FB_WWW_PROD Key design differences from other bundler - No build-time manifest - Module IDs use Haste module names (e.g. `"MyComponent"`), with named exports encoded as `"Module#export"`, rather than file paths resolved through a manifest - Client-side loading uses `Bootloader.handlePayload()` + `JSResource().load()` - `resolveClientReferenceMetadata` and `resolveClientReference` are pass-throughs ## How did you test this change? <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> E2E integration test is set up on Meta's internal system.
|
Hi @modeysamuel-lab! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
How did you test this change?