mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2026-03-13 10:22:08 +08:00
chore(status): updating status file
This commit is contained in:
@@ -1,488 +1,171 @@
|
||||
# React Router 6 Upgrade - Status Analysis
|
||||
# React Router 6 Status Report
|
||||
|
||||
**Branch:** `sk/react-router-6`
|
||||
**Design Docs:** [PR #305](https://github.com/ionic-team/ionic-framework-design-documents/pull/305)
|
||||
**Last Updated:** October 30, 2025
|
||||
**Last Updated:** November 22, 2025
|
||||
|
||||
## Executive Summary
|
||||
## Summary
|
||||
|
||||
The `sk/react-router-6` branch contains significant progress toward React Router 6 support with ~55k insertions across 94 files. Core routing logic has been migrated to React Router 6 APIs, test applications exist for both RRv5 and RRv6, and fundamental navigation patterns are working.
|
||||
The React Router 6 work is in good shape. The core router integration matches the design docs, the test app exercises most of the interesting cases, and the latest Cypress run shows that almost everything behaves as expected. What remains are a small set of advanced flows around not-found handling, some tab context behavior, and one swipe-to-go-back scenario.
|
||||
|
||||
**Current Status:** 🔴 **Critical Bugs Identified - Not Alpha Ready**
|
||||
**Alignment with Design Docs:** 🟢 Strong (85% aligned architecturally)
|
||||
**E2E Test Pass Rate:** 🔴 44% (30/68 tests passing)
|
||||
**Alpha Readiness:** 🔴 Blocked by critical bugs (~40% complete for production)
|
||||
At this point the work feels like late-stage refinement rather than architectural surgery.
|
||||
|
||||
- Total Cypress tests: 69
|
||||
- Passing: 62
|
||||
- Failing: 7
|
||||
- Suites with failures: `routing.cy.js`, `swipe-to-go-back.cy.js`, `tab-context.cy.js`
|
||||
|
||||
Once those are cleaned up and the types/docs are brought up to date, this should be ready for an alpha release.
|
||||
|
||||
---
|
||||
|
||||
## Alignment with Design Documents
|
||||
## Test status (Cypress – React Router 6 test app)
|
||||
|
||||
### ✅ Successfully Implemented (Aligned with Docs)
|
||||
Latest run of `npm run cypress` against the RR6 test app:
|
||||
|
||||
#### 1. **Core API Migration**
|
||||
- ✅ **Package Dependencies**: Updated peer dependencies from `react-router@^5.0.1` → `>=6.0.0`
|
||||
- ✅ **Hook Migration**:
|
||||
- Using `useLocation()`, `useNavigate()`, `useParams()` from React Router 6
|
||||
- Removed legacy `useHistory()` and `props.match` patterns
|
||||
- ✅ **Route Definition**: Using `element` prop instead of `component`/`render`
|
||||
- ✅ **matchPath API**: Migrated to React Router 6 signature (pathname + options object)
|
||||
- `dynamic-ionpage-classnames.cy.js` – 1/1 passing
|
||||
- `dynamic-routes.cy.js` – 3/3 passing
|
||||
- `dynamic-tabs.cy.js` – 3/3 passing
|
||||
- `multiple-tabs.cy.js` – 4/4 passing
|
||||
- `nested-outlets.cy.js` – 11/11 passing
|
||||
- `outlet-ref.cy.js` – 1/1 passing
|
||||
- `overlays.cy.js` – 3/3 passing
|
||||
- `refs.cy.js` – 2/2 passing
|
||||
- `replace-actions.cy.js` – 1/1 passing
|
||||
- `routing.cy.js` – 24/28 passing (4 failing)
|
||||
- `swipe-to-go-back.cy.js` – 7/8 passing (1 failing)
|
||||
- `tab-context.cy.js` – 0/2 passing (2 failing)
|
||||
- `tabs.cy.js` – 2/2 passing
|
||||
|
||||
#### 2. **IonRouter.tsx** (0005-ion-router.md)
|
||||
- ✅ Implements `handleHistoryChange` with POP/REPLACE/PUSH logic as documented
|
||||
- ✅ Tracks `pushedByRoute` for linear navigation detection
|
||||
- ✅ Supports tab navigation with `currentTab` tracking
|
||||
- ✅ Implements `handleNavigateBack` with default href fallback
|
||||
- ✅ Parameter-based route detection and matching (`areParamsEqual`)
|
||||
- ✅ Uses `LocationHistory` for history stack management
|
||||
So the bulk of the suite is green. The remaining problems are concentrated in three areas:
|
||||
|
||||
**Code Evidence:**
|
||||
```typescript path=/Users/shane.king/code/ionic/ionic-framework/packages/react-router/src/ReactRouter/IonRouter.tsx start=54
|
||||
export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildren<IonRouterProps>) => {
|
||||
const location = useLocation();
|
||||
const navigate = useNavigate();
|
||||
|
||||
const didMountRef = useRef(false);
|
||||
const locationHistory = useRef(new LocationHistory());
|
||||
const currentTab = useRef<string | undefined>(undefined);
|
||||
const viewStack = useRef(new ReactRouterViewStack());
|
||||
```
|
||||
|
||||
#### 3. **ReactRouterViewStack.tsx** (0003-react-router-view-stack.md)
|
||||
- ✅ Manages view items with mount/unmount lifecycle
|
||||
- ✅ Implements `getChildrenToRender` for rendering active views
|
||||
- ✅ Handles route matching with parameter and wildcard support
|
||||
- ✅ View reuse logic for navigation history
|
||||
- ✅ Cleanup of stale view items to prevent memory leaks
|
||||
- ✅ Navigate component special handling (unmount after redirect)
|
||||
|
||||
**Code Evidence:**
|
||||
```typescript path=/Users/shane.king/code/ionic/ionic-framework/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx start=70
|
||||
export class ReactRouterViewStack extends ViewStacks {
|
||||
private pendingViewItems: Map<string, ViewItem> = new Map();
|
||||
private deactivationQueue: Map<string, NodeJS.Timeout> = new Map();
|
||||
private viewItemCounter = 0;
|
||||
```
|
||||
|
||||
#### 4. **StackManager.tsx** (0004-stack-manager.md)
|
||||
- ✅ Page transition management with entering/leaving view detection
|
||||
- ✅ Swipe-to-go-back gesture support
|
||||
- ✅ Nested outlet support with parent path calculation
|
||||
- ✅ Route matching for parameterized routes (e.g., `/user/:id`)
|
||||
- ✅ Clone workaround for same-view transitions with different params
|
||||
- ✅ Proper handling of index routes and relative routes
|
||||
|
||||
**Code Evidence:**
|
||||
```typescript path=/Users/shane.king/code/ionic/ionic-framework/packages/react-router/src/ReactRouter/StackManager.tsx start=773
|
||||
if (leavingViewItem && leavingViewItem.ionPageElement && enteringViewItem === leavingViewItem) {
|
||||
// If a page is transitioning to another version of itself
|
||||
// we clone it so we can have an animation to show
|
||||
// (e.g., `/user/1` → `/user/2`)
|
||||
const match = matchComponent(leavingViewItem.reactElement, routeInfo.pathname);
|
||||
```
|
||||
|
||||
#### 5. **Router Variants**
|
||||
- ✅ `IonReactRouter` using `BrowserRouter`
|
||||
- ✅ `IonReactMemoryRouter` using `MemoryRouter`
|
||||
- ✅ `IonReactHashRouter` using `HashRouter`
|
||||
|
||||
#### 6. **Test Infrastructure**
|
||||
- ✅ Separate test apps for React Router 5 and 6
|
||||
- ✅ Test pages covering:
|
||||
- Dynamic routes
|
||||
- Nested outlets
|
||||
- Tab navigation
|
||||
- Parameters (including nested params)
|
||||
- Replace actions
|
||||
- Swipe-to-go-back
|
||||
- Overlays
|
||||
- Multiple tabs
|
||||
- Dynamic tabs
|
||||
1. A handful of routing edge cases in `routing.cy.js`.
|
||||
2. One swipe-back path that gets confused after tab switching.
|
||||
3. The small tab-context demo that drives tabs via `IonTabsContext`.
|
||||
|
||||
---
|
||||
|
||||
## ⚠️ Gaps & Potential Issues
|
||||
## What’s working well
|
||||
|
||||
### 1. **Index Route Handling**
|
||||
**Issue:** Complex logic in `ReactRouterViewStack` and `StackManager` for index routes may have edge cases.
|
||||
This is the “you don’t need to worry about these anymore” list.
|
||||
|
||||
**Evidence:**
|
||||
```typescript path=/Users/shane.king/code/ionic/ionic-framework/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx start=54
|
||||
const resolveIndexRouteMatch = (viewItem: ViewItem, pathname: string): PathMatch<string> | null => {
|
||||
if (!viewItem.routeData?.childProps?.index) {
|
||||
return null;
|
||||
}
|
||||
// Complex normalization logic...
|
||||
}
|
||||
```
|
||||
### Core architecture
|
||||
|
||||
**Risk:** Index routes in nested outlets may not behave consistently with React Router 6.
|
||||
The main pieces match the RR6 design docs and behave sensibly in practice:
|
||||
|
||||
### 2. **Wildcard Route Precedence**
|
||||
**Issue:** Multiple passes to sort and prioritize wildcard routes vs. specific routes.
|
||||
- **`IonRouter`** uses `useLocation` / `useNavigate` from React Router 6, tracks a `LocationHistory`, and computes `RouteInfo` objects that drive Ionic’s view stacks and directions.
|
||||
- **`ReactRouterViewStack`** owns view items per outlet, handles RR6-style matching (including params and wildcards), and coordinates mount/unmount via `ViewLifeCycleManager`.
|
||||
- **`StackManager`** glues a given `ion-router-outlet` to the view stack. It:
|
||||
- picks entering/leaving views,
|
||||
- calls `routerOutlet.commit(...)` for transitions,
|
||||
- handles nested outlets via a computed parent path, and
|
||||
- wires in swipe-to-go-back.
|
||||
- **Router variants** (`IonReactRouter`, `IonReactHashRouter`, `IonReactMemoryRouter`) all run on top of this stack and are behaving as expected.
|
||||
|
||||
**Evidence:** StackManager lines 876-904 implement custom sorting that may conflict with React Router 6's built-in matching.
|
||||
There are no obvious architectural gaps left; the remaining bugs are about particular flows using this machinery, not about missing pieces.
|
||||
|
||||
**Risk:** Catch-all routes (`*`) may activate when they shouldn't, or vice versa.
|
||||
### Navigation scenarios covered by passing suites
|
||||
|
||||
### 3. **Type Safety**
|
||||
**Issue:** Several `@types/react-router-dom@^5.x` types still in devDependencies but code uses RRv6.
|
||||
All of these are currently green in Cypress and line up with the v5 behavior:
|
||||
|
||||
**Evidence:**
|
||||
```json path=/Users/shane.king/code/ionic/ionic-framework/packages/react-router/package.json start=55
|
||||
- **Dynamic routes** – adding/removing routes at runtime and hitting them directly.
|
||||
- **Dynamic tabs** – adding a second tab at runtime and keeping the view state correct.
|
||||
- **Multiple tab bars** – apps that switch between different tab sets.
|
||||
- **Nested outlets** – both the simple nested-outlet demo and the deeper `nested-outlet2` flows now pass end‑to‑end.
|
||||
- **Overlays** – modal navigation (including navigating away while a modal is open) cleans up correctly.
|
||||
- **Refs** – forwarding refs to Ionic components works for both function and class components.
|
||||
- **Replace actions** – replace-style navigation behaves as expected in the dedicated demo.
|
||||
- **Tabs** – the basic tabs test suite (separate from the tab-context demo) is fully passing.
|
||||
|
||||
Taken together, that covers the majority of patterns people actually use: plain stacks, nested outlets, “tabs within a shell route”, and overlays.
|
||||
|
||||
---
|
||||
|
||||
## Remaining problem areas
|
||||
|
||||
### 1. `routing.cy.js` – four failing tests
|
||||
|
||||
The main routing demo is mostly green, but four tests still fail:
|
||||
|
||||
1. **Not-found route** – `/routing/asdf` is expected to render a `not-found` page and never does. This suggests that either the catch‑all route is not wired correctly for the RR6 matching rules, or the page is mounted but never becomes the active view item.
|
||||
2. **Menu + redirect interaction** – the “Menu → Favorites → Menu → Home with redirect” flow expects Favorites to be hidden and Home to be visible; Cypress still finds the old page. This points to a corner case in how we hide/unmount views when a redirect happens off a menu selection.
|
||||
3. **Back button visibility on pushed page** – after pushing a new page, the test expects the back button to appear. The view content appears, but we never hit the “show back button” state. This is likely a mismatch between our `LocationHistory` bookkeeping and how we decide whether the current view has something to go back to.
|
||||
4. **Parameterized route instances** – the “mount new view item instances of parameterized routes” test times out with a value mismatch (`'1'` vs something else). This is the same area where we do view cloning for `/user/1` → `/user/2`; we may be reusing the wrong view item or not updating route params the way the test expects.
|
||||
|
||||
All of these are focused on how we track history and view identity, not on basic RR6 API usage.
|
||||
|
||||
### 2. `swipe-to-go-back.cy.js` – one failing test
|
||||
|
||||
Seven of the eight swipe tests pass. The remaining failure is:
|
||||
|
||||
- **“should swipe and go back to correct tab after switching tabs”** – after switching tabs and using the swipe gesture, we expect to land back on the correct tab page (`data-pageid=home`) but never see it.
|
||||
|
||||
The other swipe tests (including aborting gestures and swiping within a tab) behave correctly, so the problem is very specific: combining tab switching with a swipe‑back is leaving the `LocationHistory` and/or active view in a slightly inconsistent state.
|
||||
|
||||
### 3. `tab-context.cy.js` – both tests failing
|
||||
|
||||
The small tab-context demo, which exercises `IonTabsContext.selectTab`, still fails both of its tests:
|
||||
|
||||
1. Navigating from `/tab-context` and calling `selectTab('tab2')` tries to click a button that lives on a page still marked as hidden (`ion-page-hidden`). In other words, we update the active tab in context, but the underlying view stack has not actually switched visible pages.
|
||||
2. Starting on `/tab-context/tab1` and trying to flip back to tab1 via context never finds the expected `tab2` page, which again points to view items not lining up with what the context thinks is active.
|
||||
|
||||
The “normal” tabs test suite does not have this problem, so this looks isolated to how we bridge the context helper into the RR6 router + view stack.
|
||||
|
||||
---
|
||||
|
||||
## Non-test gaps and cleanup work
|
||||
|
||||
A few things that don’t show up as Cypress failures but are worth fixing before we put a label on this.
|
||||
|
||||
### TypeScript and types
|
||||
|
||||
`@ionic/react-router` is built against React Router 6, but the devDependencies still pull in v5 type packages:
|
||||
|
||||
```json
|
||||
"@types/react-router": "^5.0.3",
|
||||
"@types/react-router-dom": "^5.1.5",
|
||||
"@types/react-router-dom": "^5.1.5"
|
||||
```
|
||||
|
||||
**Action Needed:** Update to `@types/react-router@6` and `@types/react-router-dom@6`.
|
||||
That hasn’t bitten us in the test app, but it’s confusing and will matter for downstream consumers. We should:
|
||||
|
||||
### 4. **Unmount Behavior**
|
||||
**Issue:** The `routeOptions.unmount` flag logic is custom and not part of standard React Router behavior.
|
||||
- Drop the v5 type packages or replace them with the RR6 equivalents.
|
||||
- Run a full type check across the package once that change is made.
|
||||
|
||||
**Risk:** May cause confusion for developers expecting standard React Router patterns.
|
||||
### Documentation
|
||||
|
||||
### 5. **Documentation**
|
||||
**Gap:** No public-facing documentation or migration guide for users.
|
||||
Right now the only “docs” for this work are the design document and this status file. Before releasing anything, we should have:
|
||||
|
||||
**Action Needed:** Create migration guide following design doc 0001-upgrade-5-to-6.md.
|
||||
- A short migration guide for going from the v5 integration to the v6 one.
|
||||
- A clear explanation of any Ionic‑specific behavior that differs from plain React Router 6 (for example, `routeOptions.unmount`, how `LocationHistory` works, and how nested outlets should be declared with `/*`).
|
||||
|
||||
### 6. **Testing Coverage**
|
||||
**Gap:** No automated E2E tests running against React Router 6 test app visible in the diff.
|
||||
### Debug logging and general polish
|
||||
|
||||
**Action Needed:** Ensure Cypress/Playwright tests validate all navigation patterns.
|
||||
During bring‑up we added logging in a few places to trace routing behavior. Before shipping we should:
|
||||
|
||||
- Audit `IonRouter`, `ReactRouterViewStack`, and `StackManager` for stray `console.log` / `console.warn` calls.
|
||||
- Make sure any remaining logs are behind a flag or obviously diagnostic.
|
||||
|
||||
---
|
||||
|
||||
## Current Work Status
|
||||
## Short-term plan
|
||||
|
||||
### Recent Commits (Last 20)
|
||||
1. ✅ Nested parameter access support
|
||||
2. ✅ Redirect with replace routing (test app improvement)
|
||||
3. ✅ Wildcard and parameter route matching improvements
|
||||
4. ✅ Navigation stack reliability improvements
|
||||
5. ✅ Memory/hash router migration issues addressed
|
||||
6. ✅ Route params support added
|
||||
7. ✅ Nested routes and redirection working
|
||||
This is a realistic sequence of work to get from the current state to “alpha ready”:
|
||||
|
||||
### What's Working
|
||||
- ✅ Basic navigation (push/pop/replace)
|
||||
- ✅ Tab navigation
|
||||
- ✅ Nested outlets
|
||||
- ✅ Route parameters (`:id` patterns)
|
||||
- ✅ Wildcard routes
|
||||
- ✅ Browser back/forward
|
||||
- ✅ Swipe-to-go-back gesture
|
||||
- ✅ Memory and hash router variants
|
||||
1. **Close out the remaining E2E failures**
|
||||
- Fix the not‑found and redirect/menu cases in `routing.cy.js`.
|
||||
- Correct the back‑button logic and parameterized route instance handling in that same spec.
|
||||
- Align swipe‑back behavior after tab switches with the existing (passing) swipe tests.
|
||||
- Make `IonTabsContext.selectTab` and the RR6 router agree on which view is active in the tab-context demo.
|
||||
|
||||
### Known Issues from Commit Messages
|
||||
1. ⚠️ "Fixing redirect for test app" - indicates redirect edge cases were problematic
|
||||
2. ⚠️ "Addressing some migration issues" - vague, suggests unresolved items
|
||||
2. **Clean up types and build configuration**
|
||||
- Remove or update the v5 `@types/react-router*` packages.
|
||||
- Run TypeScript with strict settings for `@ionic/react-router` and fix any fallout.
|
||||
|
||||
---
|
||||
3. **Write the migration notes**
|
||||
- Document the route shape expectations (e.g., `parent/*` when using nested outlets).
|
||||
- Call out differences vs the v5 integration, especially around history behavior and any removed APIs.
|
||||
|
||||
## Objectives for Closed Alpha
|
||||
4. **Do a quick manual pass on the test app**
|
||||
- Click through the main demos (tabs, nested outlets, overlays, swipe‑back) with an eye for anything the tests don’t currently cover.
|
||||
|
||||
### 🔴 Critical (Must Have)
|
||||
|
||||
1. **Type Safety Cleanup**
|
||||
- Update to `@types/react-router@6` and `@types/react-router-dom@6`
|
||||
- Remove or update any RRv5 type references
|
||||
- Fix TypeScript compilation errors
|
||||
|
||||
2. **Migration Guide**
|
||||
- Write comprehensive upgrade guide based on 0001-upgrade-5-to-6.md
|
||||
- Include breaking changes list
|
||||
- Provide code snippets for common patterns
|
||||
- Document API changes (component → element, exact removal, etc.)
|
||||
|
||||
3. **Automated Testing**
|
||||
- Ensure E2E test suite passes for React Router 6 test app
|
||||
- Add tests for edge cases:
|
||||
- Nested param routes
|
||||
- Index routes in nested outlets
|
||||
- Multiple wildcard routes
|
||||
- Tab context switching
|
||||
- Browser refresh on deep routes
|
||||
|
||||
4. **Known Bug Fixes**
|
||||
- Verify all "fixing" commits have corresponding tests
|
||||
- Document any workarounds or limitations
|
||||
- Test all navigation patterns manually
|
||||
|
||||
### 🟡 Important (Should Have)
|
||||
|
||||
5. **Performance Testing**
|
||||
- View item cleanup (currently 100-item limit) may need tuning
|
||||
- Test with many nested outlets
|
||||
- Test rapid navigation scenarios
|
||||
|
||||
6. **Developer Experience**
|
||||
- Clear error messages when routes misconfigured
|
||||
- Better debugging logs (or remove debug logs for production)
|
||||
- TypeScript autocomplete works correctly
|
||||
|
||||
7. **Edge Case Coverage**
|
||||
- Multiple router outlets at same level
|
||||
- Dynamic route addition/removal
|
||||
- Lazy-loaded route components
|
||||
- Redirects within nested outlets
|
||||
|
||||
### 🟢 Nice to Have
|
||||
|
||||
8. **Documentation**
|
||||
- API reference for new hooks
|
||||
- Comparison with standard React Router 6
|
||||
- Migration patterns for common Ionic React Router use cases
|
||||
|
||||
9. **Example Apps**
|
||||
- Tabs starter template updated to RRv6
|
||||
- Conference app example updated to RRv6
|
||||
|
||||
10. **Developer Tools**
|
||||
- React DevTools integration
|
||||
- Navigation history visualization
|
||||
|
||||
---
|
||||
|
||||
## Implementation Checklist for Alpha
|
||||
|
||||
### Pre-Alpha Blockers
|
||||
- [ ] Fix TypeScript types (RRv5 → RRv6)
|
||||
- [ ] Remove all debug `console.log` statements or gate with `NODE_ENV`
|
||||
- [ ] Verify E2E tests pass
|
||||
- [ ] Manual QA on test app for all navigation patterns
|
||||
- [ ] Document breaking changes
|
||||
|
||||
### Alpha Requirements
|
||||
- [ ] Migration guide published
|
||||
- [ ] Known issues list created
|
||||
- [ ] Example migration for conference app or similar
|
||||
- [ ] Alpha disclaimer in README
|
||||
- [ ] Community feedback mechanism (GitHub Discussions thread)
|
||||
|
||||
### Post-Alpha
|
||||
- [ ] Performance benchmarking vs RRv5
|
||||
- [ ] Memory leak testing
|
||||
- [ ] Cross-browser testing (Safari, Firefox, Chrome)
|
||||
- [ ] Mobile device testing (iOS/Android)
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|------|------------|--------|------------|
|
||||
| Index route bugs | Medium | High | Comprehensive testing of index routes |
|
||||
| Route matching conflicts | Medium | High | Test all wildcard + specific route combos |
|
||||
| Type errors in production | Low | Medium | Fix types before alpha |
|
||||
| Migration confusion | High | Medium | Detailed migration guide |
|
||||
| Breaking changes | High | High | Document all breaking changes |
|
||||
| Performance regression | Low | Medium | Benchmark before release |
|
||||
|
||||
---
|
||||
|
||||
## Recommendation
|
||||
|
||||
**Ready for Closed Alpha:** 🟡 With Caveats
|
||||
|
||||
The branch shows strong alignment with the design documents and implements the core architecture correctly. However, before alpha release:
|
||||
|
||||
1. **Must fix TypeScript types** - This will cause developer pain
|
||||
2. **Must create migration guide** - Users need clear upgrade path
|
||||
3. **Must verify test coverage** - We need confidence in stability
|
||||
|
||||
**Timeline Suggestion:**
|
||||
- 1-2 days: Type fixes + cleanup
|
||||
- 2-3 days: Testing + bug fixes
|
||||
- 1-2 days: Documentation
|
||||
- **Total: ~1 week to alpha-ready**
|
||||
|
||||
**Alpha Scope:**
|
||||
- Target advanced users / early adopters
|
||||
- Focus on new projects (migration can come in beta)
|
||||
- Clearly communicate breaking changes
|
||||
- Set up feedback loop (GitHub Discussions)
|
||||
|
||||
---
|
||||
|
||||
## E2E Test Results (October 30, 2025)
|
||||
|
||||
### Initial Test Results (Before Fixes)
|
||||
|
||||
```
|
||||
Total Tests: 68
|
||||
Passing: 30 (44%)
|
||||
Failing: 38 (56%)
|
||||
Test Suites: 13
|
||||
Duration: ~8 minutes
|
||||
Exit Code: 40 (FAILURE)
|
||||
```
|
||||
|
||||
**Status:** 🔴 **NOT READY FOR RELEASE** - Critical bugs identified
|
||||
|
||||
### Updated Results (After Wildcard Fixes)
|
||||
|
||||
```
|
||||
Total Tests: 69
|
||||
Passing: 36 (52%)
|
||||
Failing: 33 (48%)
|
||||
Test Suites: 13
|
||||
Duration: ~6 minutes
|
||||
Exit Code: 33 (FAILURE)
|
||||
```
|
||||
|
||||
**Status:** 🟡 **IMPROVED** - +6 tests passing (+8% pass rate)
|
||||
|
||||
**Key Finding:** React Router 6 requires `/*` wildcards on routes that contain nested outlets or child routes. Without them, parent components unmount when child routes navigate, breaking Ionic's view stack management.
|
||||
|
||||
### Test Suite Breakdown (Updated)
|
||||
|
||||
| Test Suite | Tests | Before | After | Status | Change |
|
||||
|-----------|-------|--------|-------|--------|---------|
|
||||
| dynamic-ionpage-classnames | 1 | 1/1 | 1/1 | ✅ PASS | - |
|
||||
| dynamic-routes | 3 | 3/3 | 3/3 | ✅ PASS | - |
|
||||
| **dynamic-tabs** | 3 | **0/3** | **3/3** | ✅ **FIXED** | **+3** ✅ |
|
||||
| multiple-tabs | 4 | 4/4 | 4/4 | ✅ PASS | - |
|
||||
| **nested-outlets** | 11 | **4/11** | **8/11** | 🟡 **IMPROVED** | **+4** ✅ |
|
||||
| outlet-ref | 1 | 0/1 | 0/1 | 🔴 FAIL | - |
|
||||
| overlays | 3 | 1/3 | 1/3 | 🔴 FAIL | - |
|
||||
| refs | 2 | 1/2 | 1/2 | 🔴 FAIL | - |
|
||||
| replace-actions | 1 | 0/1 | 0/1 | 🔴 FAIL | - |
|
||||
| **routing** | 28 | 11/28 | 11/28 | 🔴 **HIGH PRIORITY** | - |
|
||||
| **swipe-to-go-back** | 8 | **0/4** | **4/8** | 🟡 **IMPROVED** | **+4** ✅ |
|
||||
| **tab-context** | 2 | 0/2 | 0/2 | 🔴 FAIL | - |
|
||||
| tabs | 2 | 0/2 | 0/2 | 🔴 FAIL | - |
|
||||
|
||||
### ✅ Fixes Applied (October 30, 2025)
|
||||
|
||||
#### Key Discovery: Wildcard Routes for Nested Outlets
|
||||
|
||||
**Problem:** Routes with nested `IonRouterOutlet` components were failing because parent components unmounted when child routes navigated.
|
||||
|
||||
**Solution:** Add `/*` wildcards to routes that contain nested outlets or child routes.
|
||||
|
||||
**Pattern:**
|
||||
```typescript
|
||||
// ❌ WRONG: Parent unmounts when navigating to child routes
|
||||
<Route path="/parent" element={<ComponentWithNestedOutlet />} />
|
||||
|
||||
// ✅ CORRECT: Parent stays mounted for all child routes
|
||||
<Route path="/parent/*" element={<ComponentWithNestedOutlet />} />
|
||||
```
|
||||
|
||||
**Where to apply:**
|
||||
1. **Top-level routes** in App.tsx that render components with nested outlets
|
||||
2. **Intermediate routes** within components that themselves have nested outlets
|
||||
|
||||
**Fixes Applied:**
|
||||
|
||||
1. **Dynamic Tabs (3/3 tests now passing)**
|
||||
- App.tsx: `/dynamic-tabs` → `/dynamic-tabs/*`
|
||||
- DynamicTabs.tsx: Added `id="dynamic-tabs"` to IonRouterOutlet
|
||||
- Made Tab2 route stable (always present with conditional element)
|
||||
|
||||
2. **Nested Outlets (8/11 tests now passing, +4 improvement)**
|
||||
- App.tsx:
|
||||
- `/nested-outlet` → `/nested-outlet/*`
|
||||
- `/nested-outlet2` → `/nested-outlet2/*`
|
||||
- NestedOutlet.tsx: `/nested-outlet/secondpage` → `/nested-outlet/secondpage/*`
|
||||
- NestedOutlet2.tsx:
|
||||
- `/nested-outlet2/list` → `/nested-outlet2/list/*`
|
||||
- `/nested-outlet2/home` → `/nested-outlet2/home/*`
|
||||
|
||||
3. **Swipe-to-go-back (4/8 tests now passing, bonus improvement)**
|
||||
- Wildcard fixes improved gesture navigation reliability
|
||||
|
||||
**Remaining Issues:**
|
||||
- **Nested Outlets:** 3 tests still failing - complex back-navigation edge cases within deeply nested outlets
|
||||
- **Routing:** 17/28 tests failing - broader navigation issues requiring further investigation
|
||||
- **Tabs:** 2/2 tests failing - tab switching and back-button navigation issues
|
||||
|
||||
---
|
||||
|
||||
### Critical Bugs Identified (Initial Analysis)
|
||||
|
||||
#### 1. ✅ **Dynamic Tabs - Nested Router Error** (FIXED)
|
||||
**Impact:** Blocks dynamic tab functionality entirely
|
||||
|
||||
**Error:**
|
||||
```
|
||||
You cannot render a <Router> inside another <Router>.
|
||||
You should never have more than one in your app.
|
||||
```
|
||||
|
||||
**Failing Tests:** 3/3 dynamic-tabs tests
|
||||
**Root Cause:** Dynamically rendered tabs creating nested `BrowserRouter` or `Router` component
|
||||
**Fix Required:** Investigate DynamicTabs component - ensure no Router wrapping
|
||||
|
||||
#### 2. 🔴 **Nested Outlets Navigation Failures** (HIGH)
|
||||
**Impact:** Nested outlet navigation is broken
|
||||
|
||||
**Failing Tests:** 7/11 nested-outlets tests
|
||||
**Pattern:** Pages fail to load after navigation from parent outlet
|
||||
|
||||
**Common Error:**
|
||||
```
|
||||
Expected to find element: `div.ion-page[data-pageid=secondpage]`,
|
||||
but never found it.
|
||||
```
|
||||
|
||||
**Root Cause:**
|
||||
- Nested outlet route matching not working correctly
|
||||
- `getParentPath()` logic in StackManager may have issues (lines 83-151)
|
||||
- Relative routes in nested outlets not resolving properly
|
||||
|
||||
#### 3. 🔴 **Core Routing Issues** (HIGH)
|
||||
**Impact:** Core navigation patterns broken
|
||||
|
||||
**Failing Tests:** 17/28 routing tests
|
||||
|
||||
**Key Failure Categories:**
|
||||
- Back Navigation (5 tests) - Browser back button not working
|
||||
- Tab Switching (4 tests) - Details pages not persisting across tabs
|
||||
- Multi-Level Navigation (3 tests) - Deep navigation stacks failing
|
||||
- Query Parameters (2 tests) - Lost on back navigation
|
||||
|
||||
#### 4. 🟡 **Overlays Not Dismissing** (MEDIUM)
|
||||
**Impact:** Modals persist after navigation
|
||||
|
||||
**Failing Tests:** 2/3 overlays tests
|
||||
- Overlay dismissal works with back action ✅
|
||||
- Fails with push/replace actions ❌
|
||||
|
||||
#### 5. 🟡 **Swipe to Go Back** (MEDIUM)
|
||||
**Impact:** iOS gesture navigation broken
|
||||
|
||||
**Failing Tests:** 4/4 swipe-to-go-back tests
|
||||
**Root Cause:** Gesture handler may not be properly initialized
|
||||
|
||||
### Working Features
|
||||
|
||||
**Fully Passing Test Suites:**
|
||||
- ✅ dynamic-ionpage-classnames (1/1) - IonPage CSS classes work
|
||||
- ✅ dynamic-routes (3/3) - Dynamic route addition works
|
||||
- ✅ multiple-tabs (4/4) - Multiple tab bars navigation works
|
||||
- ✅ tabs (3/3) - Basic tab navigation works
|
||||
|
||||
### Estimated Fix Time
|
||||
|
||||
| Priority | Issue | Estimated Time |
|
||||
|----------|-------|----------------|
|
||||
| P1 | Nested Router Error | 2-4 hours |
|
||||
| P1 | Nested Outlets | 1-2 days |
|
||||
| P1 | Core Routing Issues | 2-3 days |
|
||||
| P2 | Overlay Dismissal | 4-8 hours |
|
||||
| P2 | Replace Actions | 4-8 hours |
|
||||
| P2 | Swipe to Go Back | 1 day |
|
||||
| P3 | Refs & Minor Issues | 4-8 hours |
|
||||
| **Total** | | **5-8 days** |
|
||||
|
||||
---
|
||||
|
||||
## Next Steps
|
||||
|
||||
1. **IMMEDIATE:** Fix Critical Bug #1 (Nested Router Error)
|
||||
2. **IMMEDIATE:** Fix Critical Bug #2 (Nested Outlets Navigation)
|
||||
3. **IMMEDIATE:** Fix Critical Bug #3 (Core Routing Issues)
|
||||
4. Re-run E2E tests to verify fixes
|
||||
5. If pass rate > 90%, proceed to manual QA
|
||||
6. If pass rate > 95%, consider alpha release with known issues list
|
||||
If we keep the changes tightly focused on these areas, we shouldn’t need to revisit the overall architecture again.
|
||||
|
||||
Reference in New Issue
Block a user