chore: archive legacy recorder #895
Conversation
…ps) => { to legacy
WalkthroughImport paths were updated across multiple legacy UI and context components to reference files under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
legacy/src/LeftSidePanelContent.tsx (1)
44-49: Socket listener cleanup never removes the registered handlerIn the
useEffectthat subscribes to"activePairId", the cleanup uses a new arrow function:socket?.on("activePairId", (data) => activePairIdHandler(data, socket)); return () => { socket?.off("activePairId", (data) => activePairIdHandler(data, socket)); };Because
socket.offreceives a different function reference, the original listener isn’t actually detached. If the socket instance ever changes and this effect re‑runs, you can end up with multiple active listeners.A safer pattern is to reuse the same handler reference:
- useEffect(() => { - socket?.on("activePairId", (data) => activePairIdHandler(data, socket)); - return () => { - socket?.off("activePairId", (data) => activePairIdHandler(data, socket)); - } - }, [socket, setActiveId]); + useEffect(() => { + if (!socket) return; + + const handler = (data: string) => activePairIdHandler(data, socket); + + socket.on("activePairId", handler); + return () => { + socket.off("activePairId", handler); + }; + }, [socket, activePairIdHandler]);This ensures
offsees the exact same function instance that was passed toon.legacy/src/DisplayWhereConditionSettings.tsx (1)
2-7: Import path migration tosrc/componentslooks consistentThe updated imports for
MuiDropdown,AddButton,RemoveButton,KeyValueForm, andWarningTextall correctly point two levels up fromlegacy/srcintosrc/components/.... This aligns with the described refactor and doesn’t affect runtime logic.If you expect more of these legacy files to keep living for a while, you might later consider TS path aliases (e.g.
@components/ui/...) instead of deep../../src/...hops to make future moves cheaper, but that’s optional for this chore PR.legacy/src/AddWhatCondModal.tsx (1)
2-8: Updated imports correctly target centralizedsrcmodulesThe imports for
GenericModal,KeyValueForm,ClearButton, anduseSocketStorenow consistently reference thesrc/...structure using../../src/...fromlegacy/src, which matches the overall migration pattern and keeps behavior unchanged.As with the other legacy file, consider path aliases in the future to avoid long relative segments, but it’s not necessary to merge this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
legacy/src/AddWhatCondModal.tsx(1 hunks)legacy/src/AddWhereCondModal.tsx(1 hunks)legacy/src/DisplayWhereConditionSettings.tsx(1 hunks)legacy/src/LeftSidePanel.tsx(1 hunks)legacy/src/LeftSidePanelContent.tsx(1 hunks)legacy/src/LeftSidePanelSettings.tsx(1 hunks)legacy/src/PairDetail.tsx(1 hunks)src/components/recorder/Pair.tsx(1 hunks)src/components/run/ColapsibleRow.tsx(1 hunks)src/components/run/RunSettings.tsx(1 hunks)
🔇 Additional comments (8)
legacy/src/LeftSidePanelSettings.tsx (1)
3-5: Import normalization to src/ modules looks consistent*Repointing
Dropdown,RunSettings, anduseSocketStoreto the../../src/...locations keeps this legacy panel wired into the shared UI/context implementations without changing behavior. The types and props used below still match, so this path refactor looks good.src/components/run/RunSettings.tsx (1)
6-6: RunSettings now consuming modalStyle from CollapsibleRow is safeWiring
modalStylein from../run/ColapsibleRowkeeps run‑related modal styling in one place and doesn’t introduce a circular dependency.GenericModalusage is unchanged, so this is a clean, non‑behavioral refactor.legacy/src/LeftSidePanelContent.tsx (1)
2-8: Path updates to src/ recorder and UI modules are coherent*
Pair,useSocketStore,AddButton,AddPair, andGenericModalnow all resolve via../../src/..., which keeps this legacy content panel wired into the shared, non‑legacy implementations. The usage of each import below hasn’t changed, so this is a straightforward path normalization.legacy/src/AddWhereCondModal.tsx (1)
1-1: UI and socket imports now consistently use src/ modules*Pointing
MuiDropdown,GenericModal, anduseSocketStoreat../../src/...keeps this legacy modal using the shared UI and socket store implementations, while still relying on its own localmodalStyle. The component logic and emitted socket payloads are unchanged, so this is a safe structural cleanup.Also applies to: 8-8, 12-12
legacy/src/LeftSidePanel.tsx (1)
3-5: Import normalization to src/ for workflow, context, and settings*
getActiveWorkflow,getParamsOfActiveWorkflow,useSocketStore,emptyWorkflow,useGlobalInfoStore, andRunSettingsnow all come from thesrctree, which centralizes shared logic while leaving panel behavior intact. The way these symbols are used (API calls, context access, andRunSettingsstate) hasn’t changed, so this is a clean path refactor.Also applies to: 7-7, 9-9, 11-11
src/components/run/ColapsibleRow.tsx (1)
233-243: Exporting modalStyle from CollapsibleRow for shared run modalsDefining and exporting
modalStylehere matches how it’s used inside this component and byRunSettingsModal, and keeps run‑related modal styling co‑located with the runs UI rather than in an unrelated recorder module. Since it’s a plain object at module scope, referencing it from JSX above is safe.legacy/src/PairDetail.tsx (1)
9-12: WarningText import path is correctThe import
import { WarningText } from "../../src/components/ui/texts"correctly resolves tosrc/components/ui/texts.tsx, which exportsWarningTextat line 3. This import will work at build time; no changes are needed.src/components/recorder/Pair.tsx (1)
7-8: The legacy imports resolve correctly. BothPairEditForm.tsxandPairDisplayDiv.tsxexist atlegacy/src/and the relative paths fromsrc/components/recorder/Pair.tsxare correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
legacy/src/Pair.tsx (1)
3-10: Import path updates are consistent with the new src layoutThe updated imports to
"../../src/api/..." and"../../src/components/ui/..."are internally consistent and match what we’d expect from moving shared code undersrc/. No behavior change is introduced here, so this looks safe.If you plan more work on these legacy files later, you might eventually consider using TS path aliases (e.g.
@/api,@/components/ui) instead of long relative hops, but that’s optional and outside the scope of this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
legacy/src/LeftSidePanelContent.tsx(1 hunks)legacy/src/Pair.tsx(1 hunks)
🔇 Additional comments (1)
legacy/src/LeftSidePanelContent.tsx (1)
4-8: All import paths resolve correctly.The updated import paths from
legacy/src/LeftSidePanelContent.tsxto the mainsrc/directory are valid and properly structured. All target files exist:
src/context/socket.tsx✓src/components/ui/buttons/AddButton.tsx✓src/api/workflow.ts✓src/components/ui/GenericModal.tsx✓The reorganization aligns with the PR objective.
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.