fix: site.entry-point should not be a hard deprecation

To make migration of v1 projects easier, Sites projects should still work, including the `entry-point` field (which currently errors out). This enables `site.entry-point` as a valid entry point, with a deprecation warning.
cloudflare · May 3, 2022 · 398d964 · 398d964
1 parent 97f945f
commit 398d964
Show file tree

Hide file tree

Showing 15 changed files with 448 additions and 54 deletions.
diff --git a/.changeset/afraid-oranges-happen.md b/.changeset/afraid-oranges-happen.md
@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: `site.entry-point` should not be a hard deprecation
+
+To make migration of v1 projects easier, Sites projects should still work, including the `entry-point` field (which currently errors out). This enables `site.entry-point` as a valid entry point, with a deprecation warning.
diff --git a/examples/legacy-site-app/public/404.html b/examples/legacy-site-app/public/404.html
@@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+    <head>
+        <link rel="icon" type="image/x-icon" href="favicon.ico">
+        <link href="https://fonts.googleapis.com/css?family=Pacifico&display=swap" rel="stylesheet">
+        <link href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" rel="stylesheet">
+        <style>
+            h1 {
+                font-family: Pacifico, sans-serif;
+                font-size: 4em;
+                color: #3eb5f1;
+                margin: 0;
+            }
+
+            h2 {
+                font-weight: 300;
+                font-family: sans-serif;
+            }
+
+            .centered {
+                position: fixed;
+                top: 50%;
+                left: 50%;
+                transform: translate(-50%, -50%);
+                text-align: center;
+            }
+
+            #ferris {
+                width: 75%;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="centered">
+            <h1>404 Not Found</h1>
+            <h2>Oh dang! We couldn't find that page.</h2>
+            <img id="ferris" alt="a sad crab is unable to unable to lasso a paper airplane. 404 not found." src="./img/404-wrangler-ferris.gif">
+        </div>
+    </body>
+</html>
diff --git a/examples/legacy-site-app/public/favicon.ico b/examples/legacy-site-app/public/favicon.ico
diff --git a/examples/legacy-site-app/public/img/200-wrangler-ferris.gif b/examples/legacy-site-app/public/img/200-wrangler-ferris.gif
diff --git a/examples/legacy-site-app/public/img/404-wrangler-ferris.gif b/examples/legacy-site-app/public/img/404-wrangler-ferris.gif
diff --git a/examples/legacy-site-app/public/index.html b/examples/legacy-site-app/public/index.html
@@ -0,0 +1,40 @@
+<!doctype html>
+<html>
+    <head>
+        <link rel="icon" type="image/x-icon" href="favicon.ico">
+        <link href="https://fonts.googleapis.com/css?family=Pacifico&display=swap" rel="stylesheet">
+        <link href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.min.css" rel="stylesheet">
+        <style>
+            h1 {
+                font-family: Pacifico, sans-serif;
+                font-size: 4em;
+                color: #3eb5f1;
+                margin: 0;
+            }
+
+            h2 {
+                font-weight: 300;
+                font-family: sans-serif;
+            }
+
+            .centered {
+                position: fixed;
+                top: 50%;
+                left: 50%;
+                transform: translate(-50%, -50%);
+                text-align: center;
+            }
+
+            #ferris {
+                width: 75%;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="centered">
+            <h1>200 Success</h1>
+            <h2>Hello World! Welcome to your Workers Site.</h2>
+            <img id="ferris" alt="a happy crab is wearing a cowboy hat and holding a lasso. 200 success." src="./img/200-wrangler-ferris.gif">
+        </div>
+    </body>
+</html>
diff --git a/examples/legacy-site-app/workers-site/.gitignore b/examples/legacy-site-app/workers-site/.gitignore
@@ -0,0 +1,3 @@
+node_modules
+dist
+worker
diff --git a/examples/legacy-site-app/workers-site/index.js b/examples/legacy-site-app/workers-site/index.js
@@ -0,0 +1,81 @@
+import { getAssetFromKV, mapRequestToAsset } from '@cloudflare/kv-asset-handler'
+
+/**
+ * The DEBUG flag will do two things that help during development:
+ * 1. we will skip caching on the edge, which makes it easier to
+ *    debug.
+ * 2. we will return an error message on exception in your Response rather
+ *    than the default 404.html page.
+ */
+const DEBUG = false
+
+addEventListener('fetch', event => {
+  event.respondWith(handleEvent(event))
+})
+
+async function handleEvent(event) {
+  let options = {}
+
+  /**
+   * You can add custom logic to how we fetch your assets
+   * by configuring the function `mapRequestToAsset`
+   */
+  // options.mapRequestToAsset = handlePrefix(/^\/docs/)
+
+  try {
+    if (DEBUG) {
+      // customize caching
+      options.cacheControl = {
+        bypassCache: true,
+      }
+    }
+
+    const page = await getAssetFromKV(event, options)
+
+    // allow headers to be altered
+    const response = new Response(page.body, page)
+
+    response.headers.set('X-XSS-Protection', '1; mode=block')
+    response.headers.set('X-Content-Type-Options', 'nosniff')
+    response.headers.set('X-Frame-Options', 'DENY')
+    response.headers.set('Referrer-Policy', 'unsafe-url')
+    response.headers.set('Feature-Policy', 'none')
+
+    return response
+
+  } catch (e) {
+    // if an error is thrown try to serve the asset at 404.html
+    if (!DEBUG) {
+      try {
+        let notFoundResponse = await getAssetFromKV(event, {
+          mapRequestToAsset: req => new Request(`${new URL(req.url).origin}/404.html`, req),
+        })
+
+        return new Response(notFoundResponse.body, { ...notFoundResponse, status: 404 })
+      } catch (e) {}
+    }
+
+    return new Response(e.message || e.toString(), { status: 500 })
+  }
+}
+
+/**
+ * Here's one example of how to modify a request to
+ * remove a specific prefix, in this case `/docs` from
+ * the url. This can be useful if you are deploying to a
+ * route on a zone, or if you only want your static content
+ * to exist at a specific path.
+ */
+function handlePrefix(prefix) {
+  return request => {
+    // compute the default (e.g. / -> index.html)
+    let defaultAssetKey = mapRequestToAsset(request)
+    let url = new URL(defaultAssetKey.url)
+
+    // strip the prefix from the path for lookup
+    url.pathname = url.pathname.replace(prefix, '/')
+
+    // inherit all other props from the default request
+    return new Request(url.toString(), defaultAssetKey)
+  }
+}
diff --git a/examples/legacy-site-app/workers-site/package-lock.json b/examples/legacy-site-app/workers-site/package-lock.json
diff --git a/examples/legacy-site-app/workers-site/package.json b/examples/legacy-site-app/workers-site/package.json
@@ -0,0 +1,10 @@
+{
+  "private": true,
+  "version": "1.0.0",
+  "description": "A template for kick starting a Cloudflare Workers project",
+  "main": "index.js",
+  "license": "MIT",
+  "dependencies": {
+    "@cloudflare/kv-asset-handler": "~0.1.2"
+  }
+}
diff --git a/examples/legacy-site-app/wrangler.toml b/examples/legacy-site-app/wrangler.toml
@@ -0,0 +1,6 @@
+account_id = ""
+name = "legacy-site-app"
+type = "webpack"
+workers_dev = true
+site = { bucket = "./public" }
+compatibility_date = "2022-04-24"
diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts
@@ -230,8 +230,9 @@ describe("normalizeAndValidateConfig()", () => {
         const expectedConfig: RawConfig = {
           site: {
             bucket: "BUCKET",
-            include: ["INCLUDE_1", "INCLUDE_2"],
+            "entry-point": "my-site",
             exclude: ["EXCLUDE_1", "EXCLUDE_2"],
+            include: ["INCLUDE_1", "INCLUDE_2"],
           },
         };
 
@@ -243,13 +244,22 @@ describe("normalizeAndValidateConfig()", () => {
 
         expect(config).toEqual(expect.objectContaining(expectedConfig));
         expect(diagnostics.hasErrors()).toBe(false);
-        expect(diagnostics.hasWarnings()).toBe(false);
+        expect(diagnostics.hasWarnings()).toBe(true);
+        expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - DEPRECATION: \\"site.entry-point\\":
+              Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
+              \`\`\`
+              main = \\"my-site/index.js\\"
+              \`\`\`"
+        `);
       });
 
       it("should error if `site` config is missing `bucket`", () => {
         const expectedConfig: RawConfig = {
           // @ts-expect-error we're intentionally passing an invalid configuration here
           site: {
+            "entry-point": "workers-site",
             include: ["INCLUDE_1", "INCLUDE_2"],
             exclude: ["EXCLUDE_1", "EXCLUDE_2"],
           },
@@ -262,18 +272,27 @@ describe("normalizeAndValidateConfig()", () => {
         );
 
         expect(config).toEqual(expect.objectContaining(expectedConfig));
+        expect(diagnostics.hasWarnings()).toBe(true);
         expect(diagnostics.hasErrors()).toBe(true);
+        expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - DEPRECATION: \\"site.entry-point\\":
+              Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
+              \`\`\`
+              main = \\"workers-site/index.js\\"
+              \`\`\`"
+        `);
         expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
             - \\"site.bucket\\" is a required field."
         `);
-        expect(diagnostics.hasWarnings()).toBe(false);
       });
 
       it("should error on invalid `site` values", () => {
         const expectedConfig = {
           site: {
             bucket: "BUCKET",
+            "entry-point": 111,
             include: [222, 333],
             exclude: [444, 555],
           },
@@ -286,17 +305,26 @@ describe("normalizeAndValidateConfig()", () => {
         );
 
         expect(config).toEqual(expect.objectContaining(expectedConfig));
-        expect(diagnostics.hasWarnings()).toBe(false);
+        expect(diagnostics.hasWarnings()).toBe(true);
         expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
             - Expected \\"sites.include.[0]\\" to be of type string but got 222.
             - Expected \\"sites.include.[1]\\" to be of type string but got 333.
             - Expected \\"sites.exclude.[0]\\" to be of type string but got 444.
-            - Expected \\"sites.exclude.[1]\\" to be of type string but got 555."
+            - Expected \\"sites.exclude.[1]\\" to be of type string but got 555.
+            - Expected \\"site.entry-point\\" to be of type string but got 111."
+        `);
+        expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - DEPRECATION: \\"site.entry-point\\":
+              Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
+              \`\`\`
+              main = \\"111/index.js\\"
+              \`\`\`"
         `);
       });
 
-      it("should error with a deprecation warning if entry-point is defined", async () => {
+      it("should log a deprecation warning if entry-point is defined", async () => {
         const { config, diagnostics } = normalizeAndValidateConfig(
           {
             site: {
@@ -311,21 +339,24 @@ describe("normalizeAndValidateConfig()", () => {
         expect(config.site).toMatchInlineSnapshot(`
           Object {
             "bucket": "some/path",
+            "entry-point": "some/other/script.js",
             "exclude": Array [],
             "include": Array [],
           }
         `);
         expect(diagnostics.hasWarnings()).toBe(true);
-        expect(diagnostics.hasErrors()).toBe(true);
+        expect(diagnostics.hasErrors()).toBe(false);
         expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
-            - Unexpected fields found in site field: \\"entry-point\\""
+            - DEPRECATION: \\"site.entry-point\\":
+              Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
+              \`\`\`
+              main = \\"some/other/script.js\\"
+              \`\`\`"
         `);
         expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
-            - NO LONGER SUPPORTED: \\"site.entry-point\\":
-              The \`site.entry-point\` config field is no longer used.
-              The entry-point should be specified via the command line or the \`main\` config field."
+          "
         `);
       });
     });