-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Source map support #429
Source map support #429
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,10 +27,15 @@ declare class Console { | |
|
||
interface Window { | ||
console: Console; | ||
mainSource: string; // TODO(ry) This shouldn't be global. | ||
// TODO(ry) These shouldn't be global. | ||
mainSource: string; | ||
setMainSourceMap(sm: string): void; | ||
} | ||
|
||
// Globals in the runtime environment | ||
declare let console: Console; | ||
declare let mainSource: string; // TODO(ry) This shouldn't be global. | ||
declare const window: Window; | ||
|
||
// TODO(ry) These shouldn't be global. | ||
declare let mainSource: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems duplicative with the Window interface above. Do we really need both? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need either - but I'm not sure the right solution now. I will fix it later. |
||
declare function setMainSourceMap(sm: string): void; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ import * as util from "./util"; | |
import { log } from "./util"; | ||
import { assetSourceCode } from "./assets"; | ||
import * as os from "./os"; | ||
//import * as sourceMaps from "./v8_source_maps"; | ||
import * as sourceMaps from "./v8_source_maps"; | ||
import { window, globalEval } from "./globals"; | ||
//import * as deno from "./deno"; | ||
|
||
|
@@ -39,26 +39,39 @@ window.onerror = ( | |
os.exit(1); | ||
}; | ||
|
||
/* | ||
export function setup(mainJs: string, mainMap: string): void { | ||
// This is called during snapshot creation with the contents of | ||
// out/debug/gen/bundle/main.js.map. | ||
import { RawSourceMap } from "source-map"; | ||
let mainSourceMap: RawSourceMap = null; | ||
function setMainSourceMap(rawSourceMap: RawSourceMap) { | ||
util.assert(Number(rawSourceMap.version) === 3); | ||
mainSourceMap = rawSourceMap; | ||
} | ||
window["setMainSourceMap"] = setMainSourceMap; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to reflect this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks - done. (Although it's global right now. Will fix later) |
||
|
||
export function setup(): void { | ||
sourceMaps.install({ | ||
installPrepareStackTrace: true, | ||
getGeneratedContents: (filename: string): string => { | ||
if (filename === "/main.js") { | ||
return mainJs; | ||
} else if (filename === "/main.map") { | ||
return mainMap; | ||
getGeneratedContents: (filename: string): string | RawSourceMap => { | ||
util.log("getGeneratedContents", filename); | ||
if (filename === "gen/bundle/main.js") { | ||
util.assert(window["mainSource"].length > 0); | ||
return window["mainSource"]; | ||
} else if (filename === "main.js.map") { | ||
return mainSourceMap; | ||
} else if (filename === "deno_main.js") { | ||
return ""; | ||
} else { | ||
const mod = FileModule.load(filename); | ||
if (!mod) { | ||
console.error("getGeneratedContents cannot find", filename); | ||
util.log("getGeneratedContents cannot find", filename); | ||
return null; | ||
} | ||
return mod.outputCode; | ||
} | ||
} | ||
}); | ||
} | ||
*/ | ||
|
||
// This class represents a module. We call it FileModule to make it explicit | ||
// that each module represents a single file. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,15 @@ | ||
// Copyright 2014 Evan Wallace | ||
// Copyright 2018 the Deno authors. All rights reserved. MIT license. | ||
// Originated from source-map-support but has been heavily modified for deno. | ||
|
||
// Because NodeJS.CallSite and Error.prepareStackTrace are used we add a | ||
// dependency on the Node types. | ||
// TODO(ry) Ideally this triple slash directive should be removed as we only | ||
// need CallSite and Error.prepareStackTrace but nothing else. | ||
/// <reference types="node" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also just copy those definitions into our own repo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes - into lib.deno.d.ts - I think that's probably the ideal solution. I'll leave this for now since it's easy. |
||
|
||
import { SourceMapConsumer, MappedPosition } from "source-map"; | ||
import { RawSourceMap } from "source-map"; | ||
import * as base64 from "base64-js"; | ||
import { arrayToStr } from "./util"; | ||
|
||
|
@@ -24,7 +32,7 @@ interface Position { | |
line: number; | ||
} | ||
|
||
type GetGeneratedContentsCallback = (fileName: string) => string; | ||
type GetGeneratedContentsCallback = (fileName: string) => string | RawSourceMap; | ||
|
||
let getGeneratedContents: GetGeneratedContentsCallback; | ||
|
||
|
@@ -190,13 +198,16 @@ function loadConsumer(source: string): SourceMapConsumer { | |
if (!code) { | ||
return null; | ||
} | ||
if (typeof code !== "string") { | ||
throw new Error("expected string"); | ||
} | ||
|
||
let sourceMappingURL = retrieveSourceMapURL(code); | ||
if (!sourceMappingURL) { | ||
throw Error("No source map?"); | ||
} | ||
|
||
let sourceMapData: string; | ||
let sourceMapData: string | RawSourceMap; | ||
if (reSourceMap.test(sourceMappingURL)) { | ||
// Support source map URL as a data url | ||
const rawData = sourceMappingURL.slice(sourceMappingURL.indexOf(",") + 1); | ||
|
@@ -209,8 +220,11 @@ function loadConsumer(source: string): SourceMapConsumer { | |
sourceMapData = getGeneratedContents(sourceMappingURL); | ||
} | ||
|
||
const rawSourceMap = | ||
typeof sourceMapData === "string" | ||
? JSON.parse(sourceMapData) | ||
: sourceMapData; | ||
//console.log("sourceMapData", sourceMapData); | ||
const rawSourceMap = JSON.parse(sourceMapData); | ||
consumer = new SourceMapConsumer(rawSourceMap); | ||
consumers.set(source, consumer); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,14 +22,16 @@ v8::StartupData SerializeInternalFields(v8::Local<v8::Object> holder, int index, | |
return {payload, size}; | ||
} | ||
|
||
v8::StartupData MakeSnapshot(const char* js_filename, const char* js_source) { | ||
v8::StartupData MakeSnapshot(const char* js_filename, | ||
const std::string& js_source, | ||
const std::string* source_map) { | ||
auto* creator = new v8::SnapshotCreator(external_references); | ||
auto* isolate = creator->GetIsolate(); | ||
v8::Isolate::Scope isolate_scope(isolate); | ||
{ | ||
v8::HandleScope handle_scope(isolate); | ||
auto context = v8::Context::New(isolate); | ||
InitializeContext(isolate, context, js_filename, js_source); | ||
InitializeContext(isolate, context, js_filename, js_source, source_map); | ||
creator->SetDefaultContext(context, v8::SerializeInternalFieldsCallback( | ||
SerializeInternalFields, nullptr)); | ||
} | ||
|
@@ -45,18 +47,25 @@ v8::StartupData MakeSnapshot(const char* js_filename, const char* js_source) { | |
int main(int argc, char** argv) { | ||
const char* snapshot_out_bin = argv[1]; | ||
const char* js_fn = argv[2]; | ||
const char* source_map_fn = argv[3]; // Optional. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still find _fn a confusing abbreviation for filename. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just following the convention here. I'll avoid it in the future. |
||
|
||
v8::V8::SetFlagsFromCommandLine(&argc, argv, true); | ||
|
||
CHECK_EQ(argc, 3); | ||
CHECK_NE(js_fn, nullptr); | ||
CHECK_NE(snapshot_out_bin, nullptr); | ||
|
||
std::string js_source; | ||
CHECK(deno::ReadFileToString(js_fn, &js_source)); | ||
|
||
std::string source_map; | ||
if (source_map_fn != nullptr) { | ||
CHECK_EQ(argc, 4); | ||
CHECK(deno::ReadFileToString(source_map_fn, &source_map)); | ||
} | ||
|
||
deno_init(); | ||
auto snapshot_blob = deno::MakeSnapshot(js_fn, js_source.c_str()); | ||
auto snapshot_blob = deno::MakeSnapshot( | ||
js_fn, js_source, source_map_fn != nullptr ? &source_map : nullptr); | ||
std::string snapshot_str(snapshot_blob.data, snapshot_blob.raw_size); | ||
|
||
std::ofstream file_(snapshot_out_bin, std::ios::binary); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { throwsError } from "./subdir/mod1.ts"; | ||
|
||
function foo() { | ||
throwsError(); | ||
} | ||
|
||
foo(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string | RawSourcemap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually fixing this caused some unknown error. I'm backing out of that and landing as originally approved. Will fix in follow up.