Skip to content

Commit 65aa819

Browse files
kkafarfacebook-github-bot
authored andcommitted
Fix new codegen API disabling behaviour of the old one (#51867)
Summary: Hey, I'm bumping `react-native-screens` to 0.80.0-rc.4 right now & noticed that codegen does not work as expected. It crawls whole library, file by file (when working on a library it also includes nodemodules etc), despite `componentProvider` field being defined in `codegenConfig` in `package.json`. Namely #49941 introduced a regression for libraries that stick to the old codegen config format due to compatibility reasons. Take a look at [this code](https://github.com/facebook/react-native/blob/78caa07ff867255724e4b0ce3557ff5793b6ce1b/packages/react-native/scripts/codegen/generate-artifacts-executor/generateRCTThirdPartyComponents.js#L60-L103). We first do a pass for "old API", [removing libraries with `componentProvider` in their codegen config from `librariesToCrawl`](https://github.com/facebook/react-native/blob/78caa07ff867255724e4b0ce3557ff5793b6ce1b/packages/react-native/scripts/codegen/generate-artifacts-executor/generateRCTThirdPartyComponents.js#L66-L75), just to later [add **ALL** libraries](https://github.com/facebook/react-native/blob/78caa07ff867255724e4b0ce3557ff5793b6ce1b/packages/react-native/scripts/codegen/generate-artifacts-executor/generateRCTThirdPartyComponents.js#L86-L89) that do not support new codegen config format back again to `librariesToCrawl`. This PR improves the filtering condition, to parse codegen annotations only for libraries that have defined any of the fields from the new config format (opted in for new system). I want to emphasise that this is a significant regression, because it ruins experience of library maintenance, vastly increasing pods installation time. ## Changelog: [IOS] [FIXED] - Fix codegen crawling all library code with `componentProvider` defined in config <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #51867 Test Plan: 1. Setup an application with any third party library, that uses "old" codegen config format, e.g. `react-native-screens@4.11.1` 2. `cd ios && bundle exec pod install` 3. Observe the codegen logs - it crawles the library instead of using provided information. Reviewed By: cortinico Differential Revision: D76126649 Pulled By: cipolleschi fbshipit-source-id: 6f9c9dffcdca7204a78b4b55db10249240146141
1 parent 198c6fc commit 65aa819

2 files changed

Lines changed: 18 additions & 8 deletions

File tree

packages/react-native/scripts/codegen/__tests__/__snapshots__/generate-artifacts-executor-test.js.snap

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,7 @@ exports[`execute test-app "RCTThirdPartyComponentsProvider.mm" should match snap
291291
292292
dispatch_once(&nativeComponentsToken, ^{
293293
thirdPartyComponents = @{
294-
@\\"TestAppDeprecatedComponent\\": NSClassFromString(@\\"RCTTestAppDeprecatedComponentClass\\"), // test-app
295294
@\\"TestAppComponent\\": NSClassFromString(@\\"RCTTestAppComponent\\"), // test-app
296-
@\\"TestLibraryDeprecatedComponent\\": NSClassFromString(@\\"RCTTestLibraryDeprecatedComponentClass\\"), // test-library
297295
@\\"TestLibraryComponent\\": NSClassFromString(@\\"RCTTestLibraryComponent\\"), // test-library
298296
};
299297
});
@@ -778,7 +776,7 @@ exports[`execute test-app-legacy "RCTThirdPartyComponentsProvider.mm" should mat
778776
779777
dispatch_once(&nativeComponentsToken, ^{
780778
thirdPartyComponents = @{
781-
779+
@\\"TestAppDeprecatedComponent\\": NSClassFromString(@\\"RCTTestAppDeprecatedComponentClass\\"), // test-app-legacy
782780
};
783781
});
784782

packages/react-native/scripts/codegen/generate-artifacts-executor/generateRCTThirdPartyComponents.js

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,23 @@ function generateRCTThirdPartyComponents(
5656

5757
const librariesToCrawl = {} /*:: as {[string]: $FlowFixMe} */;
5858

59+
// Using new API explicitly or not using any config field to define components.
60+
const componentLibrariesUsingNewApi = [];
61+
const componentLibrariesUsingOldApi = [];
62+
63+
for (const library of componentLibraries) {
64+
if (
65+
library.config.ios?.components ||
66+
!library.config.ios?.componentProvider
67+
) {
68+
componentLibrariesUsingNewApi.push(library);
69+
} else {
70+
componentLibrariesUsingOldApi.push(library);
71+
}
72+
}
73+
5974
// Old API
60-
componentLibraries.forEach(library => {
75+
componentLibrariesUsingOldApi.forEach(library => {
6176
const {config, libraryPath} = library;
6277
const libraryName = JSON.parse(
6378
fs.readFileSync(path.join(libraryPath, 'package.json'), 'utf8'),
@@ -66,9 +81,6 @@ function generateRCTThirdPartyComponents(
6681
librariesToCrawl[libraryName] = library;
6782

6883
const componentsProvider = config.ios?.componentProvider;
69-
if (!componentsProvider) {
70-
return;
71-
}
7284

7385
delete librariesToCrawl[libraryName];
7486
componentsInLibraries[libraryName] =
@@ -83,7 +95,7 @@ function generateRCTThirdPartyComponents(
8395
});
8496

8597
// New API
86-
const iosAnnotations = parseiOSAnnotations(componentLibraries);
98+
const iosAnnotations = parseiOSAnnotations(componentLibrariesUsingNewApi);
8799
for (const [libraryName, annotationMap] of Object.entries(iosAnnotations)) {
88100
const {library, components} = annotationMap;
89101
librariesToCrawl[libraryName] = library;

0 commit comments

Comments
 (0)