Add optional annotation in serialize step in codegen objective C output
Summary:
Added support for optional methods in Objective-C protocols. This change allows marking methods as optional in native modules by adding the optional property to method definitions. Optional methods are properly grouped under a single @optional directive in the generated protocol, improving code organization and readability. Note that in Objective-C, @optional directive will apply for all the subsequent methods in protocol block so we have to move all optional methods at the end. This is implemented by inserting optional/required marker in ProtocolMethodTemplate in intermediate processing step.
Changelog:
[IOS] [CHANGED] - Support optional methods in native module protocols
Test Plan:
Updated test case GenerateModuleHObjCpp-test.js.snap with optional methods to ensure they are properly marked with @optional in the generated protocol and grouped together in the output.
Hi @aladine!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
@aladine can you fix the JS linter, please?
@cipolleschi : I just updated lint and snapshot tests.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D78265319.
For the latest lint error https://github.com/facebook/react-native/actions/runs/15977428423/job/45918230480, I run prettier locally and it doesn't detect any lint problem.
I run it locally. It applies this change in the serializeMethod.js file:
}>) =>
- `${isOptional ? '@optional\n' : '@required\n'}- (${returnObjCType})${methodName}${params};`;
+ `${
+ isOptional ? '@optional\n' : '@required\n'
+ }- (${returnObjCType})${methodName}${params};`;
export type StructParameterRecord = $ReadOnly<{
Thanks Riccardo, I have fixed the lint based on your suggestion.
how does this behave for Android? I know that the PR is only for iOS, but if we define an optional method, currently Android is forced to implement it, right?
That is a good question. For Android, the abstract method has a default no-ops implementation in generated class {} https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js#L117
If there is no implementation for optional method in Turbo class(which extends the generated spec class), it will fallback to the default no-ops block.
this is a bit stuck internally.
This was the initial implementation for this feature, but then we decided not to use the @optional.
The vision here is to move to a subclass approach, but that would be a breaking change for the ecosystem. This needs more discussion, unfortunately.