Commit 39751d8
fix(codegen): skip node_modules and symlinks when crawling for components (#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: 233b8dc338b92372a34a0855f12a549e525132001 parent dcc872a commit 39751d8
2 files changed
Lines changed: 85 additions & 7 deletions
File tree
- packages/react-native/scripts/codegen
- __tests__
- generate-artifacts-executor
Lines changed: 67 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
553 | 553 | | |
554 | 554 | | |
555 | 555 | | |
556 | | - | |
| 556 | + | |
557 | 557 | | |
| 558 | + | |
558 | 559 | | |
559 | 560 | | |
560 | 561 | | |
| |||
586 | 587 | | |
587 | 588 | | |
588 | 589 | | |
589 | | - | |
| 590 | + | |
590 | 591 | | |
591 | 592 | | |
592 | 593 | | |
593 | 594 | | |
| 595 | + | |
594 | 596 | | |
595 | 597 | | |
596 | 598 | | |
| |||
619 | 621 | | |
620 | 622 | | |
621 | 623 | | |
622 | | - | |
| 624 | + | |
623 | 625 | | |
| 626 | + | |
624 | 627 | | |
625 | 628 | | |
626 | 629 | | |
| |||
635 | 638 | | |
636 | 639 | | |
637 | 640 | | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
638 | 702 | | |
639 | 703 | | |
640 | 704 | | |
| |||
Lines changed: 18 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
177 | 177 | | |
178 | 178 | | |
179 | 179 | | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
184 | 198 | | |
185 | 199 | | |
186 | 200 | | |
| |||
0 commit comments