Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Codegen] Exclude unlinked libs from codegen #47712

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

cipolleschi
Copy link
Contributor

@cipolleschi cipolleschi commented Nov 19, 2024

Summary:

There is a case where a library is added for a specific platform and unlinked for another platform, because perhaps we would like to use a different library.

The way in which this is implemented is through the react-native.config.js file, by setting a specific platform to null.

Currently, Codegen ignore this information and generates the code for all the libraries it can find, including the unlinked ones. This can result in build failures because there are some functions prototype defined with no implementation.

This change takes thos libraries into account and it should fix: #47550.

Note

Codegen changed in 0.77, so we don't need to backport this to main

Changelog:

[General][Changed] - Stop generating code for unlinked libraries

Test Plan:

Tested in a new app created with 0.76.2, following the repro steps in #47550

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Nov 19, 2024
@cipolleschi cipolleschi changed the base branch from main to 0.76-stable November 19, 2024 14:29
@cipolleschi cipolleschi marked this pull request as ready for review November 19, 2024 14:32
@cipolleschi cipolleschi requested a review from blakef November 19, 2024 14:33
@cipolleschi cipolleschi changed the title cipolleschi/exclude unlinked libs from codegen [Codegen] Exclude unlinked libs from codegen Nov 19, 2024
@blakef blakef merged commit 3cedb09 into 0.76-stable Dec 4, 2024
9 of 11 checks passed
Comment on lines +314 to +318
Object.keys(dependency.platforms).forEach(platform => {
if (dependency.platforms[platform] == null) {
notLinkedPlatforms.push(platform);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously this was sufficient for module to be linked:

  dependencies: {
    'name': {
      root: '/path/to/your/module',
    },
  },

With this change, projects with this config crash during pod install since the platforms field isn't present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising this other case. I believe that if there is just the path, it links for all the platforms, right?

Copy link
Contributor

@satya164 satya164 Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cipolleschi currently it crashes when dependency.platforms isn't specified because the above check is accessing keys on dependency.platforms unconditionally

current workaround is to specify:

  dependencies: {
    'name': {
      root: '/path/to/your/module',
      platforms: { android: {}, ios: {} },
    },
  },

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. p: Facebook Partner: Facebook Partner Pick Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants