Skip to content

fix(codegen): skip node_modules and symlinks when crawling for components - #58518

Closed
gabrieldonadel wants to merge 1 commit into
mainfrom
@gabrieldonadel/fix-generateRCTThirdPartyComponents
Closed

gabrieldonadel wants to merge 1 commit into
mainfrom
@gabrieldonadel/fix-generateRCTThirdPartyComponents

Conversation

@gabrieldonadel

Copy link
Copy Markdown
Collaborator

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 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

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 14, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Sep 14, 2026
@meta-codesync

meta-codesync Bot commented Sep 15, 2026

Copy link
Copy Markdown

@vzaidman has imported this pull request. If you are a Meta employee, you can view this in D120122552.

@meta-codesync meta-codesync Bot closed this in 39751d8 Sep 15, 2026
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Sep 15, 2026
@meta-codesync

meta-codesync Bot commented Sep 15, 2026

Copy link
Copy Markdown

@vzaidman merged this pull request in 39751d8.

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant