Skip to content

feat: use @pierre/trees for Studio file tree#424

Draft
miguel-heygen wants to merge 1 commit intomainfrom
codex/studio-tree-viewer
Draft

feat: use @pierre/trees for Studio file tree#424
miguel-heygen wants to merge 1 commit intomainfrom
codex/studio-tree-viewer

Conversation

@miguel-heygen
Copy link
Copy Markdown
Collaborator

What changed

  • replaced the Studio file tree with an @pierre/trees-based wrapper while keeping the existing Studio actions and affordances wired through it
  • preserved create, rename, delete, duplicate, import targeting, root and row context menus, selection syncing, and empty-folder .gitkeep handling
  • added focused tests for path shaping, drop targeting, move destination building, and pending-create lifecycle behavior

Why

Studio needed the richer tree viewer from @pierre/trees, but the first integration attempt exposed a wrapper bug: inline renames for newly created items could lose pending-create state and fall through to a rename request instead of the Studio create API. This patch fixes that race so new files and folders persist correctly after the inline rename flow.

Impact

  • Studio gets the new tree viewer library without regressing the existing file operations UX
  • newly created files and folders now reliably hit the correct Studio backend APIs during inline rename

Validation

  • bunx oxfmt packages/studio/src/App.tsx packages/studio/src/components/editor/FileTree.tsx packages/studio/src/components/editor/FileTree.test.ts
  • bunx oxlint packages/studio/src/App.tsx packages/studio/src/components/editor/FileTree.tsx packages/studio/src/components/editor/FileTree.test.ts
  • bun run --cwd packages/studio test src/components/editor/FileTree.test.ts
  • bun run --cwd packages/studio typecheck
  • agent-browser browser verification on local Studio for file-create and folder-create inline rename flows, including persistence after reload

Notes

  • left an unrelated untracked .thumbnails/ artifact out of the PR scope
  • keeping this PR as a draft so drag/drop move behavior can still get a final manual browser pass on GitHub if desired

@miguel-heygen miguel-heygen changed the title [codex] use @pierre/trees for Studio file tree feat: use @pierre/trees for Studio file tree Apr 22, 2026
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve on code merit — do not un-draft or merge until the rebase + CI fixes below land

Clean wrapper architecture. The Studio-side surface (FileTreeProps, create/rename/delete/duplicate/move/import callbacks) is preserved intact, and the internal mechanics match @pierre/trees' documented extension points (shadow DOM via host.shadowRoot, unsafeCSS injection for the drag-target highlight, renderContextMenu slot). Staff-level skim finds no behavioral regressions in the preserved API.

What the tests actually prove

buildStudioTreePaths / createPlaceholderPath / getDropPathData / buildMoveDestinationPath / isPendingCreateCleared are all pure functions extracted from the component — and the test suite hits the one semantic this PR is really about: isPendingCreateCleared("keeps pending creates alive across rename moves"). That's the exact race the PR body calls out (inline-rename of a newly created placeholder used to fall through to the Studio rename API instead of the create API). The test locks that in, and the component's renaming.onRename handler correctly branches on pending.placeholderPath === event.sourcePath before deciding which callback to fire. Fix is surgical.

Expansion persistence across displayPaths changes (via collectExpandedPaths(host) + model.resetPaths(…, {initialExpandedPaths})) is the right shape — without it, any backend file refresh would collapse all open folders. Nice detail.

Things worth noting (non-blocking)

  1. useEffect(() => { hostRef.current = getHostElement(...); }) with no deps runs on every render. The next effect with [displayPaths, model] also refreshes hostRef.current, so the unkeyed version is defensive chain on top. Cheap, but if you ever profile render work this is a free effect-callback you can drop.
  2. canDrag: (paths) => paths.length === 1 — multi-select drag is disabled. Consistent with existing Studio behavior as far as I can tell, but worth noting: if the old tree supported multi-drag anywhere (even unintentionally), this is a silent removal.
  3. @pierre/trees bundle-size impact wasn't in the PR body. A one-liner in the description with the bun run build --filter @hyperframes/studio before/after bundle sizes would close the loop for the ops-side concern.
  4. Keyboard-nav / accessibility coverage is effectively "we trust @pierre/trees" — the PR body's browser verification is scoped to inline-rename, not focus management / arrow keys / roving tabindex. Worth a focused agent-browser pass on: tab into tree → arrow up/down → enter to open → shift+F10 for context menu → escape to close. Those are the failure modes that wouldn't show up in the FileTree.test.ts suite.

🚨 Before un-drafting (blockers)

  1. Stale base. PR was opened 2026-04-22 on 7800a9ff; main has moved substantially since (multiple merges today: #458 / #461 / #353 / #463). Needs a rebase onto current main before merge — and CI reruns on the rebased head.
  2. Format check was FAILURE on the current head — bun run format should clean that.
  3. Semantic PR title check was FAILURE on the current head — the title feat: use @pierre/trees for Studio file tree is structurally valid conventional-commit, so this is likely a scope-requirement config; verify what the check wants (maybe feat(studio): …) and update accordingly.

Browser-test ask — honest capability gap

My session doesn't have Playwright / Puppeteer / a Studio dev-server + headed-browser setup wired in. I can't run the agent-browser verification pass myself without the tooling you used locally. Your own verification is captured in the PR body (file-create + folder-create inline rename + persistence across reload); for the drag/drop behavior you flagged as wanting "a final manual browser pass", that still needs to happen before un-drafting — either you locally, or routed to someone on the team with the Studio dev-server + browser-automation setup.

On code merit, stamp is clean. The draft status plus the above blockers are the right gate for un-drafting.


Review by hyperframes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants