Repository navigation
fix(mobile): break native step bar import cycle - #2947
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Define the shared step-bar contract apps/mobile/src/ui/primitives/step-bar-contract.ts |
Adds shared action and prop types, plus label helpers that include step details when provided. |
Wire renderers and check platform resolution apps/mobile/src/ui/primitives/step-bar.tsx, apps/mobile/src/ui/primitives/step-bar.android.tsx, apps/mobile/src/ui/primitives/step-bar.ios.tsx, scripts/test/step-bar-platform-resolution.test.mjs, changes/2945-native-step-bar-cycle.md |
The shared renderer entry point and platform renderers import and re-export the contract symbols. The test checks that Metro resolves renderer dependencies to source files other than the renderer itself. The changelog records the fix in English and Chinese. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- Issue 2926 describes native Git import flows that use the step bar. This fix addresses the Android startup failure that can block those flows.
- Issue 2923 describes Environment selection and shared step-bar use. This fix addresses the Android startup failure that can prevent the screen from loading.
- Issue 2928 describes Zip and marketplace import flows that reuse the step bar. This fix addresses the Android startup failure that can block those flows.
- Issue 2802 describes Bot creation and import journeys. This fix addresses a shared startup failure that can prevent the journeys from starting, but does not implement their feature behavior.
Merge Risk: 
Minimal · up to 68c68
-
No actionable issue is established for this change; it is mergeable after normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Description check
-
Explanation
The description is mostly complete. It explains the problem, links the issue, documents merge risk, verification results, the untested iOS runtime, the structural change, and before/after evidence. It does not explicitly state the CI test-plan mode, but this omission does not prevent understanding the change.
Full details: Linked Issues check
-
Explanation
Directly linked issue
#2945has three coding requirements. (1) Separate the platform-neutral step-bar contract and labels from the platform renderers: the PR adds apps/mobile/src/ui/primitives/step-bar-contract.ts with StepBarAction, StepBarProps, stepBarLabel, and stepBarAccessibilityLabel. step-bar.android.tsx and step-bar.ios.tsx now import and re-export these symbols from step-bar-contract. step-bar.tsx drops its former definitions and imports them from the contract. This removes the self-import cycle that caused the recursive getter. (2) Add regression coverage using Android module resolution: scripts/test/step-bar-platform-resolution.test.mjs parses each renderer, collects relative runtime imports and re-exports, resolves them with Metro, and asserts that they resolve to a source file other than the renderer. Per the PR, both the Android and iOS cases failed before the fix and pass after it. (3) Rerun the real phone before and after: this is manual device verification and is non-coding. The PR reports Android 16 arm64 evidence (failed before, Environment list rendered after). Under the instructions, it is not assessed as a coding requirement. Evidence came from the whole-PR summary and PR description; no repository inspection was needed to establish these behaviors.
Full details: Out of Scope Changes check
-
Explanation
All changed files relate to the
#2945objectives. The new step-bar-contract.ts holds the shared contract and label helpers. The android, ios, and base step-bar files change only their import and re-export sources, with exported names and signatures unchanged. The regression test scripts/test/step-bar-platform-resolution.test.mjs covers the required Android resolution check. The changes/2945-native-step-bar-cycle.md changelog entry is documentation for this fix. The added label helpers preserve existing label formatting, per the PR. No unrelated behavior change appears in the summary.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 5 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Issue #2945 · QA source #2930 · Agent claim
Why the change
Native platform resolution made the step bar import its own exports, crashing mobile startup before Environment selection or Bot creation could open.
Special things to note
codex/local/01a17bef-21c0-75f0-bf5d-2fedb6df7b73.Change outline
The new regression uses the installed Expo Metro resolver on each renderer's actual runtime imports and re-exports; both platform cases failed before the fix and pass after it. The contract preserves existing label formatting and exported types.
Before — Android native load failure. After minimizing the stack-overflow overlay, the Environment fixture never renders.
After — Environment list rendered. One native selected row and the Add 1 Environment capsule are visible. Both captures use the same 1220 × 1712 screen region; the upper account area, fixture email, status notifications and local paths are omitted. Original full PNG/XML/error evidence remains machine-local.
Refs #2922; Closes #2945.
Summary by CodeRabbit