Add beforeTransition hook to Flow and parity test [iOS]#803
Add beforeTransition hook to Flow and parity test [iOS]#803
Conversation
Expose the flow's beforeTransition hook on iOS so callers can observe or modify the navigation state before a transition, matching JVM behaviour.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #803 +/- ##
==========================================
+ Coverage 85.82% 85.85% +0.03%
==========================================
Files 505 505
Lines 22823 22868 +45
Branches 2657 2657
==========================================
+ Hits 19587 19633 +46
+ Misses 2908 2907 -1
Partials 328 328 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/canary |
|
/canary |
| /// The backing JSValue. Use when returning this state from a waterfall hook (e.g. beforeTransition). | ||
| public var jsValue: JSValue { rawValue } | ||
|
|
There was a problem hiding this comment.
Does this need to be public?
| let tapMethod: @convention(block) (JSValue?, JSValue?) -> JSValue? = { value, value2 in | ||
| guard let val = value, let val2 = value2 else { return nil } | ||
| let transitionValue = val2.toString() ?? "" | ||
| let typedState = NavigationBaseState.createInstance(value: val) | ||
| return hook(typedState, transitionValue)?.jsValue ?? val | ||
| } |
There was a problem hiding this comment.
Nit: Can we do guard let value and keep the original names instead of shadowing the names? I think that's a little cleaner. Also, can we do value1 and value2 to keep the names consistent? E.g.
let tapMethod: @convention(block) (JSValue?, JSValue?) -> JSValue? = { value1, value2 in
guard let value1, let value2 else { return nil }
let transitionValue = value2.toString() ?? ""
let typedState = NavigationBaseState.createInstance(value: value1)
return hook(typedState, transitionValue)?.jsValue ?? value1
}| - parameters: | ||
| - hook: A function (state, transitionValue) -> state (or nil to pass through) | ||
| */ | ||
| public func tap(_ hook: @escaping (NavigationBaseState?, String) -> NavigationBaseState?) { |
There was a problem hiding this comment.
can we make this generic so it’s reusable and aligned with web’s SyncWaterfallHook?
have you tried using SyncWaterfallHook2JS<T, R, U> from this PR where T = first arg (e.g. state), U = second arg (e.g. transition name or options), R = return type (typically same as T for waterfall)?
This PR adds the beforeTransition flow hook on iOS so apps can observe or modify the navigation state before a transition, in line with the existing behaviour on JVM and in the TypeScript core.
The core player exposes a beforeTransition SyncWaterfallHook (state + transition name → state). JVM already exposes it on FlowInstance.Hooks; iOS FlowHooks only had transition and afterTransition. This change adds beforeTransition on iOS and a test that mirrors the JVM integration test.
Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ADoes your PR have any documentation updates?
📦 Published PR as canary version:
0.15.1--canary.803.31915Try this version out locally by upgrading relevant packages to 0.15.1--canary.803.31915