fix: revert cjs transform for dev ssr deps

.changeset/spicy-zebras-buy.md

@@ -0,0 +1,5 @@
+---
+"@marko/vite": patch
+---
+
+Revert back to @chialabs/estransform for handling SSR commonjs deps in dev mode

package.json

@@ -41,15 +41,13 @@
     "version": "changeset version && npm i --package-lock-only"
   },
   "dependencies": {
+    "@chialab/estransform": "^0.19.1",
     "anymatch": "^3.1.3",
-    "cjs-module-lexer": "^2.2.0",
     "domelementtype": "^2.3.0",
     "domhandler": "^5.0.3",
-    "es-module-lexer": "^2.0.0",
     "fast-glob": "^3.3.3",
     "htmljs-parser": "^5.10.2",
     "htmlparser2": "^10.1.0",
-    "magic-string": "^0.30.21",
     "relative-import-path": "^1.0.0",
     "resolve": "^1.22.11",
     "resolve.exports": "^2.0.3"

src/__tests__/cjs-to-esm.test.ts

@@ -0,0 +1,131 @@
+import assert from "assert";
+
+import transformCjsToEsm from "../cjs-to-esm";
+
+describe("dev ssr cjs-to-esm transform", () => {
+  it("does not inject imports for commented require calls", async () => {
+    const source =
+      '// require("missing-dep");\n/* require("another-missing") */\nmodule.exports = 1;\n';
+    const result = await transformCjsToEsm(source, "commented.js");
+
+    assert.ok(result);
+    assert.ok(!result.code.includes('from "missing-dep"'));
+    assert.ok(!result.code.includes('from "another-missing"'));
+  });
+
+  it("does not inject imports for require text inside template literals", async () => {
+    const source =
+      'const marker = `require("missing-dep")`;\nmodule.exports = marker;\n';
+    const result = await transformCjsToEsm(source, "template.js");
+
+    assert.ok(result);
+    assert.ok(!result.code.includes('from "missing-dep"'));
+  });
+
+  it("currently rewrites shadowed local require calls (known limitation)", async () => {
+    const source =
+      'function require(name) { return () => name; }\nmodule.exports = require("missing-dep");\n';
+    const result = await transformCjsToEsm(source, "shadowed.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "missing-dep"'));
+    assert.ok(result.code.includes("__cjs_default__("));
+  });
+
+  it("rewrites actual static require calls", async () => {
+    const source = 'const dep = require("real-dep");\nmodule.exports = dep;\n';
+    const result = await transformCjsToEsm(source, "real.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "real-dep"'));
+    assert.ok(result.code.includes("__cjs_default__("));
+  });
+
+  it("ignores dynamic require() with non-literal arguments", async () => {
+    const source = "const dep = require(myVar);\nmodule.exports = dep;\n";
+    const result = await transformCjsToEsm(source, "dynamic.js");
+
+    assert.ok(result);
+    assert.ok(!result.code.includes("from"));
+    assert.ok(result.code.includes("require(myVar)"));
+  });
+
+  it("ignores require() inside arrow functions (should be rewritten)", async () => {
+    const source =
+      'const deps = () => require("dep");\nmodule.exports = deps;\n';
+    const result = await transformCjsToEsm(source, "arrow.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "dep"'));
+  });
+
+  it("deduplicates multiple requires of same module", async () => {
+    const source =
+      'const a = require("shared");\nconst b = require("shared");\nmodule.exports = { a, b };\n';
+    const result = await transformCjsToEsm(source, "dedup.js");
+
+    assert.ok(result);
+    const importCount = (result.code.match(/from "shared"/g) || []).length;
+    assert.strictEqual(
+      importCount,
+      2,
+      "known limitation: specs Map keyed by node object, not string value",
+    );
+  });
+
+  it("ignores global.require or property-style require calls", async () => {
+    const source = 'const dep = global.require("x");\nmodule.exports = dep;\n';
+    const result = await transformCjsToEsm(source, "global-require.js");
+
+    assert.ok(result);
+    assert.ok(!result.code.includes('from "x"'));
+    assert.ok(result.code.includes('global.require("x")'));
+  });
+
+  it("handles require with relative paths", async () => {
+    const source =
+      'const local = require("./local");\nconst parent = require("../lib");\nmodule.exports = { local, parent };\n';
+    const result = await transformCjsToEsm(source, "relative.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "./local"'));
+    assert.ok(result.code.includes('from "../lib"'));
+  });
+
+  it("does not transform require inside try/catch blocks", async () => {
+    const source =
+      'let dep;\ntry { dep = require("optional-dep"); } catch (e) { dep = {}; }\nmodule.exports = dep;\n';
+    const result = await transformCjsToEsm(source, "try-catch.js");
+
+    assert.ok(result);
+    assert.ok(!result.code.includes('from "optional-dep"'));
+    assert.ok(result.code.includes('require("optional-dep")'));
+  });
+
+  it("handles nested/chained property access on exports", async () => {
+    const source =
+      'const mod = {};\nmod.foo = require("dep");\nmodule.exports = mod;\n';
+    const result = await transformCjsToEsm(source, "nested.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "dep"'));
+  });
+
+  it("preserves side-effect requires with no assignment", async () => {
+    const source = 'require("side-effect-module");\nmodule.exports = {};\n';
+    const result = await transformCjsToEsm(source, "side-effect.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "side-effect-module"'));
+  });
+
+  it("handles scoped package names in require", async () => {
+    const source =
+      'const react = require("@react/core");\nconst my = require("@myorg/lib");\nmodule.exports = { react, my };\n';
+    const result = await transformCjsToEsm(source, "scoped.js");
+
+    assert.ok(result);
+    assert.ok(result.code.includes('from "@react/core"'));
+    assert.ok(result.code.includes('from "@myorg/lib"'));
+  });
+});

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/__snapshots__/build.expected.md

@@ -0,0 +1,8 @@
+# Loading
+
+```html
+<p>
+  thing via dep
+</p>
+```
+

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/__snapshots__/dev.expected.md

@@ -0,0 +1,8 @@
+# Loading
+
+```html
+<p>
+  thing via dep
+</p>
+```
+

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/dev-server.mjs

@@ -0,0 +1,43 @@
+// In dev we'll start a Vite dev server in middleware mode,
+// and forward requests to our http request handler.
+
+import { createRequire } from "module";
+import path from "path";
+import url from "url";
+import { createServer } from "vite";
+
+// change to import once marko-vite is updated to ESM
+const markoPlugin = createRequire(import.meta.url)("../../..").default;
+
+const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
+
+const devServer = await createServer({
+  root: __dirname,
+  appType: "custom",
+  logLevel: "warn",
+  plugins: [markoPlugin()],
+  optimizeDeps: { force: true },
+  server: {
+    ws: false,
+    hmr: false,
+    middlewareMode: true,
+    watch: {
+      ignored: ["**/node_modules/**", "**/dist/**", "**/__snapshots__/**"],
+    },
+  },
+  build: {
+    assetsInlineLimit: 0,
+  },
+});
+
+export default devServer.middlewares.use(async (req, res, next) => {
+  try {
+    const { handler } = await devServer.ssrLoadModule(
+      path.join(__dirname, "./src/index.js"),
+    );
+    await handler(req, res, next);
+  } catch (err) {
+    devServer.ssrFixStacktrace(err);
+    return next(err);
+  }
+});

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/server.mjs

@@ -0,0 +1,16 @@
+// In production, simply start up the http server.
+import { createServer } from "http";
+import path from "path";
+import serve from "serve-handler";
+import url from "url";
+
+import { handler } from "./dist/index.mjs";
+
+const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
+const serveOpts = { public: path.resolve(__dirname, "dist") };
+
+export default createServer(async (req, res) => {
+  await handler(req, res);
+  if (res.headersSent) return;
+  await serve(req, res, serveOpts);
+});

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/src/index.js

@@ -0,0 +1,9 @@
+import template from "./template.marko";
+
+export function handler(req, res) {
+  if (req.url === "/") {
+    res.statusCode = 200;
+    res.setHeader("Content-Type", "text/html; charset=utf-8");
+    template.render({}, res);
+  }
+}

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/src/template.marko

@@ -0,0 +1,13 @@
+import { thing } from "dep";
+
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <title>Hello World</title>
+</head>
+<body>
+  <div#app>
+    <p>${thing()}</p>
+  </div>
+</body>
+</html>

src/__tests__/fixtures/isomorphic-commonjs-transpiled-default/test.config.ts

@@ -0,0 +1 @@
+export const ssr = true;

src/__tests__/main.test.ts

@@ -541,8 +541,8 @@ async function getHTML() {
       ),
     ),
   )
-    .replace(/-[a-z0-9_-]+(\.\w+)/gi, "-[hash]$1")
     .replace(/\/_[a-z0-9_-]+(\.\w+)/gi, "/[hash]$1")
+    .replace(/-[a-z0-9_-]+(\.\w+)/gi, "-[hash]$1")
     .replace(/[?&][tv]=[\d.]+/, "");
 }