Skip to content

Commit 4b65d0e

Browse files
kkafarreact-native-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. `[email protected]` 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 a1bad64 commit 4b65d0e

File tree

2 files changed

+18
-8
lines changed

2 files changed

+18
-8
lines changed

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
@@ -52,8 +52,23 @@ function generateRCTThirdPartyComponents(libraries, outputDir) {
5252

5353
const librariesToCrawl = {};
5454

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

6479
const componentsProvider = config.ios?.componentProvider;
65-
if (!componentsProvider) {
66-
return;
67-
}
6880

6981
delete librariesToCrawl[libraryName];
7082
componentsInLibraries[libraryName] =
@@ -79,7 +91,7 @@ function generateRCTThirdPartyComponents(libraries, outputDir) {
7991
});
8092

8193
// New API
82-
const iosAnnotations = parseiOSAnnotations(componentLibraries);
94+
const iosAnnotations = parseiOSAnnotations(componentLibrariesUsingNewApi);
8395
for (const [libraryName, annotationMap] of Object.entries(iosAnnotations)) {
8496
const {library, components} = annotationMap;
8597
librariesToCrawl[libraryName] = library;

0 commit comments

Comments
 (0)