Skip to content

fix: respect compress settings in dev server #4159

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.

Sign up for GitHub

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

Open
hellozyemlya wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −3
Open

fix: respect compress settings in dev server #4159

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/dev/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { stat } from "node:fs/promises";
import { createReadStream } from "node:fs";
import { createGzip, createBrotliCompress } from "node:zlib";
import { createVFSHandler } from "./vfs.ts";
import type { CompressOptions } from "../types/config.ts";

import devErrorHandler, {
defaultHandler as devErrorHandlerInternal,
Expand Down Expand Up @@ -70,6 +71,7 @@ export class NitroDevApp {
dir: asset.dir,
base: assetBase,
fallthrough: asset.fallthrough,
compress: this.nitro.options.compressPublicAssets,
})
);
}
Expand Down Expand Up @@ -97,7 +99,7 @@ export class NitroDevApp {
// TODO: upstream to h3/node
function serveStaticDir(
event: H3Event,
opts: { dir: string; base: string; fallthrough?: boolean }
opts: { dir: string; base: string; fallthrough?: boolean; compress?: boolean | CompressOptions }
) {
const dir = resolve(opts.dir) + "/";
const r = (id: string) => {
Expand All @@ -107,6 +109,15 @@ function serveStaticDir(
return resolved;
}
};

// Determine compression settings
const compressOpts =
opts.compress === true
? { gzip: true, brotli: true, zstd: false }
: opts.compress === false
? { gzip: false, brotli: false, zstd: false }
: opts.compress || { gzip: false, brotli: false, zstd: false };

return serveStatic(event, {
fallthrough: opts.fallthrough,
getMeta: async (id) => {
Expand All @@ -126,12 +137,12 @@ function serveStaticDir(
if (!path) return;
const stream = createReadStream(path);
const acceptEncoding = event.req.headers.get("accept-encoding") || "";
if (acceptEncoding.includes("br")) {
if (compressOpts.brotli && acceptEncoding.includes("br")) {
event.res.headers.set("Content-Encoding", "br");
event.res.headers.delete("Content-Length");
event.res.headers.set("Vary", "Accept-Encoding");
return stream.pipe(createBrotliCompress());
} else if (acceptEncoding.includes("gzip")) {
} else if (compressOpts.gzip && acceptEncoding.includes("gzip")) {
Comment on lines +140 to +145
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Honor Accept-Encoding quality values (q) instead of substring matching.

Line 140 and Line 145 use .includes("br") / .includes("gzip"), which will incorrectly compress even when the client sends br;q=0 or gzip;q=0.

💡 Proposed fix 
-      const acceptEncoding = event.req.headers.get("accept-encoding") || "";
-      if (compressOpts.brotli && acceptEncoding.includes("br")) {
+      const acceptEncoding = event.req.headers.get("accept-encoding") || "";
+      if (compressOpts.brotli && acceptsEncoding(acceptEncoding, "br")) {
         event.res.headers.set("Content-Encoding", "br");
         event.res.headers.delete("Content-Length");
         event.res.headers.set("Vary", "Accept-Encoding");
         return stream.pipe(createBrotliCompress());
-      } else if (compressOpts.gzip && acceptEncoding.includes("gzip")) {
+      } else if (compressOpts.gzip && acceptsEncoding(acceptEncoding, "gzip")) {
         event.res.headers.set("Content-Encoding", "gzip");
         event.res.headers.delete("Content-Length");
         event.res.headers.set("Vary", "Accept-Encoding");
         return stream.pipe(createGzip());
       }
function acceptsEncoding(header: string, encoding: "br" | "gzip") {
  return header.split(",").some((item) => {
    const [name, ...params] = item.trim().toLowerCase().split(";");
    if (name !== encoding && name !== "*") {
      return false;
    }
    const q = params
      .map((p) => p.trim())
      .find((p) => p.startsWith("q="))
      ?.slice(2);

    if (!q) {
      return true;
    }

    const quality = Number.parseFloat(q);
    return Number.isFinite(quality) && quality > 0;
  });
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/dev/app.ts` around lines 140 - 145, Replace the naive substring checks
for brotli/gzip in the compression branch with an Accept-Encoding parser that
honors q-values: add an acceptsEncoding(header, encoding) helper (as proposed)
and use it instead of acceptEncoding.includes("br") and
acceptEncoding.includes("gzip") in the logic around
stream.pipe(createBrotliCompress()) and the gzip branch; the helper should split
on commas, parse each item into name and params, treat "*" as matching, parse
any "q=" param to a number, and only return true if a matching encoding has no q
or q > 0.0 so clients that send q=0 are correctly respected.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the git log for the serveStaticDir function to see what it looked like before this PR
git log --oneline -10 src/dev/app.ts
# Check the original state of the Accept-Encoding checks before this PR
git show HEAD~1:src/dev/app.ts 2>/dev/null | grep -n "acceptEncoding\|includes\|Accept-Encoding" || \
git log --all --oneline | head -5

Length of output: 724

@jantimon Yes, the q=0 / naive substring-matching issue is pre-existing — it was there before this PR. The git history confirms that commit bbf2808 (the parent of this PR) already had:

if (acceptEncoding.includes("br")) { ... }
else if (acceptEncoding.includes("gzip")) { ... }

This PR only added the compressOpts.brotli && and compressOpts.gzip && guards in front of those same checks. The .includes() approach itself was introduced earlier in 9ee9f21 (refactor(dev): improve serveStaticDir with compression and streaming).

Since it's pre-existing and out of scope for this PR, would you like me to open a separate follow-up issue to track it?

🧬 Code Graph Analysis Results

  • File: src/types/config.ts (lines 335-339)
    Content:
    export interface CompressOptions {
    gzip?: boolean;
    brotli?: boolean;
    zstd?: boolean;
    }

  • File: src/runtime/internal/error/dev.ts (lines 101-117)
    Content:
    export async function loadStackTrace(error: any): Promise {
    if (!(error instanceof Error)) {
    return;
    }

    const { ErrorParser } = await import("youch-core");

    const parsed = await new ErrorParser().defineSourceLoader(sourceLoader).parse(error);

    const stack = error.message + "\n" + parsed.frames.map((frame) => fmtFrame(frame)).join("\n");

    Object.defineProperty(error, "stack", { value: stack });

    if (error.cause) {
    await loadStackTrace(error.cause).catch(consola.error);
    }
    }

  • File: src/dev/server.ts (lines 208-247)
    Content:
    #generateError() {
    const error: any = this.#buildError || this.#workerError;
    if (error) {
    try {
    error.unhandled = false;
    let id = error.id || error.path;
    if (id) {
    const cause = (error as { errors?: any[] }).errors?.[0];
    const loc = error.location || error.loc || cause?.location || cause?.loc;
    if (loc) {
    id += :${loc.line}:${loc.column};
    }
    error.stack = (error.stack || "").replace(/(^\s*at\s+.+)/m, at ${id}\n$1);
    }
    } catch {
    // ignore
    }
    return new HTTPError(error);
    }

    return new Response(
  JSON.stringify(
    {
      error: "Dev server is unavailable.",
      hint: "Please reload the page and check the console for errors if the issue persists.",
    },
    null,
    2
  ),
  {
    status: 503,
    statusText: "Dev server is unavailable",
    headers: {
      "Content-Type": "application/json",
      "Cache-Control": "no-store",
      Refresh: "3",
    },
  }
);

    }

  • File: src/dev/vfs.ts (lines 4-157)
    Content:
    export function createVFSHandler(nitro: Nitro) {
    return defineHandler(async (event) => {
    const { socket } = event.runtime?.node?.req || {};

    // prettier-ignore
const isUnixSocket =
  // No network addresses
  (!socket?.remoteAddress && !socket?.localAddress) &&
  // Empty address object
  Object.keys(socket?.address?.() || {}).length === 0 &&
  // Socket is readable/writable but has no port info
  socket?.readable && socket?.writable && !socket?.remotePort;

const ip = getRequestIP(event, { xForwardedFor: isUnixSocket });

const isLocalRequest = ip && /^::1$|^127\.\d+\.\d+\.\d+$/.test(ip);
if (!isLocalRequest) {
  throw new HTTPError({
    statusText: `Forbidden IP: "${ip || "?"}"`,
    status: 403,
  });
}

const url = event.context.params?._ || "";
const isJson =
  url.endsWith(".json") || event.req.headers.get("accept")?.includes("application/json");
const id = decodeURIComponent(url.replace(/^(\.json)?\/?/, "") || "");

if (id && !nitro.vfs.has(id)) {
  throw new HTTPError({ message: "File not found", status: 404 });
}

const content = id ? await nitro.vfs.get(id)?.render() : undefined;

if (isJson) {
  return {
    rootDir: nitro.options.rootDir,
    entries: [...nitro.vfs.keys()].map((id) => ({
      id,
      path: "/_vfs.json/" + encodeURIComponent(id),
    })),
    current: id
      ? {
          id,
          content,
        }
      : null,
  };
}

const directories: Record<string, any> = { [nitro.options.rootDir]: {} };
const fpaths = [...nitro.vfs.keys()];

for (const item of fpaths) {
  const segments = item.replace(nitro.options.rootDir, "").split("/").filter(Boolean);
  let currentDir = item.startsWith(nitro.options.rootDir)
    ? directories[nitro.options.rootDir]
    : directories;

  for (const segment of segments) {
    if (!currentDir[segment]) {
      currentDir[segment] = {};
    }

    currentDir = currentDir[segment];
  }
}

const generateHTML = (directory: Record<string, any>, path: string[] = []): string =>
  Object.entries(directory)
    .map(([fname, value = {}]) => {
      const subpath = [...path, fname];
      const key = subpath.join("/");
      const encodedUrl = encodeURIComponent(key);

      const linkClass =
        url === `/${encodedUrl}` ? "bg-gray-700 text-white" : "hover:bg-gray-800 text-gray-200";

      return Object.keys(value).length === 0
        ? `
        <li class="flex flex-nowrap">
          <a href="/_vfs/${encodedUrl}" class="w-full text-sm px-2 py-1 border-b border-gray-10 ${linkClass}">
            ${fname}
          </a>
        </li>
        `
        : `
        <li>
          <details ${url.startsWith(`/${encodedUrl}`) ? "open" : ""}>
            <summary class="w-full text-sm px-2 py-1 border-b border-gray-10 hover:bg-gray-800 text-gray-200">
              ${fname}
            </summary>
            <ul class="ml-4">
              ${generateHTML(value, subpath)}
            </ul>
          </details>
        </li>
        `;
    })
    .join("");

const rootDirectory = directories[nitro.options.rootDir];
delete directories[nitro.options.rootDir];
const items = generateHTML(rootDirectory, [nitro.options.rootDir]) + generateHTML(directories);

const files = `
  <div class="h-full overflow-auto border-r border-gray:10">
    <p class="text-white text-bold text-center py-1 opacity-50">Virtual Files</p>
    <ul class="flex flex-col">${items}</ul>
  </div>
  `;

const file = id
  ? editorTemplate({
      readOnly: true,
      language: id.endsWith("html") ? "html" : "javascript",
      theme: "vs-dark",
      value: content,
      wordWrap: "wordWrapColumn",
      wordWrapColumn: 80,
    })
  : `
    <div class="w-full h-full flex opacity-50">
      <h1 class="text-white m-auto">Select a virtual file to inspect</h1>
    </div>
  `;

event.res.headers.set("Content-Type", "text/html; charset=utf-8");
return /* html */ `

    <!doctype html>

    <script src="https://cdn.jsdelivr.net/npm/@unocss/runtime"></script> <style> html { background: `#1E1E1E`; color: white; } [un-cloak] { display: none; } </style>
    ${files} ${file}
    `; }); }

  • File: src/runtime/internal/route-rules.ts (lines 40-59)
    Content:
    export const proxy: RouteRuleCtor<"proxy"> = ((m) =>
    function proxyRouteRule(event) {
    let target = m.options?.to;
    if (!target) {
    return;
    }
    if (target.endsWith("/**")) {
    let targetPath = event.url.pathname + event.url.search;
    const strpBase = (m.options as any)._proxyStripBase;
    if (strpBase) {
    targetPath = withoutBase(targetPath, strpBase);
    }
    target = joinURL(target.slice(0, -3), targetPath);
    } else if (event.url.search) {
    target = withQuery(target, Object.fromEntries(event.url.searchParams));
    }
    return proxyRequest(event, target, {
    ...m.options,
    });
    }) satisfies RouteRuleCtor<"proxy">;

🧠 Learnings used 
Learnt from: pi0
Repo: nitrojs/nitro PR: 3887
File: src/build/vite/bundler.ts:77-99
Timestamp: 2026-01-21T18:01:10.943Z
Learning: In defu v6, array properties are concatenated rather than overwritten when merging defaults with overrides. Example: defu({ array:['a','b'] }, { array:['c'] }) -> { array:['a','b','c'] }. When reviewing code that uses defu to merge config objects (e.g., src/build/vite/bundler.ts), assume arrays accumulate values from defaults and overrides. This affects plugins/loader configurations and any array-based config. If you need array replacement, consider a different merge strategy or post-process arrays after defu.

Learnt from: Teages
Repo: nitrojs/nitro PR: 4166
File: docs/4.examples/graphql-pothos.md:176-179
Timestamp: 2026-04-05T08:40:20.433Z
Learning: For projects using Pothos GraphQL v4 (`pothos/core` ^4.x), remember that output fields are nullable by default (unlike Pothos v3). The `nullable` option defaults to `true` unless you set `defaultFieldNullability: false` on the `SchemaBuilder` in your schema. During code review, ensure the intended nullability is explicit/consistent: set `nullable: true` explicitly when clarity is important, and use `nullable: false` (or `defaultFieldNullability: false`) when fields must be non-null.

event.res.headers.set("Content-Encoding", "gzip");
event.res.headers.delete("Content-Length");
event.res.headers.set("Vary", "Accept-Encoding");
Expand Down