fix: deno-types directive should have higher precedence than X-TypeSc…

…ript-Types header (denoland#6761)
nayeemrmn · Jul 24, 2020 · b45f9a7 · b45f9a7
1 parent f162ced
commit b45f9a7
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 6 deletions.
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
@@ -1961,6 +1961,12 @@ itest!(type_directives_redirect {
   http_server: true,
 });
 
+itest!(type_headers_deno_types {
+  args: "run --reload type_headers_deno_types.ts",
+  output: "type_headers_deno_types.ts.out",
+  http_server: true,
+});
+
 itest!(ts_type_imports {
   args: "run --reload ts_type_imports.ts",
   output: "ts_type_imports.ts.out",

diff --git a/cli/tests/type_headers_deno_types.ts b/cli/tests/type_headers_deno_types.ts
@@ -0,0 +1,18 @@
+/**
+ * Following import uses two distinct ways to provide types:
+ * - X-TypeScript-Types headers
+ * - @deno-types directive
+ * 
+ * Because "@deno-types" directive must be placed by user explicitly it
+ * should have higher precedence than type header.
+ * 
+ * This is verified by providing conflicting type declaration
+ * depending on a way. There should be no TS error, otherwise
+ * it means that wrong type declarations are used (from X-TypeScript-Types)
+ * header.
+ */
+
+// @deno-types="http://127.0.0.1:4545/type_headers_deno_types.foo.d.ts"
+import { foo } from "http://127.0.0.1:4545/type_headers_deno_types.foo.js";
+
+foo("hello");
diff --git a/cli/tests/type_headers_deno_types.ts.out b/cli/tests/type_headers_deno_types.ts.out
@@ -0,0 +1,5 @@
+Download http://[WILDCARD]:4545/type_headers_deno_types.foo.js
+Download http://[WILDCARD]:4545/type_headers_deno_types.foo.d.ts
+Download http://[WILDCARD]:4545/type_headers_deno_types.d.ts
+Check [WILDCARD]/type_headers_deno_types.ts
+hello
diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
@@ -869,11 +869,13 @@ delete Object.prototype.__proto__;
           importedFile.mediaType === MediaType.JSX;
         // If JS or JSX perform substitution for types if available
         if (isJsOrJsx) {
-          if (importedFile.typeHeaders.length > 0) {
+          // @deno-types has highest precedence, followed by
+          // X-TypeScript-Types header
+          if (importDesc.resolvedTypeDirective) {
+            mappedUrl = importDesc.resolvedTypeDirective;
+          } else if (importedFile.typeHeaders.length > 0) {
             const typeHeaders = importedFile.typeHeaders[0];
             mappedUrl = typeHeaders.resolvedSpecifier;
-          } else if (importDesc.resolvedTypeDirective) {
-            mappedUrl = importDesc.resolvedTypeDirective;
           } else if (importedFile.typesDirectives.length > 0) {
             const typeDirective = importedFile.typesDirectives[0];
             mappedUrl = typeDirective.resolvedSpecifier;
@@ -928,11 +930,13 @@ delete Object.prototype.__proto__;
           importedFile.mediaType === MediaType.JSX;
         // If JS or JSX perform substitution for types if available
         if (isJsOrJsx) {
-          if (importedFile.typeHeaders.length > 0) {
+          // @deno-types has highest precedence, followed by
+          // X-TypeScript-Types header
+          if (importDesc.resolvedTypeDirective) {
+            mappedUrl = importDesc.resolvedTypeDirective;
+          } else if (importedFile.typeHeaders.length > 0) {
             const typeHeaders = importedFile.typeHeaders[0];
             mappedUrl = typeHeaders.resolvedSpecifier;
-          } else if (importDesc.resolvedTypeDirective) {
-            mappedUrl = importDesc.resolvedTypeDirective;
           } else if (importedFile.typesDirectives.length > 0) {
             const typeDirective = importedFile.typesDirectives[0];
             mappedUrl = typeDirective.resolvedSpecifier;

diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs
@@ -293,6 +293,39 @@ pub async fn run_all_servers() {
       );
       res
     }))
+    .or(warp::path!("type_headers_deno_types.foo.js").map(|| {
+      let mut res = Response::new(Body::from("export function foo(text) { console.log(text); }"));
+      let h = res.headers_mut();
+      h.insert(
+        "Content-type",
+        HeaderValue::from_static("application/javascript"),
+      );
+      h.insert(
+        "X-TypeScript-Types",
+        HeaderValue::from_static(
+          "http://localhost:4545/type_headers_deno_types.d.ts",
+        ),
+      );
+      res
+    }))
+    .or(warp::path!("type_headers_deno_types.d.ts").map(|| {
+      let mut res = Response::new(Body::from("export function foo(text: number): void;"));
+      let h = res.headers_mut();
+      h.insert(
+        "Content-type",
+        HeaderValue::from_static("application/typescript"),
+      );
+      res
+    }))
+    .or(warp::path!("type_headers_deno_types.foo.d.ts").map(|| {
+      let mut res = Response::new(Body::from("export function foo(text: string): void;"));
+      let h = res.headers_mut();
+      h.insert(
+        "Content-type",
+        HeaderValue::from_static("application/typescript"),
+      );
+      res
+    }))
     .or(warp::path!("cli"/"tests"/"subdir"/"xTypeScriptTypesRedirect.d.ts").map(|| {
       let mut res = Response::new(Body::from(
         "import './xTypeScriptTypesRedirected.d.ts';",