fix(core): address #922 review — posix paths, diamond imports, comment safety

miguel-heygen · miguel-heygen · commit 72506f3b4a5c · 2026-05-17T17:35:31.000-04:00
Fixes from review #4306329284 on hf#922: - Normalize path.relative() output with .split(sep).join("/") so rebased url() paths use forward slashes on Windows, matching the posix-path convention in rewriteSubCompPaths.ts. - Return empty string (not the original @import statement) when the visited set detects a diamond import. Previously the stale @import leaked through and caused a 404 after bundling. - Strip CSS block comments before @import matching so commented-out imports (/* @import url(...) */) are not resolved. Comments are restored after processing via placeholder substitution.
diff --git a/packages/core/src/compiler/htmlBundler.test.ts b/packages/core/src/compiler/htmlBundler.test.ts
@@ -898,4 +898,45 @@ describe("bundleToSingleHtml", () => {
 
     expect(bundled).toContain("url('styles/sprite.png?v=2#section')");
   });
+
+  it("deduplicates diamond @import (same file imported by two parents)", async () => {
+    const dir = makeTempProject({
+      "index.html": `<!doctype html>
+<html><body>
+  <link rel="stylesheet" href="styles/main.css">
+  <div data-composition-id="root" data-width="320" data-height="180"></div>
+  <script>window.__timelines = window.__timelines || {}; window.__timelines.root = {}</script>
+</body></html>`,
+      "styles/main.css": `@import url('./a.css');\n@import url('./b.css');`,
+      "styles/a.css": `@import url('./shared.css');\n.a { color: red; }`,
+      "styles/b.css": `@import url('./shared.css');\n.b { color: blue; }`,
+      "styles/shared.css": `:root { --shared: 1; }`,
+    });
+
+    const bundled = await bundleToSingleHtml(dir);
+
+    const sharedCount = (bundled.match(/--shared: 1/g) || []).length;
+    expect(sharedCount).toBe(1);
+    expect(bundled).toContain(".a { color: red; }");
+    expect(bundled).toContain(".b { color: blue; }");
+    expect(bundled).not.toContain("@import");
+  });
+
+  it("does not resolve @import inside CSS comments", async () => {
+    const dir = makeTempProject({
+      "index.html": `<!doctype html>
+<html><body>
+  <link rel="stylesheet" href="app.css">
+  <div data-composition-id="root" data-width="320" data-height="180"></div>
+  <script>window.__timelines = window.__timelines || {}; window.__timelines.root = {}</script>
+</body></html>`,
+      "app.css": `/* @import url('./old.css'); */\nbody { margin: 0; }`,
+      "old.css": `.old { display: none; }`,
+    });
+
+    const bundled = await bundleToSingleHtml(dir);
+
+    expect(bundled).toContain("/* @import url('./old.css'); */");
+    expect(bundled).not.toContain(".old { display: none; }");
+  });
 });
diff --git a/packages/core/src/compiler/htmlBundler.ts b/packages/core/src/compiler/htmlBundler.ts
@@ -92,6 +92,29 @@ const CSS_IMPORT_RE =
 
 const REBASE_URL_RE = /\burl\(\s*(["']?)([^)"']+)\1\s*\)/g;
 
+const CSS_COMMENT_RE = /\/\*[\s\S]*?\*\//g;
+
+function withCommentsStripped<T>(
+  css: string,
+  fn: (stripped: string) => T,
+): { result: T; restore: (s: string) => string } {
+  const comments: string[] = [];
+  const stripped = css.replace(CSS_COMMENT_RE, (m) => {
+    const idx = comments.length;
+    comments.push(m);
+    return `/*__hf_c${idx}__*/`;
+  });
+  const result = fn(stripped);
+  const restore = (s: string) => {
+    let out = s;
+    for (let i = 0; i < comments.length; i++) {
+      out = out.replace(`/*__hf_c${i}__*/`, comments[i]!);
+    }
+    return out;
+  };
+  return { result, restore };
+}
+
 function rebaseCssUrls(css: string, cssFileDir: string, projectDir: string): string {
   const resolvedRoot = resolve(projectDir);
   const resolvedDir = resolve(cssFileDir);
@@ -101,7 +124,7 @@ function rebaseCssUrls(css: string, cssFileDir: string, projectDir: string): str
     const { basePath, suffix } = splitUrlSuffix(urlValue.trim());
     if (!basePath) return full;
     const absolutePath = resolve(resolvedDir, basePath);
-    const rebased = relative(resolvedRoot, absolutePath);
+    const rebased = relative(resolvedRoot, absolutePath).split(sep).join("/");
     if (rebased === basePath) return full;
     return `url(${quote || ""}${rebased}${suffix}${quote || ""})`;
   });
@@ -113,29 +136,32 @@ function inlineCssFile(
   projectDir: string,
   visited: Set<string> = new Set(),
 ): string {
-  const placeholders: string[] = [];
-  const withPlaceholders = css.replace(
+  const { result: strippedCss, restore: restoreComments } = withCommentsStripped(css, (s) => s);
+  const importPlaceholders: string[] = [];
+  const withPlaceholders = strippedCss.replace(
     CSS_IMPORT_RE,
     (full, _q1, urlPath, _q2, barePath, mediaQuery) => {
       const importPath = urlPath ?? barePath;
       if (!importPath || !isRelativeUrl(importPath)) return full;
       const resolved = resolve(cssFileDir, importPath);
       const normalizedBase = resolve(projectDir) + sep;
-      if (!resolved.startsWith(normalizedBase) || visited.has(resolved)) return full;
+      if (!resolved.startsWith(normalizedBase)) return full;
+      if (visited.has(resolved)) return "";
       const content = safeReadFile(resolved);
       if (content == null) return full;
       visited.add(resolved);
       const inlined = inlineCssFile(content, dirname(resolved), projectDir, visited);
       const trimmedMedia = (mediaQuery || "").trim();
       const block = trimmedMedia ? `@media ${trimmedMedia} {\n${inlined}\n}\n` : inlined + "\n";
-      const idx = placeholders.length;
-      placeholders.push(block);
+      const idx = importPlaceholders.length;
+      importPlaceholders.push(block);
       return `/*__hf_import_${idx}__*/`;
     },
   );
   let rebased = rebaseCssUrls(withPlaceholders, cssFileDir, projectDir);
-  for (let i = 0; i < placeholders.length; i++) {
-    rebased = rebased.replace(`/*__hf_import_${i}__*/`, placeholders[i]!);
+  rebased = restoreComments(rebased);
+  for (let i = 0; i < importPlaceholders.length; i++) {
+    rebased = rebased.replace(`/*__hf_import_${i}__*/`, importPlaceholders[i]!);
   }
   return rebased;
 }
