diff --git a/js/compiler.ts b/js/compiler.ts index bac3289371..20ddbae9fc 100644 --- a/js/compiler.ts +++ b/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 - inlineSourceMap: true, - inlineSources: true, + sourceMap: 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, - "Only single file should be output." + output.outputFiles.length === 2, + `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; diff --git a/js/compiler_test.ts b/js/compiler_test.ts index 6bf7ac2d65..d7a5c877c4 100644 --- a/js/compiler_test.ts +++ b/js/compiler_test.ts @@ -9,11 +9,12 @@ import * as ts from "typescript"; const { DenoCompiler } = (deno as any)._compiler; interface ModuleInfo { - moduleName: string | null; - filename: string | null; - mediaType: MediaType | null; - sourceCode: string | null; - outputCode: string | null; + moduleName: string | undefined; + filename: string | undefined; + mediaType: MediaType | undefined; + sourceCode: string | undefined; + outputCode: string | undefined; + sourceMap: string | undefined; } const compilerInstance = DenoCompiler.instance(); @@ -36,18 +37,20 @@ enum MediaType { } function mockModuleInfo( - moduleName: string | null, - filename: string | null, - mediaType: MediaType | null, - sourceCode: string | null, - outputCode: string | null + moduleName: string | undefined, + filename: string | undefined, + mediaType: MediaType | undefined, + sourceCode: string | undefined, + 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 { - codeCacheStack.push({ fileName, sourceCode, outputCode }); + codeCache( + 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(); - for (const key of [ + const expectedKeys = [ "allowJs", + "checkJs", "module", "outDir", - "inlineSourceMap", - "inlineSources", + "sourceMap", "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() { diff --git a/js/os.ts b/js/os.ts index 0d54de998c..30c9743488 100644 --- a/js/os.ts +++ b/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. diff --git a/src/deno_dir.rs b/src/deno_dir.rs index b0e9ba43c7..1144bd0abb 100644 --- a/src/deno_dir.rs +++ b/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 { - let path = self.cache_path(filename, source_code); - debug!("load_cache {}", path.display()); - fs::read_to_string(&path) + ) -> Result<(String, String), std::io::Error> { + let (output_code, source_map) = self.cache_path(filename, source_code); + debug!( + "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, + pub maybe_source_map: Option, } #[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() - .join("gen/a3e29aece8d35a19bf9da2bb1c086af71fb36ed5.js"), + ( + temp_dir + .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()); diff --git a/src/msg.fbs b/src/msg.fbs index a3c9bcb849..e7ebe66842 100644 --- a/src/msg.fbs +++ b/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 { diff --git a/src/ops.rs b/src/ops.rs index b7a20a46ea..95522fb6b8 100644 --- a/src/ops.rs +++ b/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 { - Some(ref output_code) => { - msg_args.output_code = Some(builder.create_string(output_code)); - } - _ => (), - }; + if let Some(ref output_code) = out.maybe_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()) }())) }