Fix JavaScript dependencies in bundles. (#4215)

Fixes #4602

We turned off `allowJs` by default, to keep the compiler from grabbing
a bunch of files that it wouldn't actually do anything useful with.  On
the other hand, this caused problems with bundles, where the compiler
needs to gather all the dependencies, including JavaScript ones.  This
fixes this so that when we are bundling, we analyse JavaScript imports
in the compiler.

6 files changed, 82 insertions, 23 deletions

diff --git a/cli/js/compiler.ts b/cli/js/compiler.ts
index 6cd5a6590..79d517df2 100644
--- a/cli/js/compiler.ts
+++ b/cli/js/compiler.ts
@@ -140,7 +140,7 @@ async function tsCompilerOnMessage({
       const resolvedRootModules = await processImports(
         rootNames.map(rootName => [rootName, rootName]),
         undefined,
-        host.getCompilationSettings().checkJs
+        bundle || host.getCompilationSettings().checkJs
       );
 
       let emitSkipped = true;
@@ -208,27 +208,46 @@ async function tsCompilerOnMessage({
         ? rootName
         : resolveModules([rootName])[0];
 
-      // recursively process imports, loading each file into memory.  If there
-      // are sources, these files are pulled out of the there, otherwise the
-      // files are retrieved from the privileged side
-      const rootNames = sources
-        ? processLocalImports(sources, [[resolvedRootName, resolvedRootName]])
-        : await processImports([[resolvedRootName, resolvedRootName]]);
-
       // if there are options, convert them into TypeScript compiler options,
       // and resolve any external file references
       let convertedOptions: ts.CompilerOptions | undefined;
+      let additionalFiles: string[] | undefined;
       if (options) {
         const result = convertCompilerOptions(options);
         convertedOptions = result.options;
-        if (result.files) {
-          // any files supplied in the configuration are resolved externally,
-          // even if sources are provided
-          const resolvedNames = resolveModules(result.files);
-          rootNames.push(
-            ...(await processImports(resolvedNames.map(rn => [rn, rn])))
+        additionalFiles = result.files;
+      }
+
+      const checkJsImports =
+        bundle || (convertedOptions && convertedOptions.checkJs);
+
+      // recursively process imports, loading each file into memory.  If there
+      // are sources, these files are pulled out of the there, otherwise the
+      // files are retrieved from the privileged side
+      const rootNames = sources
+        ? processLocalImports(
+            sources,
+            [[resolvedRootName, resolvedRootName]],
+            undefined,
+            checkJsImports
+          )
+        : await processImports(
+            [[resolvedRootName, resolvedRootName]],
+            undefined,
+            checkJsImports
           );
-        }
+
+      if (additionalFiles) {
+        // any files supplied in the configuration are resolved externally,
+        // even if sources are provided
+        const resolvedNames = resolveModules(additionalFiles);
+        rootNames.push(
+          ...(await processImports(
+            resolvedNames.map(rn => [rn, rn]),
+            undefined,
+            checkJsImports
+          ))
+        );
       }
 
       const state: WriteFileState = {
diff --git a/cli/js/compiler_api_test.ts b/cli/js/compiler_api_test.ts
index 64ba70afb..fcca0a22b 100644
--- a/cli/js/compiler_api_test.ts
+++ b/cli/js/compiler_api_test.ts
@@ -135,6 +135,15 @@ test(async function bundleApiConfig() {
   assert(!actual.includes(`random`));
 });
 
+test(async function bundleApiJsModules() {
+  const [diagnostics, actual] = await bundle("/foo.js", {
+    "/foo.js": `export * from "./bar.js";\n`,
+    "/bar.js": `export const bar = "bar";\n`
+  });
+  assert(diagnostics == null);
+  assert(actual.includes(`System.register("bar",`));
+});
+
 test(async function diagnosticsTest() {
   const [diagnostics] = await compile("/foo.ts", {
     "/foo.ts": `document.getElementById("foo");`
diff --git a/cli/js/compiler_host.ts b/cli/js/compiler_host.ts
index 78ed4889a..585a7833e 100644
--- a/cli/js/compiler_host.ts
+++ b/cli/js/compiler_host.ts
@@ -37,6 +37,7 @@ export interface ConfigureResponse {
 /** Options that need to be used when generating a bundle (either trusted or
  * runtime). */
 export const defaultBundlerOptions: ts.CompilerOptions = {
+  allowJs: true,
   inlineSourceMap: false,
   module: ts.ModuleKind.System,
   outDir: undefined,
diff --git a/cli/js/compiler_imports.ts b/cli/js/compiler_imports.ts
index 6e8a6585d..903bcfe4d 100644
--- a/cli/js/compiler_imports.ts
+++ b/cli/js/compiler_imports.ts
@@ -120,7 +120,7 @@ export function processLocalImports(
   sources: Record<string, string>,
   specifiers: Array<[string, string]>,
   referrer?: string,
-  checkJs = false
+  processJsImports = false
 ): string[] {
   if (!specifiers.length) {
     return [];
@@ -145,9 +145,9 @@ export function processLocalImports(
     if (!sourceFile.processed) {
       processLocalImports(
         sources,
-        sourceFile.imports(checkJs),
+        sourceFile.imports(processJsImports),
         sourceFile.url,
-        checkJs
+        processJsImports
       );
     }
   }
@@ -163,7 +163,7 @@ export function processLocalImports(
 export async function processImports(
   specifiers: Array<[string, string]>,
   referrer?: string,
-  checkJs = false
+  processJsImports = false
 ): Promise<string[]> {
   if (!specifiers.length) {
     return [];
@@ -179,9 +179,9 @@ export async function processImports(
     sourceFile.cache(specifiers[i][0], referrer);
     if (!sourceFile.processed) {
       await processImports(
-        sourceFile.imports(checkJs),
+        sourceFile.imports(processJsImports),
         sourceFile.url,
-        checkJs
+        processJsImports
       );
     }
   }
diff --git a/cli/js/compiler_sourcefile.ts b/cli/js/compiler_sourcefile.ts
index 23fefbf41..cc7d4aa3e 100644
--- a/cli/js/compiler_sourcefile.ts
+++ b/cli/js/compiler_sourcefile.ts
@@ -91,7 +91,7 @@ export class SourceFile {
   }
 
   /** Process the imports for the file and return them. */
-  imports(checkJs: boolean): Array<[string, string]> {
+  imports(processJsImports: boolean): Array<[string, string]> {
     if (this.processed) {
       throw new Error("SourceFile has already been processed.");
     }
@@ -134,7 +134,7 @@ export class SourceFile {
       }
     } else if (
       !(
-        !checkJs &&
+        !processJsImports &&
         (this.mediaType === MediaType.JavaScript ||
           this.mediaType === MediaType.JSX)
       )
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
index d7e807793..a475e9111 100644
--- a/cli/tests/integration_tests.rs
+++ b/cli/tests/integration_tests.rs
@@ -437,6 +437,36 @@ fn bundle_tla() {
 }
 
 #[test]
+fn bundle_js() {
+  use tempfile::TempDir;
+
+  // First we have to generate a bundle of some module that has exports.
+  let mod6 = util::root_path().join("cli/tests/subdir/mod6.js");
+  assert!(mod6.is_file());
+  let t = TempDir::new().expect("tempdir fail");
+  let bundle = t.path().join("mod6.bundle.js");
+  let mut deno = util::deno_cmd()
+    .current_dir(util::root_path())
+    .arg("bundle")
+    .arg(mod6)
+    .arg(&bundle)
+    .spawn()
+    .expect("failed to spawn script");
+  let status = deno.wait().expect("failed to wait for the child process");
+  assert!(status.success());
+  assert!(bundle.is_file());
+
+  let output = util::deno_cmd()
+    .current_dir(util::root_path())
+    .arg("run")
+    .arg(&bundle)
+    .output()
+    .expect("failed to spawn script");
+  // check that nothing went to stderr
+  assert_eq!(output.stderr, b"");
+}
+
+#[test]
 fn repl_test_console_log() {
   let (out, err, code) = util::run_and_collect_output(
     "repl",