Fix iOS Codegen fallback for libraries without an iOS config - #57790
Closed
RealBhupesh wants to merge 1 commit into
Closed
RealBhupesh wants to merge 1 commit into
RealBhupesh wants to merge 1 commit into
Conversation
|
Hi @RealBhupesh! 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! |
Contributor
Author
|
Hi React Native maintainers, I’ve completed the Meta CLA for this contribution. This PR is ready for review when convenient. It includes a regression test covering component libraries without an iOS Codegen config. Thank you! |
Contributor
Author
|
signed it |
RealBhupesh
marked this pull request as ready for review
August 2, 2026 07:31
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
|
@fabriziocucci has imported this pull request. If you are a Meta employee, you can view this in D114523918. |
cortinico
approved these changes
Aug 3, 2026
cortinico
left a comment
cortinico
left a comment
Contributor
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
@fabriziocucci merged this pull request in 917d97e. |
This was referenced Sep 14, 2026
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 15, 2026
…ents (#58518) Summary: `findFilesWithExtension` walks a library's directory looking for `.mm` files that declare a Fabric component. The walk descends into every subdirectory, `node_modules` included, resolves symlinks through `statSync`, and keeps no visited set. This got broken after #57790 because `parseiOSAnnotations` now enters every library that declares a `codegenConfig` without an `ios` key into its map with an empty `components` object, so nothing removes it from `librariesToCrawl`. Previously such libraries were filtered before crawling. An app that declares `codegenConfig` with `"type": "all"` and no `ios` key now gets treated as a component library, so the walk covers the entire project. What we notice in the [expo repo](expo/expo#50134) is that under pnpm, the traversal never terminates. Workspace packages link into each other's `node_modules` and form cycles. Because symlinks are followed, the walk only stops once paths hit the 1023 byte limit. CocoaPods progress stops right after the "Using React Native Core and React Native Dependencies prebuilt versions" line while a `generate-codegen-artifacts.js` child sits at 100% CPU. Under npm and yarn, the walk completes but with the wrong result. In those cases, `node_modules` holds real directories, but the walk covers the whole dependency tree. Crawling one app of roughly 1100 packages turned up 382 `.mm` files, 43 of which declare a component, among them React core views such as `RCTImageComponentView` and `RCTScrollViewComponentView` from `react-native-macos`. Each one is written into the app's entry in `RCTThirdPartyComponentsProvider.mm`. The existing `/react-native/` path filter does not exclude them, since it requires a trailing separator and `react-native-macos` has none. ## Changelog: [IOS] [FIXED] - Codegen no longer crawls `node_modules` or follows symlinks when discovering components Pull Request resolved: #58518 Test Plan: Two cases added to `packages/react-native/scripts/codegen/__tests__/generate-artifacts-executor-test.js`. The three existing `findFilesWithExtension` mocks move from `statSync` to `lstatSync`. The symlink case builds a link pointing back at its own parent, so the pre-fix walk never terminates. **Negative control:** with the `generateRCTThirdPartyComponents.js` change reverted and the tests left in place, both new cases fail. All 24 existing snapshots pass unchanged, so the change is additive for projects that already declare an `ios` config. Measured by crawling the app directory of two real projects: | Project | Before | After | | --- | --- | --- | | `apps/bare-expo`, pnpm | 116,800 reads in 30s, still running | 34 files, 1.1s | | yarn app, ~1100 packages | 382 files, 5.8s | 8 files, 0.3s | Reviewed By: fabriziocucci Differential Revision: D120122552 Pulled By: vzaidman fbshipit-source-id: 233b8dc338b92372a34a0855f12a549e52513200
fabriziocucci
pushed a commit
that referenced
this pull request
Sep 15, 2026
…ents (#58518) Summary: `findFilesWithExtension` walks a library's directory looking for `.mm` files that declare a Fabric component. The walk descends into every subdirectory, `node_modules` included, resolves symlinks through `statSync`, and keeps no visited set. This got broken after #57790 because `parseiOSAnnotations` now enters every library that declares a `codegenConfig` without an `ios` key into its map with an empty `components` object, so nothing removes it from `librariesToCrawl`. Previously such libraries were filtered before crawling. An app that declares `codegenConfig` with `"type": "all"` and no `ios` key now gets treated as a component library, so the walk covers the entire project. What we notice in the [expo repo](expo/expo#50134) is that under pnpm, the traversal never terminates. Workspace packages link into each other's `node_modules` and form cycles. Because symlinks are followed, the walk only stops once paths hit the 1023 byte limit. CocoaPods progress stops right after the "Using React Native Core and React Native Dependencies prebuilt versions" line while a `generate-codegen-artifacts.js` child sits at 100% CPU. Under npm and yarn, the walk completes but with the wrong result. In those cases, `node_modules` holds real directories, but the walk covers the whole dependency tree. Crawling one app of roughly 1100 packages turned up 382 `.mm` files, 43 of which declare a component, among them React core views such as `RCTImageComponentView` and `RCTScrollViewComponentView` from `react-native-macos`. Each one is written into the app's entry in `RCTThirdPartyComponentsProvider.mm`. The existing `/react-native/` path filter does not exclude them, since it requires a trailing separator and `react-native-macos` has none. ## Changelog: [IOS] [FIXED] - Codegen no longer crawls `node_modules` or follows symlinks when discovering components Pull Request resolved: #58518 Test Plan: Two cases added to `packages/react-native/scripts/codegen/__tests__/generate-artifacts-executor-test.js`. The three existing `findFilesWithExtension` mocks move from `statSync` to `lstatSync`. The symlink case builds a link pointing back at its own parent, so the pre-fix walk never terminates. **Negative control:** with the `generateRCTThirdPartyComponents.js` change reverted and the tests left in place, both new cases fail. All 24 existing snapshots pass unchanged, so the change is additive for projects that already declare an `ios` config. Measured by crawling the app directory of two real projects: | Project | Before | After | | --- | --- | --- | | `apps/bare-expo`, pnpm | 116,800 reads in 30s, still running | 34 files, 1.1s | | yarn app, ~1100 packages | 382 files, 5.8s | 8 files, 0.3s | Reviewed By: fabriziocucci Differential Revision: D120122552 Pulled By: vzaidman fbshipit-source-id: 233b8dc338b92372a34a0855f12a549e52513200 (cherry picked from commit 39751d8)
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
2 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
iosblock.Changelog:
[iOS] [FIXED] - iOS Codegen now discovers third-party component libraries without an iOS configuration.
Fixes #55308
Test plan
yarn jest packages/react-native/scripts/codegen/__tests__/generate-artifacts-executor-test.js --runInBand --testNamePattern="crawls component libraries without an iOS config"fs.rmdirSync(..., {recursive: true}).