feat(devtools)!: make embedded dock the only client mode#1041
Merged
Conversation
The DevTools client now always renders as the shared-frame anchor inside the Vite DevTools dock, so the standalone app shell is gone: SideNav, split view, the docking panel, the first-visit welcome page and their settings entries are removed, along with the `?embed=1` detection flag they were gated behind. Drop the Vitest integration (`@nuxt/test-utils/module` + `@vitest/ui`) now that Vite+ ships its own, and hide the redundant `nuxt:devtools` anchor dock button once its per-tab members have bootstrapped.
Deploying nuxt-devtools with
|
| Latest commit: |
04ac32c
|
| Status: | ⚡️ Build in progress... |
Contributor
|
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:
📝 WalkthroughWalkthroughThe DevTools client removes sidebar, docking, split-screen, embedding, and welcome-page flows. It adopts a single-frame layout with simplified routing and frame navigation. UI option contracts, defaults, storage, tab categorization, and settings are reduced accordingly. Shared-frame registration no longer uses the embed query parameter, while tests and tooling are updated for the new layout, dock metadata, dependency set, and conditional ESLint configuration. Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/devtools/client/composables/state-tabs.ts (1)
69-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve pinned-tab priority in the new comparator.
At Line 86, sorting uses only
tab.category, so thepinnedcategory is never selected. Pinned tabs now remain in their original category rather than appearing first (and in persisted pin order); this also changes the command order consumed bystate-commands.ts.Proposed fix
- .sort((a, b) => categoryOrder.indexOf(a.category || 'app') - categoryOrder.indexOf(b.category || 'app'))) + .sort((a, b) => { + const pinnedTabs = settings.pinnedTabs.value + const aPinned = pinnedTabs.includes(a.name) + const bPinned = pinnedTabs.includes(b.name) + + if (aPinned && bPinned) + return pinnedTabs.indexOf(a.name) - pinnedTabs.indexOf(b.name) + + const aCategory = aPinned ? 'pinned' : (a.category || 'app') + const bCategory = bPinned ? 'pinned' : (b.category || 'app') + return categoryOrder.indexOf(aCategory) - categoryOrder.indexOf(bCategory) + }))🤖 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 `@packages/devtools/client/composables/state-tabs.ts` around lines 69 - 86, Update the comparator in the computed tab list around the existing categoryOrder sort to prioritize pinned tabs before category ordering, while preserving persisted pin order among pinned tabs and their original ordering for non-pinned tabs. Ensure the resulting command order consumed by state-commands.ts remains consistent with this pinned-first behavior.
🤖 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 `@packages/devtools/src/runtime/plugins/view/client.ts`:
- Around line 323-354: Bound the polling in hideAnchorOnceSubTabsReady so it
cannot run indefinitely when the nuxt:devtools metadata never appears. Add a
maximum timeout or attempt limit, and clear the interval during teardown or HMR
using the module’s existing lifecycle/disposal mechanism while preserving the
current behavior when members become available.
---
Outside diff comments:
In `@packages/devtools/client/composables/state-tabs.ts`:
- Around line 69-86: Update the comparator in the computed tab list around the
existing categoryOrder sort to prioritize pinned tabs before category ordering,
while preserving persisted pin order among pinned tabs and their original
ordering for non-pinned tabs. Ensure the resulting command order consumed by
state-commands.ts remains consistent with this pinned-first behavior.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 79d5396b-ad35-4e84-83b3-da9fa767cb0b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
packages/devtools-kit/src/_types/options.tspackages/devtools/client/app.vuepackages/devtools/client/components/DockingPanel.vuepackages/devtools/client/components/SideNav.vuepackages/devtools/client/components/SideNavItem.vuepackages/devtools/client/components/SplitScreen.vuepackages/devtools/client/components/TabsGrid.vuepackages/devtools/client/components/TimelineView.vuepackages/devtools/client/composables/embed.tspackages/devtools/client/composables/frame-nav.tspackages/devtools/client/composables/state-tabs.tspackages/devtools/client/composables/storage.tspackages/devtools/client/middleware/route.global.tspackages/devtools/client/nuxt.config.tspackages/devtools/client/pages/index.vuepackages/devtools/client/pages/modules/overview.vuepackages/devtools/client/pages/settings.vuepackages/devtools/package.jsonpackages/devtools/src/constant.tspackages/devtools/src/module-main.tspackages/devtools/src/runtime/plugins/view/client.tspnpm-workspace.yamltests/e2e/fixtures/devtools.tstests/e2e/specs/iframe.spec.tstests/e2e/specs/nuxt-group.spec.ts
💤 Files with no reviewable changes (14)
- packages/devtools/client/components/SideNavItem.vue
- packages/devtools/client/nuxt.config.ts
- packages/devtools/client/composables/embed.ts
- packages/devtools/client/components/TabsGrid.vue
- packages/devtools/client/components/SplitScreen.vue
- packages/devtools/client/pages/index.vue
- packages/devtools/client/components/TimelineView.vue
- packages/devtools/client/components/DockingPanel.vue
- packages/devtools/src/constant.ts
- packages/devtools-kit/src/_types/options.ts
- packages/devtools/client/components/SideNav.vue
- packages/devtools/package.json
- pnpm-workspace.yaml
- packages/devtools/client/pages/settings.vue
The `ci`/`autofix` workflows ran `pnpm lint` before the client Nuxt app was prepared, so the root ESLint config's import of the generated `packages/devtools/client/.nuxt/eslint.config.mjs` failed. Add a `pnpm dev:prepare` step before lint. The code-server and data-inspector dock entries now register under the `modules` / `advanced` in-group categories (no `defaultOrder`); update their unit and e2e expectations, which still asserted the old `framework` / `-200`.
Revert the CI-workflow edits (a bot push that touches `.github/workflows/**` suppresses Actions from running) and instead make the root ESLint config resilient: when the client Nuxt app hasn't been prepared yet — e.g. a fresh `pnpm install && pnpm lint` in CI — skip its generated flat config and register `eslint-plugin-unimport` as a fallback so the client's inline `unimport/auto-insert` disable directives still resolve.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
eslint.config.mjs (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise both generated-config branches in validation.
hasClientConfigcreates materially different lint configurations. Add checks for both a fresh unprepared checkout and a prepared client; otherwise regressions in either the fallback or dynamic-import path can go unnoticed.As per coding guidelines, use the documented
pnpmvalidation commands, includingpnpm lint; runpnpm prepareorpnpm buildbeforepnpm typecheck.🤖 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 `@eslint.config.mjs` around lines 10 - 11, Add validation coverage for both hasClientConfig branches in the ESLint configuration: run the documented pnpm lint command from a fresh unprepared checkout and again after generating the client configuration with pnpm prepare or pnpm build. Run pnpm typecheck only after preparation, and ensure both fallback and dynamically imported configurations are exercised.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 10-11: Add validation coverage for both hasClientConfig branches
in the ESLint configuration: run the documented pnpm lint command from a fresh
unprepared checkout and again after generating the client configuration with
pnpm prepare or pnpm build. Run pnpm typecheck only after preparation, and
ensure both fallback and dynamically imported configurations are exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8abcb62d-1d90-402d-ac0d-27039ed2a934
📒 Files selected for processing (1)
eslint.config.mjs
Bump `@devframes/hub` to 0.7.13 and adopt its new render-only `visibility` field (devframe#136): set `visibility: 'false'` on the shared-frame anchor so only its synthesized per-tab members render, while the entry stays registered and keeps hosting the shared iframe / driving the frame-nav loop. This replaces the host-side client-dock override hack that dynamically set `when: 'false'` after members bootstrapped. The dock-bar button is suppressed by the viewer once it honors `visibility` (vitejs/devtools#470, @vitejs/devtools 0.4.7); the field is inert on older viewers.
Bump `@vitejs/devtools` and `@vitejs/devtools-kit` to 0.4.7, whose viewer honors the render-only `visibility` field (vitejs/devtools#470). Together with `@devframes/hub` 0.7.13, the anchor's `visibility: 'false'` now actually suppresses its dock-bar button while the entry keeps driving the shared frame.
…ols-client # Conflicts: # pnpm-lock.yaml # pnpm-workspace.yaml
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
2 participants
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.
Summary
Building on #1035, the DevTools client is now always the shared-frame anchor embedded in the Vite DevTools dock, so the whole standalone app shell becomes dead weight. This PR removes everything that is not the embedded mode, drops the now-superseded Vitest integration, and hides the redundant anchor dock button.
Embed is the only mode
SideNav,SideNavItem,TabsGrid,DockingPanel,SplitScreen, theNSplitPanesplit view, and the split-screen command.?embed=1detection composable and everyisEmbedded/isUtilityViewbranch — the client renders a single tab directly and always starts thedevframe:frame-navshim.index.vue) and its flow; the route middleware now always lands on/modules/overview.sidebarExpanded/sidebarScrollable/interactionCloseOnOutsideClickoptions they drove.?embed=1marker from the dock URL.Remove the Vitest integration
Vite+ now ships its own Vitest surface, so the DevTools client no longer registers
@nuxt/test-utils/module(dropping the@nuxt/test-utils+@vitest/uidevDependencies and the unused@vitest/uicatalog entry).Hide the redundant
nuxt:devtoolsanchorThe anchor must stay a selectable entry long enough to bootstrap its per-tab members, so it can't be statically hidden. Once those members exist, the Nuxt host suppresses the anchor's own dock button by overriding its entry with a falsy
whenclause (preserving itssubTabs/frameId, so the shared iframe keeps working).Notes
pnpm lint,pnpm typecheck, andpnpm test:unitpass (the one pre-existing, unrelatedcode-server.test.tsfailure also fails on the base branch).This PR was created with the help of an agent.