[033-pig-ipc-api-layer]: Extract pig/drag IPC into api/ layer - #39
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR extracts Tauri IPC (invoke/listen) into a frontend API module ( ChangesPig API Abstraction
Task Configuration
Sequence Diagram(s)sequenceDiagram
participant App as App (React)
participant Hook as usePigMovement
participant API as src/api/pig.ts
participant Rust as Tauri/Rust
App->>Hook: user interactions (drag start/move/end)
Hook->>API: updatePigRects(rects)
API->>Rust: invoke update_pig_rects
Rust-->>API: ack/event responses
Rust->>API: emit gather-pigs / display-region events
API->>Hook: subscribed callbacks invoked
Hook->>App: update local state (pigs / region)
App->>Hook: setDragActive(active)
Hook->>API: setPigDragActive(active)
API->>Rust: invoke set_pig_drag_active
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/pig.ts`:
- Line 3: The import of PigHitRect and SpawnRegion in api/pig.ts creates an
inverted dependency on the hooks layer; extract these interfaces into a new
shared types module (e.g., types/pig) that declares the SpawnRegion and
PigHitRect interfaces, then update api/pig.ts and usePigMovement to import
PigHitRect and SpawnRegion from that shared module instead of from
usePigMovement so the api layer no longer depends on the hooks layer.
In `@src/components/App.tsx`:
- Around line 33-36: The effect currently assigns unsubPromise =
subscribeGatherPigs(gather) (and similarly subscribeDisplayRegion) and returns a
cleanup that assumes the promise resolves; if the promise rejects this yields
unhandled rejections. Fix by handling promise rejections: when creating
unsubPromise (from subscribeGatherPigs and subscribeDisplayRegion) attach a
.catch handler or use an async IIFE with try/catch to log the error and set a
safe no-op unsubscribe; ensure the cleanup calls a defined function (e.g., a
no-op) if the subscription failed so unsubPromise.then(unsub => unsub()) cannot
throw. Reference subscribeGatherPigs, subscribeDisplayRegion and the
unsubPromise/cleanup logic in the effect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35cce2cb-fe2b-4006-b9fc-826cbaa2c8db
📒 Files selected for processing (3)
src/api/pig.tssrc/components/App.tsxsrc/hooks/usePigMovement.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/App.tsx`:
- Around line 28-46: Extract IPC side-effects and related state from App into a
new hook (e.g., usePigMovement): move the logic that calls setPigDragActive
(used by handleSetDragActive), subscribeGatherPigs(gather) and
subscribeDisplayRegion(setRegion) into the hook so the hook owns
subscribing/unsubscribing and error fallback handling (preserve the catch(() =>
() => {}) pattern and unsubPromise.then(unsub => unsub())); have the hook manage
any local state (gather/region) and expose a stable setDragActive callback
(wrapping setPigDragActive with proper error handling) and the region/gather
values; then update App to be view-only by removing direct calls to
setPigDragActive/subscribe* and using usePigMovement to get the handler and
state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17c07d3c-1f7e-402f-be3c-6cfcfea0d84f
📒 Files selected for processing (5)
ralph/PLAN.mdsrc/api/pig.tssrc/components/App.tsxsrc/hooks/usePigMovement.tssrc/types/pig.ts
Closes issues/033-pig-ipc-api-layer.md Completion promise: No invoke or listen calls remain in src/components/ or src/hooks/; all pig/drag IPC lives in src/api/pig.ts. - New src/api/pig.ts: setPigDragActive, updatePigRects, subscribeGatherPigs, subscribeDisplayRegion typed wrappers - New src/types/pig.ts: SpawnRegion, PigHitRect (shared so api/ does not import from hooks/) - App.tsx + usePigMovement.ts: drop tauri invoke/listen imports, use pig api wrappers; subscriptions wrap a no-op fallback so cleanup cannot throw an unhandled rejection on strict-mode unmount - Drop stale ralph/ Taskfile include left over from dabed05
archae0pteryx
force-pushed
the
chore/pig-ipc-api-layer
branch
from
May 5, 2026 02:29
fa1e615 to
e8d3b75
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/App.tsx (1)
4-46: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove pig API side-effects out of
Appso the component stays view-only.
Appstill owns I/O (setPigDragActiveandsubscribe*effects). That responsibility should live inusePigMovement(or a dedicated hook), withAppconsuming only state/callbacks.As per coding guidelines
src/components/**/*.{ts,tsx}should be view-only with no direct I/O, andsrc/hooks/**/*.{ts,tsx}should handle state + effects and callapi/clients.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/App.tsx` around lines 4 - 46, App currently performs I/O (calls setPigDragActive and calls subscribeGatherPigs/subscribeDisplayRegion in useEffect), which must be moved into the hook layer; remove handleSetDragActive and the two subscribe useEffect blocks from App and instead implement those side-effects inside usePigMovement (or a new dedicated hook) so the hook: 1) watches drag state and calls setPigDragActive(active) (exposing only startDrag/moveDrag/endDrag or a setter, not performing the API call in App), 2) calls subscribeGatherPigs(gather) and manages the unsubscribe promise internally (accepting the gather callback or using an internal gather handler), and 3) calls subscribeDisplayRegion(setRegion) and handles unsubscribe internally; update usePigMovement's signature to accept/return the necessary callbacks/state (e.g., accept focuses and selectedId and return pigs, startDrag/moveDrag/endDrag and any setter needed) so App becomes view-only and no longer imports or calls setPigDragActive, subscribeGatherPigs, or subscribeDisplayRegion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/components/App.tsx`:
- Around line 4-46: App currently performs I/O (calls setPigDragActive and calls
subscribeGatherPigs/subscribeDisplayRegion in useEffect), which must be moved
into the hook layer; remove handleSetDragActive and the two subscribe useEffect
blocks from App and instead implement those side-effects inside usePigMovement
(or a new dedicated hook) so the hook: 1) watches drag state and calls
setPigDragActive(active) (exposing only startDrag/moveDrag/endDrag or a setter,
not performing the API call in App), 2) calls subscribeGatherPigs(gather) and
manages the unsubscribe promise internally (accepting the gather callback or
using an internal gather handler), and 3) calls
subscribeDisplayRegion(setRegion) and handles unsubscribe internally; update
usePigMovement's signature to accept/return the necessary callbacks/state (e.g.,
accept focuses and selectedId and return pigs, startDrag/moveDrag/endDrag and
any setter needed) so App becomes view-only and no longer imports or calls
setPigDragActive, subscribeGatherPigs, or subscribeDisplayRegion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef1eb248-9fb6-45ac-a729-797a0bc9e4e3
📒 Files selected for processing (5)
Taskfile.yamlsrc/api/pig.tssrc/components/App.tsxsrc/hooks/usePigMovement.tssrc/types/pig.ts
💤 Files with no reviewable changes (1)
- Taskfile.yaml
1 task
App was still calling setPigDragActive and subscribing to gather-pigs / display-region directly. CLAUDE.md requires components to be view-only and hooks to own state + effects. Hook now exposes setDragActive and manages both subscriptions internally.
This was referenced May 5, 2026
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
1 participant
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.
Closes issues/033-pig-ipc-api-layer.md
Summary
src/api/pig.tsexports four typed wrappers:setPigDragActive,updatePigRects,subscribeGatherPigs,subscribeDisplayRegionsrc/types/pig.tssharesSpawnRegionandPigHitRectsoapi/does not depend onhooks/App.tsxandusePigMovement.tsdrop direct tauriinvoke/listen; subscriptions wrap a no-op fallback so cleanup cannot throw an unhandled rejection on strict-mode unmountralph/Taskfile include left over from dabed05Test plan
task checkgreeninvoke/listenin production code undersrc/componentsorsrc/hooks(remaining grep hits are test mocks)Summary by CodeRabbit
Refactor
Tests
Chores