-
-
Notifications
You must be signed in to change notification settings - Fork 2
fix: enable nuxt devtools and fix image flicker #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe PR updates routing and asset handling: src/index.js expands Nuxt route coverage to include '/nuxt_assets' alongside '/_nuxt' and '/cli', introduces NUXT_DEV_ONLY_ROUTES, NUXT_PROD_ONLY_ROUTES, and NUXT_ROUTES (conditional on isDev) and exports them. Multiple Vue components and pages switch image sources to absolute '/nuxt_assets/...' paths. nuxt.config.ts adds a public directory configuration pointing to app/public/. Overall changes adjust route matching, exported route constants, and where static assets are sourced and served. Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/index.js (1)
260-262: Clarify ternary spread inNUXT_ROUTESand reconsider empty prod-only constantRight now the spread element relies on ternary precedence, which is valid but slightly opaque at a glance:
const NUXT_ROUTES = [ '/cli', '/_nuxt', ...isDev ? NUXT_DEV_ONLY_ROUTES : NUXT_PROD_ONLY_ROUTES ];To make intent unambiguous and easier to skim, consider wrapping the ternary:
-const NUXT_ROUTES = [ '/cli', '/_nuxt', ...isDev ? NUXT_DEV_ONLY_ROUTES : NUXT_PROD_ONLY_ROUTES ]; +const NUXT_ROUTES = [ + '/cli', + '/_nuxt', + ...(isDev ? NUXT_DEV_ONLY_ROUTES : NUXT_PROD_ONLY_ROUTES), +];Also,
NUXT_PROD_ONLY_ROUTESis currently an empty array. Either drop it and inline[]in the ternary, or add a short comment that it’s intentionally reserved for future prod-only Nuxt routes so it doesn’t look like an oversight.Finally, since this array now controls exposure of the devtools path, double‑check that your CI/preview environments don’t run with
NODE_ENV=development, otherwise the devtools route will be exposed there as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
src/index.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping.io PR: 58
File: src/lib/initialize-nuxt/index.js:24-27
Timestamp: 2025-12-04T15:13:06.242Z
Learning: In the jsdelivr/globalping.io repository, the custom Nuxt/Koa integration does NOT generate .output/server/index.mjs. The correct pattern for production is to import from .output/server/chunks/nitro.mjs using destructuring { i: useNitroApp } as shown in src/lib/initialize-nuxt/index.js. This is not a fragile pattern but the intended integration method for their custom server setup.
📚 Learning: 2025-12-04T15:13:06.242Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping.io PR: 58
File: src/lib/initialize-nuxt/index.js:24-27
Timestamp: 2025-12-04T15:13:06.242Z
Learning: In the jsdelivr/globalping.io repository, the custom Nuxt/Koa integration does NOT generate .output/server/index.mjs. The correct pattern for production is to import from .output/server/chunks/nitro.mjs using destructuring { i: useNitroApp } as shown in src/lib/initialize-nuxt/index.js. This is not a fragile pattern but the intended integration method for their custom server setup.
Applied to files:
src/index.js
🔍 Remote MCP DeepWiki
Findings — quick, review-focused
- Nuxt is initialized only in development (initNuxt runs when isDev/dev env) and the Nuxt route handler for /new/* is registered in dev. Confirm any dev-only routes are gated by that same isDev check.
- Nuxt DevTools is enabled in nuxt.config.ts (dev-only tool). DevTools are a development-time utility and should not be exposed in production. Verify the PR doesn’t remove the isDev gating or enable Nuxt/devtools in prod/preview.
- The Nuxt route handler used in src/index.js checks for the handler, sets ctx.req.ctx = ctx, sets ctx.respond = false, then awaits nuxtRouteHandler(req,res). Any new dev route (e.g. '/nuxt_devtools') should follow the same pattern (and check nuxtRouteHandler != null) to avoid Koa double-response or 500s.
- isDev is derived from NODE_ENV; ensure preview/deployers (Render, CI, preview sites) do not set NODE_ENV=development or otherwise expose dev-only routes. If previews need the route, consider an explicit guard (e.g. only localhost or trusted preview flag).
- Because Nuxt devtools can reveal internals, confirm the change only adds the devtools route to NUXT_DEV_ONLY_ROUTES (not to prod routes) and that CI/tests still treat it as dev-only.
Reviewer checklist (concrete things to verify in this PR)
- src/index.js — initNuxt / nuxt route registration: verify isDev gating, nuxtRouteHandler null-check, and use of ctx.respond = false before calling handler.
- nuxt.config.ts — confirm devtools: { enabled: true } is not being promoted to production configuration.
- Ensure the new '/nuxt_devtools' appears only in the dev-only route group (NUXT_DEV_ONLY_ROUTES) and not in production route list.
- Confirm CI / preview environments won't accidentally set NODE_ENV=development (or otherwise trigger isDev) — if they do, consider adding an explicit check (e.g. only allow devtools on localhost or when a safe flag is set).
- Manual test: run dev server, visit the new devtools path and /new/* to confirm Nuxt dev server handles them; run production build and confirm the devtools path 404s.
Files/locations to inspect
- src/index.js — initNuxt(), nuxt route handler (/^/(new)(/.+)?$/) and where NUXT_ROUTES / NUXT_DEV_ONLY_ROUTES are composed.
- nuxt.config.ts — devtools setting.
- src/middleware/render/index.js — confirm no accidental exposure via render middleware.
If those checks pass, the PR approach (moving dev-only Nuxt routes into a dev-only group and enabling devtools for dev) aligns with the repo’s existing Nuxt/dev gating and SSR integration.
Sources
- Repository documentation and code summaries.,,
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
app/public/nuxt_assets/grid-2.svgis excluded by!**/*.svgapp/public/nuxt_assets/icons/copy.svgis excluded by!**/*.svgapp/public/nuxt_assets/icons/deb.svgis excluded by!**/*.svgapp/public/nuxt_assets/icons/github.small.svgis excluded by!**/*.svgapp/public/nuxt_assets/icons/homebrew.svgis excluded by!**/*.svgapp/public/nuxt_assets/icons/rpm.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
app/components/CopyButton.vue(1 hunks)app/components/cli/InstallCommands.vue(3 hunks)app/pages/cli.vue(2 hunks)nuxt.config.ts(1 hunks)src/index.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
nuxt.config.tsapp/components/CopyButton.vueapp/components/cli/InstallCommands.vueapp/pages/cli.vue
🧠 Learnings (1)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping.io PR: 58
File: src/lib/initialize-nuxt/index.js:24-27
Timestamp: 2025-12-04T15:13:06.242Z
Learning: In the jsdelivr/globalping.io repository, the custom Nuxt/Koa integration does NOT generate .output/server/index.mjs. The correct pattern for production is to import from .output/server/chunks/nitro.mjs using destructuring { i: useNitroApp } as shown in src/lib/initialize-nuxt/index.js. This is not a fragile pattern but the intended integration method for their custom server setup.
🔍 Remote MCP DeepWiki
Summary of additional, review-relevant findings (from repository docs/wiki)
-
Nuxt init/route gating
- Nuxt is initialized only when isDev (development); initNuxt() runs in dev and builds loadNuxt/dev handler; production path imports Nitro app. Nuxt route handler for /^/(new)(/.+)?$/ is only wired when dev (ctx.respond = false; ctx.req.ctx = ctx).,
-
Devtools / dev-only route placement
- The repo’s Nuxt integration is explicitly a dev-only path in src/index.js per the architecture overview; dev-only Nuxt routes (and any devtools endpoints) must therefore remain behind the isDev guard to avoid exposure in prod.,
-
Assets & public dir changes (impact on routing/serving)
- Build and runtime use versioned asset host (/assets/:v/*) with long caching in production; render middleware injects @shared.assetsHost and assetsVersion into templates. There is also a legacy/nuxt-specific CSS output (app/assets/css/legacy.css) produced by gulp nuxt:less that prefixes selectors with .ractive-component to avoid conflicts. Changes that add /nuxt_assets (new public dir) or switch component image URLs to /nuxt_assets must be reconciled with the server’s static-asset routing and assetsVersion handling so caching and serving remain correct.
-
Routing + middleware implications to verify in this PR
- Confirm new '/nuxt_devtools' appears only in the dev-only route group and is behind the same isDev initNuxt guard so nuxtRouteHandler is only callable in dev (to avoid Koa ctx.respond mistakes). See src/index.js routing pattern and Nuxt handler behavior.
-
Checklist for the PR review (concrete verifications to run)
- Confirm src/index.js still gates initNuxt / nuxtRouteHandler with isDev and that '/nuxt_devtools' is included only in NUXT_DEV_ONLY_ROUTES.
- Confirm any new NUXT_PROD_ONLY_ROUTES array is empty (or does not include devtools).
- Verify nuxt.config.ts devtools remains a dev-only setting (do not enable in production builds). (If nuxt.config.ts was changed in PR, ensure conditional or dev-only flag.)
- Ensure src/index.js nuxt route handler checks nuxtRouteHandler != null and sets ctx.respond = false before awaiting handler (prevents double responses).
- Confirm asset-serving logic (versioned /assets/:v/*) and the added /nuxt_assets public dir (nuxt.config.ts change) align — that /nuxt_assets is served with correct cache headers and that component image path changes point to the actual public location used in production.
- Validate CI/preview environments (Render/PR previews) do not set NODE_ENV=development; if previews need access to devtools, add an explicit guard (localhost or safe flag).
- Manual test steps to run locally/in CI:
- Start dev server, hit '/nuxt_devtools' and verify Nuxt devtools available and no Koa errors.
- Build for production, run production server, verify '/nuxt_devtools' 404s and /nuxt_assets and other assets serve with expected cache headers.
Sources consulted
- DeepWiki repository documentation pages (Architecture + Getting Started + Server/Nuxt integration):
- Direct question/search about Nuxt docs in repo (Nuxt init, dev vs prod handling):
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (3)
nuxt.config.ts (1)
36-38: Public directory configuration looks correct.This enables Nuxt to serve static assets from
app/public/. The trailing slash is functionally fine but consider removing it for consistency with Nuxt conventions ('app/public').app/components/CopyButton.vue (1)
10-10: The assetcopy.svgexists at the expected locationapp/public/nuxt_assets/icons/copy.svgand will be correctly served as/nuxt_assets/icons/copy.svgby Nuxt's public directory. No action needed.app/components/cli/InstallCommands.vue (1)
49-61: Asset paths are correctly referenced.All three icons (deb.svg, rpm.svg, homebrew.svg) exist under
app/public/nuxt_assets/icons/. The switch from imported assets to/nuxt_assets/string literals is properly implemented.
Closes #63