Deprecate gh: and npm: redirects (denoland#1270)

lucacasonato · Jul 3, 2020 · 164f588 · 164f588
1 parent 83df7aa
commit 164f588
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 25 deletions.
diff --git a/components/Registry.tsx b/components/Registry.tsx
@@ -180,6 +180,22 @@ const Registry = () => {
         />
         <div className="">
           <div className="max-w-screen-lg mx-auto px-4 sm:px-6 md:px-8 py-2 pb-8">
+            {name.startsWith("gh:") && (
+              <WarningMessage
+                title="deno.land/x/gh:owner:repo redirect deprecation notice"
+                body={`The https://deno.land/x/gh:owner:repo style redirects are deprecated and will be removed on August 1st 2020. Instead of importing via deno.land you can import this module directly from GitHub${
+                  raw ? ` via the URL ${sourceURL}` : ""
+                }.`}
+              />
+            )}
+            {name.startsWith("npm:") && (
+              <WarningMessage
+                title="deno.land/x/npm:project redirect deprecation notice"
+                body={`The https://deno.land/x/npm:project style redirects are deprecated and will be removed on August 1st 2020. Instead of importing via deno.land you can import this module directly from unpkg.com via the URL ${
+                  raw ? ` via the URL ${sourceURL}` : ""
+                }.`}
+              />
+            )}
             <Breadcrumbs
               name={name}
               version={version}
@@ -270,6 +286,36 @@ const Registry = () => {
   );
 };
 
+function WarningMessage(props: { title: string; body: string }) {
+  return (
+    <div className="rounded-md bg-yellow-50 border border-yellow-200 p-4 my-4">
+      <div className="flex">
+        <div className="flex-shrink-0">
+          <svg
+            className="h-5 w-5 text-yellow-400"
+            fill="currentColor"
+            viewBox="0 0 20 20"
+          >
+            <path
+              fillRule="evenodd"
+              d="M8.257 3.099c.765-1.36 2.722-1.36 3.486 0l5.58 9.92c.75 1.334-.213 2.98-1.742 2.98H4.42c-1.53 0-2.493-1.646-1.743-2.98l5.58-9.92zM11 13a1 1 0 11-2 0 1 1 0 012 0zm-1-8a1 1 0 00-1 1v3a1 1 0 002 0V6a1 1 0 00-1-1z"
+              clipRule="evenodd"
+            ></path>
+          </svg>
+        </div>
+        <div className="ml-3">
+          <h3 className="text-sm leading-5 font-medium text-yellow-800">
+            {props.title}
+          </h3>
+          <div className="mt-2 text-sm leading-5 text-yellow-700">
+            {props.body}
+          </div>
+        </div>
+      </div>
+    </div>
+  );
+}
+
 function ErrorMessage(props: { title: string; body: string }) {
   return (
     <div className="rounded-md bg-red-50 border border-red-200 p-4 my-4">
@@ -295,7 +341,7 @@ function ErrorMessage(props: { title: string; body: string }) {
             {props.body}
           </div>
         </div>
-      </div>{" "}
+      </div>
     </div>
   );
 }

diff --git a/pages/x/index.tsx b/pages/x/index.tsx
@@ -50,12 +50,6 @@ const ThirdPartyRegistryList = () => {
                 default branch, usually <InlineCode>master</InlineCode>.
               </p>
 
-              <p className="text-gray-900 mt-4">
-                Experimental: Use <InlineCode>npm:[package]</InlineCode> or
-                <InlineCode>gh:[owner]:[repo]</InlineCode> as module name to
-                resolve any arbitrary repository or npm package.
-              </p>
-
               <p className="text-gray-900 mt-4">
                 Functionality built-in to Deno is not listed here. The built-in
                 runtime is documented on{" "}

diff --git a/worker/__tests__/handler.ts b/worker/__tests__/handler.ts
@@ -1,6 +1,6 @@
 import { fetch, URL, Request, Response } from "@dollarshaveclub/cloudworker";
 import { handleRequest } from "../src/handler";
-import { needsWarning } from "../src/registry";
+import { needsMasterBranchWarning } from "../src/registry";
 
 /* eslint-env jest */
 
@@ -14,14 +14,31 @@ describe("worker proxying", () => {
     const result = await handleRequest(new Request("https://deno.land/"));
     expect(result.headers.get("Content-Type")).toContain("text/html");
     const text = await result.text();
-    expect(text).toContain('Deno');
+    expect(text).toContain("Deno");
   }, 5000);
 
   test("needsWarning", async () => {
-    expect(needsWarning("/std/http/server.ts")).toEqual(true);
-    expect(needsWarning("/std@v0.1.2/http/server.ts")).toEqual(false);
-    expect(needsWarning("/x/foo/bar.txt")).toEqual(false);
-    expect(needsWarning("/index.html")).toEqual(false);
+    expect(
+      needsMasterBranchWarning({
+        name: "std",
+        version: undefined,
+        url: "https://someurl",
+      })
+    ).toEqual(true);
+    expect(
+      needsMasterBranchWarning({
+        name: "std",
+        version: "v0.1.2",
+        url: "https://someurl",
+      })
+    ).toEqual(false);
+    expect(
+      needsMasterBranchWarning({
+        name: "foo",
+        version: undefined,
+        url: "https://someurl",
+      })
+    ).toEqual(false);
   });
 
   it("/typedoc/ responds with typedoc", async () => {
@@ -41,7 +58,7 @@ describe("worker proxying", () => {
     );
     expect(result.headers.get("Content-Type")).toContain("text/html");
     const text = await result.text();
-    expect(text).toContain('Deno');
+    expect(text).toContain("Deno");
   }, 5000);
 
   it("/std/http/server.ts had x-deno-warning", async () => {
@@ -67,7 +84,7 @@ describe("worker proxying", () => {
     );
     expect(result.headers.get("Content-Type")).toContain("text/html");
     const text = await result.text();
-    expect(text).toContain('Deno');
+    expect(text).toContain("Deno");
   }, 5000);
 
   it("/x/std/version.ts with no Accept responds with raw markdown", async () => {
@@ -89,7 +106,7 @@ describe("worker proxying", () => {
     );
     expect(result.headers.get("Content-Type")).toContain("text/html");
     const text = await result.text();
-    expect(text).toContain('Deno');
+    expect(text).toContain("Deno");
   }, 5000);
 
   it("/std@v0.50.0/version.ts with no Accept responds with raw markdown", async () => {
@@ -111,7 +128,7 @@ describe("worker proxying", () => {
     );
     expect(result.headers.get("Content-Type")).toContain("text/html");
     const text = await result.text();
-    expect(text).toContain('Deno');
+    expect(text).toContain("Deno");
   }, 5000);
 
   it("/x/std@v0.50.0/version.ts with no Accept responds with raw markdown", async () => {

diff --git a/worker/src/registry.ts b/worker/src/registry.ts
@@ -2,22 +2,32 @@ import { parseNameVersion, findEntry } from "../../util/registry_utils";
 
 export async function handleRegistryRequest(url: URL): Promise<Response> {
   console.log("registry request", url.pathname);
-  const remoteUrl = getRegistrySourceURL(url.pathname);
-  if (!remoteUrl) {
+  const entry = getRegistrySourceURL(url.pathname);
+  if (!entry) {
     return new Response("Not in database.json: " + url.pathname, {
       status: 404,
       statusText: "Not Found",
       headers: { "content-type": "text/plain" },
     });
   }
 
-  let response = await fetch(remoteUrl);
+  let response = await fetch(entry.url);
   response = new Response(response.body, response);
-  if (needsWarning(url.pathname)) {
+  if (needsMasterBranchWarning(entry)) {
     response.headers.set(
       "X-Deno-Warning",
       `Implicitly using master branch ${url}`
     );
+  } else if (needsGHDeprecationWarning(entry)) {
+    response.headers.set(
+      "X-Deno-Warning",
+      `The https://deno.land/x/gh:owner:repo redirects are deprecated will be removed on August 1st 2020. Import ${entry.url} instead of ${url}`
+    );
+  } else if (needsNPMDeprecationWarning(entry)) {
+    response.headers.set(
+      "X-Deno-Warning",
+      `The https://deno.land/x/npm:project redirects are deprecated will be removed on August 1st 2020. Import ${entry.url} instead of ${url}`
+    );
   }
   const originContentType = response.headers.get("content-type");
   if (
@@ -39,11 +49,25 @@ export async function handleRegistryRequest(url: URL): Promise<Response> {
   return response;
 }
 
-export function needsWarning(pathname: string): boolean {
-  return pathname.startsWith("/std") && !pathname.startsWith("/std@");
+export function needsMasterBranchWarning(entry: Entry): boolean {
+  return entry.name === "std" && entry.version === undefined;
+}
+
+export function needsGHDeprecationWarning(entry: Entry): boolean {
+  return entry.name.startsWith("gh:");
+}
+
+export function needsNPMDeprecationWarning(entry: Entry): boolean {
+  return entry.name.startsWith("npm:");
+}
+
+export interface Entry {
+  name: string;
+  version: string | undefined;
+  url: string;
 }
 
-export function getRegistrySourceURL(pathname: string): string | undefined {
+export function getRegistrySourceURL(pathname: string): Entry | undefined {
   if (pathname.startsWith("/std")) {
     return getRegistrySourceURL("/x" + pathname);
   }
@@ -55,5 +79,10 @@ export function getRegistrySourceURL(pathname: string): string | undefined {
   const [name, version] = parseNameVersion(nameBranch);
   const path = rest.join("/");
   const entry = findEntry(name);
-  return entry?.getSourceURL("/" + path, version);
+  if (!entry) return undefined;
+  return {
+    name,
+    version,
+    url: entry.getSourceURL("/" + path, version),
+  };
 }