Remix integration test (#380)

* fix: ensure pages routes are defined correctly In e151223 we introduced a bug where the RouteKey was now an array rather than a simple URL string. When it got stringified into the routing object these were invalid. E.g. `[':page*', undefined]` got stringified to `":page*,"` rather than `":page*"`. * Configure Remix example project * Add boot-up test * Prettify * Fix linting * Refactor test * Add env and cwd * Build Remix before wrangler dev server * refactor: clean up pages routing Fixes #379
cloudflare · Feb 4, 2022 · aacd1c2 · aacd1c2
1 parent 8452485
commit aacd1c2
Show file tree

Hide file tree

Showing 22 changed files with 7,204 additions and 6,439 deletions.
diff --git a/.changeset/chilled-ties-lay.md b/.changeset/chilled-ties-lay.md
@@ -0,0 +1,10 @@
+---
+"wrangler": patch
+---
+
+fix: ensure pages routes are defined correctly
+
+In e151223 we introduced a bug where the RouteKey was now an array rather than a simple URL string. When it got stringified into the routing object these were invalid.
+E.g. `[':page*', undefined]` got stringified to `":page*,"` rather than `":page*"`.
+
+Fixes #379
diff --git a/.changeset/itchy-fans-smoke.md b/.changeset/itchy-fans-smoke.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+refactor: clean up pages routing
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -14,7 +14,7 @@
   "dependencies": {
     "@changesets/changelog-github": "^0.4.2",
     "@changesets/cli": "^2.18.0",
-    "@types/jest": "^27.0.3",
+    "@types/jest": "^27.4.0",
     "@types/node": "^16.11.11",
     "@typescript-eslint/eslint-plugin": "^5.6.0",
     "@typescript-eslint/parser": "^5.6.0",
@@ -35,7 +35,8 @@
     "check:type": "npm run check:test --workspaces --if-present",
     "check:lint": "eslint \"packages/**/*.[tj]s?(x)\" --cache --cache-strategy content",
     "check:format": "prettier packages/** --check --ignore-unknown",
-    "build-and-test": "run-p build test --aggregate-output -l",
+    "prebuild-and-test": "npm run build",
+    "build-and-test": "npm test",
     "build": "npm run build --workspace=wrangler",
     "test": "npm run test --workspaces --if-present",
     "prettify": "prettier packages/** --write --ignore-unknown"

diff --git a/packages/example-remix-pages-app/.gitignore b/packages/example-remix-pages-app/.gitignore
@@ -0,0 +1,5 @@
+node_modules
+
+/.cache
+/build
+/public/build
diff --git a/packages/example-remix-pages-app/.node-version b/packages/example-remix-pages-app/.node-version
@@ -0,0 +1 @@
+16.7.0
diff --git a/packages/example-remix-pages-app/README.md b/packages/example-remix-pages-app/README.md
@@ -0,0 +1,3 @@
+# Welcome to Remix!
+
+This is a lightly modified Remix starter (`npx create-remix@latest`) which will let us test changes to wrangler and Pages Functions.
diff --git a/packages/example-remix-pages-app/app/entry.client.tsx b/packages/example-remix-pages-app/app/entry.client.tsx
@@ -0,0 +1,5 @@
+import React from "react";
+import { hydrate } from "react-dom";
+import { RemixBrowser } from "remix";
+
+hydrate(<RemixBrowser />, document);
diff --git a/packages/example-remix-pages-app/app/entry.server.tsx b/packages/example-remix-pages-app/app/entry.server.tsx
@@ -0,0 +1,22 @@
+import React from "react";
+import { renderToString } from "react-dom/server";
+import { RemixServer } from "remix";
+import type { EntryContext } from "remix";
+
+export default function handleRequest(
+  request: Request,
+  responseStatusCode: number,
+  responseHeaders: Headers,
+  remixContext: EntryContext
+) {
+  const markup = renderToString(
+    <RemixServer context={remixContext} url={request.url} />
+  );
+
+  responseHeaders.set("Content-Type", "text/html");
+
+  return new Response("<!DOCTYPE html>" + markup, {
+    status: responseStatusCode,
+    headers: responseHeaders,
+  });
+}
diff --git a/packages/example-remix-pages-app/app/root.tsx b/packages/example-remix-pages-app/app/root.tsx
@@ -0,0 +1,33 @@
+import React from "react";
+import {
+  Links,
+  LiveReload,
+  Meta,
+  Outlet,
+  Scripts,
+  ScrollRestoration,
+} from "remix";
+import type { MetaFunction } from "remix";
+
+export const meta: MetaFunction = () => {
+  return { title: "New Remix App" };
+};
+
+export default function App() {
+  return (
+    <html lang="en">
+      <head>
+        <meta charSet="utf-8" />
+        <meta name="viewport" content="width=device-width,initial-scale=1" />
+        <Meta />
+        <Links />
+      </head>
+      <body>
+        <Outlet />
+        <ScrollRestoration />
+        <Scripts />
+        {process.env.NODE_ENV === "development" && <LiveReload />}
+      </body>
+    </html>
+  );
+}
diff --git a/packages/example-remix-pages-app/app/routes/index.tsx b/packages/example-remix-pages-app/app/routes/index.tsx
@@ -0,0 +1,34 @@
+import React from "react";
+
+export default function Index() {
+  return (
+    <div style={{ fontFamily: "system-ui, sans-serif", lineHeight: "1.4" }}>
+      <h1>Welcome to Remix</h1>
+      <ul>
+        <li>
+          <a
+            target="_blank"
+            href="https://remix.run/tutorials/blog"
+            rel="noreferrer"
+          >
+            15m Quickstart Blog Tutorial
+          </a>
+        </li>
+        <li>
+          <a
+            target="_blank"
+            href="https://remix.run/tutorials/jokes"
+            rel="noreferrer"
+          >
+            Deep Dive Jokes App Tutorial
+          </a>
+        </li>
+        <li>
+          <a target="_blank" href="https://remix.run/docs" rel="noreferrer">
+            Remix Docs
+          </a>
+        </li>
+      </ul>
+    </div>
+  );
+}
diff --git a/packages/example-remix-pages-app/functions/[[path]].js b/packages/example-remix-pages-app/functions/[[path]].js
@@ -0,0 +1,12 @@
+import { createPagesFunctionHandler } from "@remix-run/cloudflare-pages";
+
+// @ts-ignore
+import * as build from "../build";
+
+const handleRequest = createPagesFunctionHandler({
+  build,
+});
+
+export function onRequest(context) {
+  return handleRequest(context);
+}
diff --git a/packages/example-remix-pages-app/package.json b/packages/example-remix-pages-app/package.json
@@ -0,0 +1,54 @@
+{
+  "private": true,
+  "name": "remix-app-template",
+  "description": "",
+  "license": "",
+  "scripts": {
+    "build": "cross-env NODE_ENV=production remix build",
+    "dev": "cross-env NODE_ENV=development run-p dev:*",
+    "postinstall": "remix setup cloudflare-pages",
+    "dev:remix": "remix watch",
+    "dev:wrangler": "wrangler pages dev ./public",
+    "start": "npm run dev:wrangler",
+    "test": "npx jest --forceExit"
+  },
+  "dependencies": {
+    "@remix-run/cloudflare-pages": "^1.1.3",
+    "@remix-run/react": "^1.1.3",
+    "react": "^17.0.2",
+    "react-dom": "^17.0.2",
+    "remix": "^1.1.3"
+  },
+  "devDependencies": {
+    "@cloudflare/workers-types": "^3.2.0",
+    "@remix-run/dev": "^1.1.3",
+    "@types/react": "^17.0.24",
+    "@types/react-dom": "^17.0.9",
+    "cross-env": "^7.0.3",
+    "esbuild": "0.13.14",
+    "npm-run-all": "^4.1.5",
+    "typescript": "^4.1.2",
+    "undici": "^4.13.0"
+  },
+  "engines": {
+    "node": ">=14"
+  },
+  "sideEffects": false,
+  "main": "dist/worker.js",
+  "jest": {
+    "restoreMocks": true,
+    "testTimeout": 30000,
+    "testRegex": ".*.(test|spec)\\.[jt]sx?$",
+    "transformIgnorePatterns": [
+      "node_modules/(?!find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream)"
+    ],
+    "transform": {
+      "^.+\\.c?(t|j)sx?$": [
+        "esbuild-jest",
+        {
+          "sourcemap": true
+        }
+      ]
+    }
+  }
+}
diff --git a/packages/example-remix-pages-app/public/favicon.ico b/packages/example-remix-pages-app/public/favicon.ico
diff --git a/packages/example-remix-pages-app/remix.config.js b/packages/example-remix-pages-app/remix.config.js
@@ -0,0 +1,13 @@
+/**
+ * @type {import('@remix-run/dev/config').AppConfig}
+ */
+module.exports = {
+  appDirectory: "app",
+  assetsBuildDirectory: "public/build",
+  publicPath: "/build/",
+  serverModuleFormat: "esm",
+  serverPlatform: "neutral",
+  serverBuildDirectory: "build",
+  devServerBroadcastDelay: 1000,
+  ignoredRouteFiles: [".*"],
+};
diff --git a/packages/example-remix-pages-app/remix.env.d.ts b/packages/example-remix-pages-app/remix.env.d.ts
@@ -0,0 +1,3 @@
+/// <reference types="@remix-run/dev" />
+/// <reference types="@remix-run/cloudflare-pages/globals" />
+/// <reference types="@cloudflare/workers-types" />
diff --git a/packages/example-remix-pages-app/tests/index.test.ts b/packages/example-remix-pages-app/tests/index.test.ts
@@ -0,0 +1,46 @@
+import { spawn, spawnSync } from "child_process";
+import { resolve } from "path";
+import { fetch } from "undici";
+import type { Response } from "undici";
+
+const waitUntilReady = async (url: string): Promise<Response> => {
+  let response: Response | undefined = undefined;
+
+  while (response === undefined) {
+    await new Promise((resolvePromise) => setTimeout(resolvePromise, 500));
+
+    try {
+      response = await fetch(url);
+    } catch {}
+  }
+
+  return response as Response;
+};
+
+const isWindows = process.platform === "win32";
+
+describe("Remix", () => {
+  beforeAll(async () => {
+    spawnSync("npm", ["run", "build"], {
+      shell: isWindows,
+      cwd: resolve(__dirname, "../"),
+    });
+    const wranglerProcess = spawn("npm", ["run", "dev:wrangler"], {
+      shell: isWindows,
+      cwd: resolve(__dirname, "../"),
+      env: { BROWSER: "none", ...process.env },
+    });
+    wranglerProcess.stdout.on("data", (chunk) => {
+      console.log(chunk.toString());
+    });
+    wranglerProcess.stderr.on("data", (chunk) => {
+      console.log(chunk.toString());
+    });
+  });
+
+  it("renders", async () => {
+    const response = await waitUntilReady("http://localhost:8788/");
+    const text = await response.text();
+    expect(text).toContain("Welcome to Remix");
+  });
+});
diff --git a/packages/example-remix-pages-app/tsconfig.json b/packages/example-remix-pages-app/tsconfig.json
@@ -0,0 +1,20 @@
+{
+  "include": ["remix.env.d.ts", "**/*.ts", "**/*.tsx"],
+  "compilerOptions": {
+    "lib": ["DOM", "DOM.Iterable", "ES2019"],
+    "isolatedModules": true,
+    "esModuleInterop": true,
+    "jsx": "react-jsx",
+    "moduleResolution": "node",
+    "resolveJsonModule": true,
+    "target": "ES2019",
+    "strict": true,
+    "baseUrl": ".",
+    "paths": {
+      "~/*": ["./app/*"]
+    },
+
+    // Remix takes care of building everything in `remix build`.
+    "noEmit": true
+  }
+}
diff --git a/packages/wrangler/pages/functions/filepath-routing.test.ts b/packages/wrangler/pages/functions/filepath-routing.test.ts
@@ -1,47 +1,66 @@
 import { toUrlPath } from "../../src/paths";
 import { compareRoutes } from "./filepath-routing";
+import type { HTTPMethod, RouteConfig } from "./routes";
 
 describe("compareRoutes()", () => {
-  const url = toUrlPath;
   test("routes / last", () => {
-    expect(compareRoutes([url("/")], [url("/foo")])).toBeGreaterThanOrEqual(1);
-    expect(compareRoutes([url("/")], [url("/:foo")])).toBeGreaterThanOrEqual(1);
-    expect(compareRoutes([url("/")], [url("/:foo*")])).toBeGreaterThanOrEqual(
-      1
-    );
+    expect(
+      compareRoutes(routeConfig("/"), routeConfig("/foo"))
+    ).toBeGreaterThanOrEqual(1);
+    expect(
+      compareRoutes(routeConfig("/"), routeConfig("/:foo"))
+    ).toBeGreaterThanOrEqual(1);
+    expect(
+      compareRoutes(routeConfig("/"), routeConfig("/:foo*"))
+    ).toBeGreaterThanOrEqual(1);
   });
 
   test("routes with fewer segments come after those with more segments", () => {
     expect(
-      compareRoutes([url("/foo")], [url("/foo/bar")])
+      compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar"))
     ).toBeGreaterThanOrEqual(1);
     expect(
-      compareRoutes([url("/foo")], [url("/foo/bar/cat")])
+      compareRoutes(routeConfig("/foo"), routeConfig("/foo/bar/cat"))
     ).toBeGreaterThanOrEqual(1);
   });
 
   test("routes with wildcard segments come after those without", () => {
-    expect(compareRoutes([url("/:foo*")], [url("/foo")])).toBe(1);
-    expect(compareRoutes([url("/:foo*")], [url("/:foo")])).toBe(1);
+    expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/foo"))).toBe(1);
+    expect(compareRoutes(routeConfig("/:foo*"), routeConfig("/:foo"))).toBe(1);
   });
 
   test("routes with dynamic segments come after those without", () => {
-    expect(compareRoutes([url("/:foo")], [url("/foo")])).toBe(1);
+    expect(compareRoutes(routeConfig("/:foo"), routeConfig("/foo"))).toBe(1);
   });
 
   test("routes with dynamic segments occurring earlier come after those with dynamic segments in later positions", () => {
-    expect(compareRoutes([url("/foo/:id/bar")], [url("/foo/bar/:id")])).toBe(1);
+    expect(
+      compareRoutes(routeConfig("/foo/:id/bar"), routeConfig("/foo/bar/:id"))
+    ).toBe(1);
   });
 
   test("routes with no HTTP method come after those specifying a method", () => {
-    expect(compareRoutes([url("/foo")], [url("/foo"), "GET"])).toBe(1);
+    expect(compareRoutes(routeConfig("/foo"), routeConfig("/foo", "GET"))).toBe(
+      1
+    );
   });
 
   test("two equal routes are sorted according to their original position in the list", () => {
-    expect(compareRoutes([url("/foo"), "GET"], [url("/foo"), "GET"])).toBe(0);
+    expect(
+      compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo", "GET"))
+    ).toBe(0);
   });
 
   test("it returns -1 if the first argument should appear first in the list", () => {
-    expect(compareRoutes([url("/foo"), "GET"], [url("/foo")])).toBe(-1);
+    expect(compareRoutes(routeConfig("/foo", "GET"), routeConfig("/foo"))).toBe(
+      -1
+    );
   });
 });
+
+function routeConfig(routePath: string, method?: string): RouteConfig {
+  return {
+    routePath: toUrlPath(routePath),
+    method: method as HTTPMethod,
+  };
+}