refactor: signal based reactivity#6704
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplace the central Router __store with a granular, server-aware RouterStores system; RouterCore now accepts a store factory; public RouterState drops pendingMatches/cachedMatches; adapters, devtools, plugins, and tests updated to use per-store subscriptions and adapter-specific store factories. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client App
participant StoreFactory as Store Factory (getStoreFactory)
participant ReactiveStores as RouterStores (per-adapter)
participant MatchComp as Match component
Client->>StoreFactory: initialize Router (isServer: false)
StoreFactory->>ReactiveStores: create reactive stores + batch + init
ReactiveStores-->>StoreFactory: return stores
Client->>MatchComp: render Match
MatchComp->>ReactiveStores: subscribe to per-match store (useStore/useStore-like)
ReactiveStores-->>MatchComp: push per-match updates
MatchComp-->>Client: re-render UI
sequenceDiagram
participant Server as SSR
participant StoreFactory as Store Factory (getStoreFactory)
participant SSRStores as Non-reactive RouterStores
participant MatchComp as Match component
Server->>StoreFactory: initialize Router (isServer: true)
StoreFactory->>SSRStores: create non-reactive stores (snapshots)
SSRStores-->>StoreFactory: return stores
Server->>MatchComp: render Match (read-only)
MatchComp->>SSRStores: read store.state directly
SSRStores-->>MatchComp: return snapshot
MatchComp-->>Server: produce HTML
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
View your CI Pipeline Execution ↗ for commit 06c3127
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5d2f3fb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🚀 Changeset Version Preview12 package(s) bumped directly, 19 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
Three drift points fixed where the rebase made them obvious: - packages/react-native-router/vite.config.ts: @tanstack/config/vite was renamed to @tanstack/vite-config on main; updated import + added tsconfigPath: './tsconfig.build.json' to match the convention. - packages/react-native-router/tsconfig.build.json: added (matches the pattern from router-plugin/start-plugin-core). - packages/router-generator/src/template.ts (react-native target): main removed config.verboseFileRoutes; the RN target now uses the same serializeRoutePath() pattern as the react/solid/vue targets. Remaining migration work in react-native-router (10 TS errors against main's router-core): - Matches.tsx uses RouterState.pendingMatches and RouterState.cachedMatches which no longer exist on main (state was refactored as part of the signal-based core in #6704 and follow-ups). Need to redesign the pending-matches rendering logic against the new state shape. - useRouterState.tsx accesses router.__store directly; main moved this to router.stores.__store. - Transitioner.tsx's getLocationChangeInfo signature changed. - Router constructor now requires a getStoreConfig argument. These are real engineering tasks (not mechanical drift) and belong to a proper feat/react-native → main migration commit, not this batch.
Benchmarks
Custom benchmarks (separate from the codspeed ones), made to exaggerate the impact of this PR:
total_cycle_msis a Router test, runs onplaywright, measures "how long to navigate 100 times" on a rerender-heavy pagemedian_req_per_secis a Start test, runs onautocannon, using the previously established "minimal", "100 links", "many nested routes" appsResults:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor