Separate source map from output code.

js/compiler.ts

@@ -49,6 +49,8 @@ type ModuleSpecifier = string;
 type OutputCode = string;
 /** The original source code */
 type SourceCode = string;
+/** The output source map */
+type SourceMap = string;
 
 /** Abstraction of the APIs required from the `os` module so they can be
  * easily mocked.
@@ -88,7 +90,8 @@ export class ModuleMetaData implements ts.IScriptSnapshot {
     public readonly fileName: ModuleFileName,
     public readonly mediaType: MediaType,
     public readonly sourceCode: SourceCode = "",
-    public outputCode: OutputCode = ""
+    public outputCode: OutputCode = "",
+    public sourceMap: SourceMap = ""
   ) {
     if (outputCode !== "" || fileName.endsWith(".d.ts")) {
       this.scriptVersion = "1";
@@ -155,9 +158,7 @@ export class DenoCompiler
     checkJs: true,
     module: ts.ModuleKind.AMD,
     outDir: "$deno$",
-    // TODO https://github.com/denoland/deno/issues/23
+    sourceMap: true,
-    inlineSourceMap: true,
-    inlineSources: true,
     stripComments: true,
     target: ts.ScriptTarget.ESNext
   };
@@ -366,6 +367,7 @@ export class DenoCompiler
    * compiler instance.
    */
   private _setupSourceMaps(): void {
+    let lastModule: ModuleMetaData | undefined;
     sourceMaps.install({
       installPrepareStackTrace: true,
       getGeneratedContents: (fileName: string): string | RawSourceMap => {
@@ -377,13 +379,25 @@ export class DenoCompiler
           return libdeno.mainSourceMap;
         } else if (fileName === "deno_main.js") {
           return "";
-        } else {
+        } else if (!fileName.endsWith(".map")) {
           const moduleMetaData = this._moduleMetaDataMap.get(fileName);
           if (!moduleMetaData) {
-            this._log("compiler.getGeneratedContents cannot find", fileName);
+            lastModule = undefined;
             return "";
           }
+          lastModule = moduleMetaData;
           return moduleMetaData.outputCode;
+        } else {
+          if (lastModule && lastModule.sourceMap) {
+            // Assuming the the map will always be asked for after the source
+            // code.
+            const { sourceMap } = lastModule;
+            lastModule = undefined;
+            return sourceMap;
+          } else {
+            // Errors thrown here are caught by source-map.
+            throw new Error(`Unable to find source map: "${fileName}"`);
+          }
         }
       }
     });
@@ -431,19 +445,26 @@ export class DenoCompiler
 
     assert(!output.emitSkipped, "The emit was skipped for an unknown reason.");
 
-    // Currently we are inlining source maps, there should be only 1 output file
-    // See: https://github.com/denoland/deno/issues/23
     assert(
-      output.outputFiles.length === 1,
+      output.outputFiles.length === 2,
-      "Only single file should be output."
+      `Expected 2 files to be emitted, got ${output.outputFiles.length}.`
     );
 
-    const [outputFile] = output.outputFiles;
+    const [sourceMapFile, outputFile] = output.outputFiles;
+    assert(
+      sourceMapFile.name.endsWith(".map"),
+      "Expected first emitted file to be a source map"
+    );
+    assert(
+      outputFile.name.endsWith(".js"),
+      "Expected second emitted file to be JavaScript"
+    );
     const outputCode = (moduleMetaData.outputCode = `${
       outputFile.text
     }\n//# sourceURL=${fileName}`);
+    const sourceMap = (moduleMetaData.sourceMap = sourceMapFile.text);
     moduleMetaData.scriptVersion = "1";
-    this._os.codeCache(fileName, sourceCode, outputCode);
+    this._os.codeCache(fileName, sourceCode, outputCode, sourceMap);
     return moduleMetaData.outputCode;
   }
 
@@ -492,6 +513,7 @@ export class DenoCompiler
     let mediaType = MediaType.Unknown;
     let sourceCode: SourceCode | undefined;
     let outputCode: OutputCode | undefined;
+    let sourceMap: SourceMap | undefined;
     if (
       moduleSpecifier.startsWith(ASSETS) ||
       containingFile.startsWith(ASSETS)
@@ -506,6 +528,7 @@ export class DenoCompiler
       sourceCode = assetSourceCode[assetName];
       fileName = `${ASSETS}/${assetName}`;
       outputCode = "";
+      sourceMap = "";
     } else {
       // We query Rust with a CodeFetch message. It will load the sourceCode,
       // and if there is any outputCode cached, will return that as well.
@@ -515,6 +538,7 @@ export class DenoCompiler
       mediaType = fetchResponse.mediaType;
       sourceCode = fetchResponse.sourceCode;
       outputCode = fetchResponse.outputCode;
+      sourceMap = fetchResponse.sourceMap;
     }
     assert(moduleId != null, "No module ID.");
     assert(fileName != null, "No file name.");
@@ -528,6 +552,7 @@ export class DenoCompiler
       sourceCode && sourceCode.length
     );
     this._log("resolveModule has outputCode:", outputCode != null);
+    this._log("resolveModule has source map:", sourceMap != null);
     this._log("resolveModule has media type:", MediaType[mediaType]);
     // fileName is asserted above, but TypeScript does not track so not null
     this._setFileName(moduleSpecifier, containingFile, fileName!);
@@ -539,7 +564,8 @@ export class DenoCompiler
       fileName!,
       mediaType,
       sourceCode,
-      outputCode
+      outputCode,
+      sourceMap
     );
     this._moduleMetaDataMap.set(fileName!, moduleMetaData);
     return moduleMetaData;

js/compiler_test.ts

@@ -9,11 +9,12 @@ import * as ts from "typescript";
 const { DenoCompiler } = (deno as any)._compiler;
 
 interface ModuleInfo {
-  moduleName: string | null;
+  moduleName: string | undefined;
-  filename: string | null;
+  filename: string | undefined;
-  mediaType: MediaType | null;
+  mediaType: MediaType | undefined;
-  sourceCode: string | null;
+  sourceCode: string | undefined;
-  outputCode: string | null;
+  outputCode: string | undefined;
+  sourceMap: string | undefined;
 }
 
 const compilerInstance = DenoCompiler.instance();
@@ -36,18 +37,20 @@ enum MediaType {
 }
 
 function mockModuleInfo(
-  moduleName: string | null,
+  moduleName: string | undefined,
-  filename: string | null,
+  filename: string | undefined,
-  mediaType: MediaType | null,
+  mediaType: MediaType | undefined,
-  sourceCode: string | null,
+  sourceCode: string | undefined,
-  outputCode: string | null
+  outputCode: string | undefined,
+  sourceMap: string | undefined
 ): ModuleInfo {
   return {
     moduleName,
     filename,
     mediaType,
     sourceCode,
-    outputCode
+    outputCode,
+    sourceMap
   };
 }
 
@@ -73,6 +76,7 @@ const modAModuleInfo = mockModuleInfo(
   "/root/project/modA.ts",
   MediaType.TypeScript,
   modASource,
+  undefined,
   undefined
 );
 
@@ -88,10 +92,10 @@ const modBModuleInfo = mockModuleInfo(
   "/root/project/modB.ts",
   MediaType.TypeScript,
   modBSource,
+  undefined,
   undefined
 );
 
-// TODO(#23) Remove source map strings from fooBarTsOutput.
 // tslint:disable:max-line-length
 const fooBarTsOutput = `define(["require", "exports", "deno"], function (require, exports, deno) {
     "use strict";
@@ -99,17 +103,21 @@ const fooBarTsOutput = `define(["require", "exports", "deno"], function (require
     console.log(deno);
     exports.foo = "bar";
 });
-//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiYmFyLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiZmlsZTovLy9yb290L3Byb2plY3QvZm9vL2Jhci50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiOzs7SUFDQSxPQUFPLENBQUMsR0FBRyxDQUFDLElBQUksQ0FBQyxDQUFDO0lBQ0wsUUFBQSxHQUFHLEdBQUcsS0FBSyxDQUFDIiwic291cmNlc0NvbnRlbnQiOlsiaW1wb3J0ICogYXMgZGVubyBmcm9tIFwiZGVub1wiO1xuY29uc29sZS5sb2coZGVubyk7XG5leHBvcnQgY29uc3QgZm9vID0gXCJiYXJcIjtcbiJdfQ==
+//# sourceMappingURL=bar.js.map
 //# sourceURL=/root/project/foo/bar.ts`;
 
-// TODO(#23) Remove source map strings from fooBazTsOutput.
+const fooBarTsSourcemap = `{"version":3,"file":"bar.js","sourceRoot":"","sources":["file:///root/project/foo/bar.ts"],"names":[],"mappings":";;;IACA,OAAO,CAAC,GAAG,CAAC,IAAI,CAAC,CAAC;IACL,QAAA,GAAG,GAAG,KAAK,CAAC"}`;
+
 const fooBazTsOutput = `define(["require", "exports", "./bar.ts"], function (require, exports, bar_ts_1) {
   "use strict";
   Object.defineProperty(exports, "__esModule", { value: true });
   console.log(bar_ts_1.foo);
 });
-//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiYmF6LmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiZmlsZTovLy9yb290L3Byb2plY3QvZm9vL2Jhei50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiOzs7SUFDQSxPQUFPLENBQUMsR0FBRyxDQUFDLFlBQUcsQ0FBQyxDQUFDIiwic291cmNlc0NvbnRlbnQiOlsiaW1wb3J0IHsgZm9vIH0gZnJvbSBcIi4vYmFyLnRzXCI7XG5jb25zb2xlLmxvZyhmb28pO1xuIl19
+//# sourceMappingURL=baz.js.map
 //# sourceURL=/root/project/foo/baz.ts`;
+
+// This is not a valid map, just mock data
+const fooBazTsSourcemap = `{"version":3,"file":"baz.js","sourceRoot":"","sources":["file:///root/project/foo/baz.ts"],"names":[],"mappings":""}`;
 // tslint:enable:max-line-length
 
 const moduleMap: {
@@ -121,6 +129,7 @@ const moduleMap: {
       "/root/project/foo/bar.ts",
       MediaType.TypeScript,
       fooBarTsSource,
+      null,
       null
     ),
     "foo/baz.ts": mockModuleInfo(
@@ -128,7 +137,8 @@ const moduleMap: {
       "/root/project/foo/baz.ts",
       MediaType.TypeScript,
       fooBazTsSource,
-      fooBazTsOutput
+      fooBazTsOutput,
+      fooBazTsSourcemap
     ),
     "modA.ts": modAModuleInfo,
     "some.txt": mockModuleInfo(
@@ -136,6 +146,7 @@ const moduleMap: {
       "/root/project/some.text",
       MediaType.Unknown,
       "console.log();",
+      null,
       null
     )
   },
@@ -145,7 +156,8 @@ const moduleMap: {
       "/root/project/foo/bar.ts",
       MediaType.TypeScript,
       fooBarTsSource,
-      fooBarTsOutput
+      fooBarTsOutput,
+      fooBarTsSourcemap
     )
   },
   "/root/project/modA.ts": {
@@ -160,6 +172,7 @@ const moduleMap: {
       "/moduleKinds/foo.ts",
       MediaType.TypeScript,
       "console.log('foo');",
+      undefined,
       undefined
     ),
     "foo.d.ts": mockModuleInfo(
@@ -167,6 +180,7 @@ const moduleMap: {
       "/moduleKinds/foo.d.ts",
       MediaType.TypeScript,
       "console.log('foo');",
+      undefined,
       undefined
     ),
     "foo.js": mockModuleInfo(
@@ -174,6 +188,7 @@ const moduleMap: {
       "/moduleKinds/foo.js",
       MediaType.JavaScript,
       "console.log('foo');",
+      undefined,
       undefined
     ),
     "foo.json": mockModuleInfo(
@@ -181,6 +196,7 @@ const moduleMap: {
       "/moduleKinds/foo.json",
       MediaType.Json,
       "console.log('foo');",
+      undefined,
       undefined
     ),
     "foo.txt": mockModuleInfo(
@@ -188,6 +204,7 @@ const moduleMap: {
       "/moduleKinds/foo.txt",
       MediaType.JavaScript,
       "console.log('foo');",
+      undefined,
       undefined
     )
   }
@@ -211,6 +228,7 @@ let codeCacheStack: Array<{
   fileName: string;
   sourceCode: string;
   outputCode: string;
+  sourceMap: string;
 }> = [];
 let codeFetchStack: Array<{
   moduleSpecifier: string;
@@ -230,18 +248,25 @@ function logMock(...args: any[]): void {
   logStack.push(args);
 }
 const osMock = {
-  codeCache(fileName: string, sourceCode: string, outputCode: string): void {
+  codeCache(
-    codeCacheStack.push({ fileName, sourceCode, outputCode });
+    fileName: string,
+    sourceCode: string,
+    outputCode: string,
+    sourceMap: string
+  ): void {
+    codeCacheStack.push({ fileName, sourceCode, outputCode, sourceMap });
     if (fileName in moduleCache) {
       moduleCache[fileName].sourceCode = sourceCode;
       moduleCache[fileName].outputCode = outputCode;
+      moduleCache[fileName].sourceMap = sourceMap;
     } else {
       moduleCache[fileName] = mockModuleInfo(
         fileName,
         fileName,
         MediaType.TypeScript,
         sourceCode,
-        outputCode
+        outputCode,
+        sourceMap
       );
     }
   },
@@ -252,7 +277,7 @@ const osMock = {
         return moduleMap[containingFile][moduleSpecifier];
       }
     }
-    return mockModuleInfo(null, null, null, null, null);
+    return mockModuleInfo(null, null, null, null, null, null);
   },
   exit(code: number): never {
     throw new Error(`os.exit(${code})`);
@@ -373,6 +398,7 @@ test(function compilerRun() {
   assert(moduleMetaData.hasRun);
   assertEqual(moduleMetaData.sourceCode, fooBarTsSource);
   assertEqual(moduleMetaData.outputCode, fooBarTsOutput);
+  assertEqual(moduleMetaData.sourceMap, fooBarTsSourcemap);
   assertEqual(moduleMetaData.exports, { foo: "bar" });
 
   assertEqual(
@@ -385,6 +411,11 @@ test(function compilerRun() {
     1,
     "Compiled code should have only been cached once."
   );
+  const [codeCacheCall] = codeCacheStack;
+  assertEqual(codeCacheCall.fileName, "/root/project/foo/bar.ts");
+  assertEqual(codeCacheCall.sourceCode, fooBarTsSource);
+  assertEqual(codeCacheCall.outputCode, fooBarTsOutput);
+  assertEqual(codeCacheCall.sourceMap, fooBarTsSourcemap);
   teardown();
 });
 
@@ -456,6 +487,7 @@ test(function compilerResolveModule() {
   );
   assertEqual(moduleMetaData.sourceCode, fooBazTsSource);
   assertEqual(moduleMetaData.outputCode, fooBazTsOutput);
+  assertEqual(moduleMetaData.sourceMap, fooBazTsSourcemap);
   assert(!moduleMetaData.hasRun);
   assert(!moduleMetaData.deps);
   assertEqual(moduleMetaData.exports, {});
@@ -507,18 +539,20 @@ test(function compilerGetModuleDependencies() {
 // TypeScript LanguageServiceHost APIs
 
 test(function compilerGetCompilationSettings() {
-  const result = compilerInstance.getCompilationSettings();
+  const expectedKeys = [
-  for (const key of [
     "allowJs",
+    "checkJs",
     "module",
     "outDir",
-    "inlineSourceMap",
+    "sourceMap",
-    "inlineSources",
     "stripComments",
     "target"
-  ]) {
+  ];
+  const result = compilerInstance.getCompilationSettings();
+  for (const key of expectedKeys) {
     assert(key in result, `Expected "${key}" in compiler options.`);
   }
+  assertEqual(Object.keys(result).length, expectedKeys.length);
 });
 
 test(function compilerGetNewLine() {

js/os.ts

@@ -11,6 +11,7 @@ interface CodeInfo {
   mediaType: msg.MediaType;
   sourceCode: string | undefined;
   outputCode: string | undefined;
+  sourceMap: string | undefined;
 }
 
 /** Exit the Deno process with optional exit code. */
@@ -52,7 +53,8 @@ export function codeFetch(
     filename: codeFetchRes.filename() || undefined,
     mediaType: codeFetchRes.mediaType(),
     sourceCode: codeFetchRes.sourceCode() || undefined,
-    outputCode: codeFetchRes.outputCode() || undefined
+    outputCode: codeFetchRes.outputCode() || undefined,
+    sourceMap: codeFetchRes.sourceMap() || undefined
   };
 }
 
@@ -60,17 +62,20 @@ export function codeFetch(
 export function codeCache(
   filename: string,
   sourceCode: string,
-  outputCode: string
+  outputCode: string,
+  sourceMap: string
 ): void {
   util.log("os.ts codeCache", filename, sourceCode, outputCode);
   const builder = flatbuffers.createBuilder();
   const filename_ = builder.createString(filename);
   const sourceCode_ = builder.createString(sourceCode);
   const outputCode_ = builder.createString(outputCode);
+  const sourceMap_ = builder.createString(sourceMap);
   msg.CodeCache.startCodeCache(builder);
   msg.CodeCache.addFilename(builder, filename_);
   msg.CodeCache.addSourceCode(builder, sourceCode_);
   msg.CodeCache.addOutputCode(builder, outputCode_);
+  msg.CodeCache.addSourceMap(builder, sourceMap_);
   const inner = msg.CodeCache.endCodeCache(builder);
   const baseRes = sendSync(builder, msg.Any.CodeCache, inner);
   assert(baseRes == null); // Expect null or error.

src/deno_dir.rs

@@ -83,19 +83,28 @@ impl DenoDir {
     self: &DenoDir,
     filename: &str,
     source_code: &str,
-  ) -> PathBuf {
+  ) -> (PathBuf, PathBuf) {
     let cache_key = source_code_hash(filename, source_code);
-    self.gen.join(cache_key + ".js")
+    (
+      self.gen.join(cache_key.to_string() + ".js"),
+      self.gen.join(cache_key.to_string() + ".js.map"),
+    )
   }
 
   fn load_cache(
     self: &DenoDir,
     filename: &str,
     source_code: &str,
-  ) -> std::io::Result<String> {
+  ) -> Result<(String, String), std::io::Error> {
-    let path = self.cache_path(filename, source_code);
+    let (output_code, source_map) = self.cache_path(filename, source_code);
-    debug!("load_cache {}", path.display());
+    debug!(
-    fs::read_to_string(&path)
+      "load_cache code: {} map: {}",
+      output_code.display(),
+      source_map.display()
+    );
+    let read_output_code = fs::read_to_string(&output_code)?;
+    let read_source_map = fs::read_to_string(&source_map)?;
+    Ok((read_output_code, read_source_map))
   }
 
   pub fn code_cache(
@@ -103,16 +112,19 @@ impl DenoDir {
     filename: &str,
     source_code: &str,
     output_code: &str,
+    source_map: &str,
   ) -> std::io::Result<()> {
-    let cache_path = self.cache_path(filename, source_code);
+    let (cache_path, source_map_path) = self.cache_path(filename, source_code);
     // TODO(ry) This is a race condition w.r.t to exists() -- probably should
     // create the file in exclusive mode. A worry is what might happen is there
     // are two processes and one reads the cache file while the other is in the
     // midst of writing it.
-    if cache_path.exists() {
+    if cache_path.exists() && source_map_path.exists() {
       Ok(())
     } else {
-      fs::write(cache_path, output_code.as_bytes())
+      fs::write(cache_path, output_code.as_bytes())?;
+      fs::write(source_map_path, source_map.as_bytes())?;
+      Ok(())
     }
   }
 
@@ -176,6 +188,7 @@ impl DenoDir {
         media_type,
         source_code,
         maybe_output_code: None,
+        maybe_source_map: None,
       });
     };
     let default_attempt = use_extension("");
@@ -233,12 +246,13 @@ impl DenoDir {
           Err(err.into())
         }
       }
-      Ok(output_code) => Ok(CodeFetchOutput {
+      Ok((output_code, source_map)) => Ok(CodeFetchOutput {
         module_name: out.module_name,
         filename: out.filename,
         media_type: out.media_type,
         source_code: out.source_code,
         maybe_output_code: Some(output_code),
+        maybe_source_map: Some(source_map),
       }),
     }
   }
@@ -368,6 +382,7 @@ pub struct CodeFetchOutput {
   pub media_type: msg::MediaType,
   pub source_code: String,
   pub maybe_output_code: Option<String>,
+  pub maybe_source_map: Option<String>,
 }
 
 #[cfg(test)]
@@ -382,9 +397,14 @@ pub fn test_setup() -> (TempDir, DenoDir) {
 fn test_cache_path() {
   let (temp_dir, deno_dir) = test_setup();
   assert_eq!(
-    temp_dir
+    (
-      .path()
+      temp_dir
-      .join("gen/a3e29aece8d35a19bf9da2bb1c086af71fb36ed5.js"),
+        .path()
+        .join("gen/a3e29aece8d35a19bf9da2bb1c086af71fb36ed5.js"),
+      temp_dir
+        .path()
+        .join("gen/a3e29aece8d35a19bf9da2bb1c086af71fb36ed5.js.map")
+    ),
     deno_dir.cache_path("hello.ts", "1+2")
   );
 }
@@ -396,12 +416,18 @@ fn test_code_cache() {
   let filename = "hello.js";
   let source_code = "1+2";
   let output_code = "1+2 // output code";
-  let cache_path = deno_dir.cache_path(filename, source_code);
+  let source_map = "{}";
+  let (cache_path, source_map_path) =
+    deno_dir.cache_path(filename, source_code);
   assert!(
     cache_path.ends_with("gen/e8e3ee6bee4aef2ec63f6ec3db7fc5fdfae910ae.js")
   );
+  assert!(
+    source_map_path
+      .ends_with("gen/e8e3ee6bee4aef2ec63f6ec3db7fc5fdfae910ae.js.map")
+  );
 
-  let r = deno_dir.code_cache(filename, source_code, output_code);
+  let r = deno_dir.code_cache(filename, source_code, output_code, source_map);
   r.expect("code_cache error");
   assert!(cache_path.exists());
   assert_eq!(output_code, fs::read_to_string(&cache_path).unwrap());

src/msg.fbs

@@ -146,14 +146,18 @@ table CodeFetchRes {
   module_name: string;
   filename: string;
   media_type: MediaType;
+  // TODO These should be [ubyte].
+  // See: https://github.com/denoland/deno/issues/1113
   source_code: string;
   output_code: string; // Non-empty only if cached.
+  source_map: string; // Non-empty only if cached.
 }
 
 table CodeCache {
   filename: string;
   source_code: string;
   output_code: string;
+  source_map: string;
 }
 
 table Chdir {

src/ops.rs

@@ -257,12 +257,12 @@ fn op_code_fetch(
       source_code: Some(builder.create_string(&out.source_code)),
       ..Default::default()
     };
-    match out.maybe_output_code {
+    if let Some(ref output_code) = out.maybe_output_code {
-      Some(ref output_code) => {
+      msg_args.output_code = Some(builder.create_string(output_code));
-        msg_args.output_code = Some(builder.create_string(output_code));
+    }
-      }
+    if let Some(ref source_map) = out.maybe_source_map {
-      _ => (),
+      msg_args.source_map = Some(builder.create_string(source_map));
-    };
+    }
     let inner = msg::CodeFetchRes::create(builder, &msg_args);
     Ok(serialize_response(
       cmd_id,
@@ -287,8 +287,11 @@ fn op_code_cache(
   let filename = inner.filename().unwrap();
   let source_code = inner.source_code().unwrap();
   let output_code = inner.output_code().unwrap();
+  let source_map = inner.source_map().unwrap();
   Box::new(futures::future::result(|| -> OpResult {
-    state.dir.code_cache(filename, source_code, output_code)?;
+    state
+      .dir
+      .code_cache(filename, source_code, output_code, source_map)?;
     Ok(empty_buf())
   }()))
 }