feat: add support for callback parameters and promise return types in navigation methods#412
Open
Doko-Demo-Doa wants to merge 2 commits into
Conversation
… navigation methods
…pes in navigation spec
Member
|
@Doko-Demo-Doa Thanks for this PR! Allow us some time to go through it and test 👍 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the packages/cli brownfield navigation spec/codegen pipeline to support (a) callback-typed method parameters and (b) Promise<T> return types, and updates iOS/Android generators so async methods forward to the delegate instead of returning a hardcoded "not_implemented" rejection.
Changes:
- Add callback and promise metadata to the parsed method/type model (
callbacksignatures on params;promiseReturnTypeon methods). - Enhance the navigation spec parser to recognize callback params, extract
Promise<T>inner types, and validate unsupported signatures. - Update iOS (Swift/Obj-C) and Android (Kotlin) generators to emit callback/async-aware delegate + module bindings, with new/updated Vitest coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/navigation/types.ts | Adds CallbackSignature/CallbackParam plus promiseReturnType and optional callback metadata for generator inputs. |
| packages/cli/src/navigation/parser.ts | Parses function-typed params as callbacks, parses Promise<T> return types, and validates unsupported signatures. |
| packages/cli/src/navigation/generators/ios.ts | Adds callback param support and forwards async Obj-C methods to the delegate (instead of rejecting). |
| packages/cli/src/navigation/generators/android.ts | Adds callback param support and forwards async Kotlin methods to the delegate (instead of rejecting). |
| packages/cli/src/navigation/tests/parser.test.ts | Adds tests for callback parsing, promise return parsing, and new validation errors. |
| packages/cli/src/navigation/tests/generators.test.ts | Adds tests asserting generated iOS/Android bindings for callbacks and async delegate forwarding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+59
| function mapParamToObjC( | ||
| param: MethodParam, | ||
| nullable: boolean = false, | ||
| options: ObjCTypeMappingOptions = {} | ||
| ): string { | ||
| return param.callback | ||
| ? 'RCTResponseSenderBlock' | ||
| : mapTsTypeToObjC(param.type, nullable, options); | ||
| } |
Comment on lines
+89
to
+96
| function mapParamToSwift( | ||
| param: MethodParam, | ||
| options: SwiftTypeMappingOptions = {} | ||
| ): string { | ||
| return param.callback | ||
| ? '@escaping RCTResponseSenderBlock' | ||
| : mapTsTypeToSwift(param.type, param.optional, options); | ||
| } |
Comment on lines
+48
to
+56
| function mapParamToKotlin( | ||
| param: MethodParam, | ||
| options: KotlinTypeMappingOptions = {}, | ||
| layer: 'delegate' | 'module' = 'delegate' | ||
| ): string { | ||
| return param.callback | ||
| ? 'Callback' | ||
| : mapTsTypeToKotlin(param.type, param.optional, options, layer); | ||
| } |
Comment on lines
+70
to
+88
| function validateMethodSignature(method: MethodSignature): void { | ||
| for (const param of method.params) { | ||
| if (getPromiseInnerType(param.type)) { | ||
| throw new Error( | ||
| `Unsupported Promise parameter "${param.name}" in method "${method.name}": Promise<T> is only supported as a method return type.` | ||
| ); | ||
| } | ||
|
|
||
| if (!param.callback) { | ||
| continue; | ||
| } | ||
|
|
||
| if (param.callback.returnType !== 'void') { | ||
| throw new Error( | ||
| `Unsupported callback parameter "${param.name}" in method "${method.name}": callback return type "${param.callback.returnType}" is not supported. Use a void callback or model the result as a Promise-returning navigation method.` | ||
| ); | ||
| } | ||
| } | ||
| } |
Comment on lines
+132
to
+208
| it('parses function-typed parameters into a callback signature', () => { | ||
| const specPath = createTempSpecFile(` | ||
| export interface BrownfieldNavigationSpec { | ||
| navigateToProfile(userId: string, onDismiss: (reason?: string) => void): void; | ||
| } | ||
| `); | ||
| tempSpecFiles.push(specPath); | ||
|
|
||
| const parsedSpec = parseNavigationSpec(specPath); | ||
|
|
||
| expect(parsedSpec.methods).toEqual([ | ||
| { | ||
| name: 'navigateToProfile', | ||
| params: [ | ||
| { name: 'userId', type: 'string', optional: false }, | ||
| { | ||
| name: 'onDismiss', | ||
| type: '(reason?: string) => void', | ||
| optional: false, | ||
| callback: { | ||
| params: [{ name: 'reason', type: 'string', optional: true }], | ||
| returnType: 'void', | ||
| }, | ||
| }, | ||
| ], | ||
| returnType: 'void', | ||
| isAsync: false, | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('parses Promise-returning methods into promiseReturnType and isAsync', () => { | ||
| const specPath = createTempSpecFile(` | ||
| export interface BrownfieldNavigationSpec { | ||
| requestPermission(name: string): Promise<boolean>; | ||
| } | ||
| `); | ||
| tempSpecFiles.push(specPath); | ||
|
|
||
| const parsedSpec = parseNavigationSpec(specPath); | ||
|
|
||
| expect(parsedSpec.methods).toEqual([ | ||
| { | ||
| name: 'requestPermission', | ||
| params: [{ name: 'name', type: 'string', optional: false }], | ||
| returnType: 'Promise<boolean>', | ||
| isAsync: true, | ||
| promiseReturnType: 'boolean', | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('throws when a parameter is typed as a Promise', () => { | ||
| const specPath = createTempSpecFile(` | ||
| export interface BrownfieldNavigationSpec { | ||
| foo(cb: Promise<string>): void; | ||
| } | ||
| `); | ||
| tempSpecFiles.push(specPath); | ||
|
|
||
| expect(() => parseNavigationSpec(specPath)).toThrow( | ||
| 'Unsupported Promise parameter "cb" in method "foo": Promise<T> is only supported as a method return type.' | ||
| ); | ||
| }); | ||
|
|
||
| it('throws when a callback parameter has a non-void return type', () => { | ||
| const specPath = createTempSpecFile(` | ||
| export interface BrownfieldNavigationSpec { | ||
| bar(cb: () => string): void; | ||
| } | ||
| `); | ||
| tempSpecFiles.push(specPath); | ||
|
|
||
| expect(() => parseNavigationSpec(specPath)).toThrow( | ||
| 'Unsupported callback parameter "cb" in method "bar": callback return type "string" is not supported. Use a void callback or model the result as a Promise-returning navigation method.' | ||
| ); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a follow-up PR to #348
Summary
Brownfield Navigation lib is mostly a mirror of Turbo Module codegen with less boilerplate. Previously, only simple methods with
voidreturn type are supported. This PR introduces Promise return type (with supported data types) and callback in arguments. Match closely with Turbo Module's Codegen Typings.Motivation
Brownfield navigation specs already supported declaring
Promise<T>return types (isAsyncwas parsed and threaded into the generators), but the Android/iOS async method generators never actually called into the delegate, they just returned a hardcoded rejection. Any consumer relying on an async navigation method today gets"not_implemented"at runtime no matter what. This PR closes that gap and, since callback-style delegate methods are a common pattern in native navigation glue (e.g. "call me back when the native screen is dismissed" or "try to do X, but if user is not authenticated, tell me the error code."), adds support for that as a sibling feature to Promises.Test plan
yarn vitest runinpackages/cli— existingparser/generatorssuites still pass (pre-existingisAsync/Promise<T>return-type path unaffected).parser.test.ts: function-typed param →MethodParam.callbackpopulated correctly (incl. nested param name/type/optionality);Promise<T>return type →isAsync/promiseReturnType; throws for aPromise<T>parameter; throws for a non-voidcallback return type.generators.test.ts: Kotlin delegate/module and Swift/Obj-C output for a method with a callback parameter; Kotlin module and Obj-C implementation output for an async method — asserts the delegate is actually invoked with the forwarded args +promise/resolve/reject, and that the oldnot_implementedstub string is gone.apps/RNApp+apps/AndroidApp+apps/AppleAppviabrownfield-navigation-codegenagainst a spec with a callback param and aPromise-returning method, and confirm the Android/iOS projects still build.Promise-returning method in the Android/iOS demo delegates and confirm end-to-end from JS: the callback fires and the promise resolves/rejects (not just that the generated code compiles). This is TBD.Small question: Should I prepare a PoC or video for demonstration?