-
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 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
This change increases size: out/debug/obj/libdeno/from_snapshot.o 19M -> 34M out/release/deno 32M -> 47M
- Loading branch information
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 |
---|---|---|
|
@@ -270,7 +270,8 @@ bool Execute(v8::Local<v8::Context> context, const char* js_filename, | |
} | ||
|
||
void InitializeContext(v8::Isolate* isolate, v8::Local<v8::Context> context, | ||
const char* js_filename, const char* js_source) { | ||
const char* js_filename, const std::string& js_source, | ||
const std::string* source_map) { | ||
v8::HandleScope handle_scope(isolate); | ||
v8::Context::Scope context_scope(context); | ||
|
||
|
@@ -293,11 +294,19 @@ void InitializeContext(v8::Isolate* isolate, v8::Local<v8::Context> context, | |
|
||
skip_onerror = true; | ||
{ | ||
auto source = deno::v8_str(js_source); | ||
auto source = deno::v8_str(js_source.c_str()); | ||
CHECK(global->Set(context, deno::v8_str("mainSource"), source).FromJust()); | ||
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. May want to reflect this in |
||
|
||
bool r = deno::ExecuteV8StringSource(context, js_filename, source); | ||
CHECK(r); | ||
|
||
if (source_map != nullptr) { | ||
CHECK_GT(source_map->length(), 1u); | ||
std::string set_source_map = "setMainSourceMap( " + *source_map + " )"; | ||
CHECK_GT(set_source_map.length(), source_map->length()); | ||
r = deno::Execute(context, "set_source_map.js", set_source_map.c_str()); | ||
CHECK(r); | ||
} | ||
} | ||
skip_onerror = false; | ||
} | ||
|
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.