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 👍 |
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?