Source map support #423
Source map support #423
Conversation
ry
force-pushed
the
source_map
branch
4 times, most recently
from
87546d5
to
a1f1bdf
Compare
|
I'm running into some issues: https://gist.github.com/piscisaureus/0103c081d3e3ccf50c1f4587212f3ecd#file-gistfile1-txt |
|
Seems like you don’t have @types/node ? |
js/v8_source_maps.ts
Outdated
| if (!code) { | ||
| return null; | ||
| } | ||
| assert(typeof code === "string"); |
There was a problem hiding this comment.
You can narrow types in TypeScript by either leveraging call flow analysis, the way assert is written is a limitation of CFA and therefore doesn't narrow the type. If you did:
if (typeof code !== "string") {
throw new Error("...");
}After that block, code would be narrowed to string without needing re-assignment without a type assertion.
| @@ -2,8 +2,9 @@ | |||
| // Copyright 2018 the Deno authors. All rights reserved. MIT license. | |||
| // Originated from source-map-support but has been heavily modified for deno. | |||
| import { SourceMapConsumer, MappedPosition } from "source-map"; | |||
| import { RawSourceMap } from "source-map"; | |||
| import * as base64 from "base64-js"; | |||
There was a problem hiding this comment.
While not related to this PR, is there a reason for this? It is only used to encode to an array to convert to a string. Are btoa not available? If it isn't, it should be able to be exposed. One less dependency. 😉
There was a problem hiding this comment.
I'm not sure actually - I suspect it's redundant. This code is forked from source-map-support and base64-js was used there. I'd be happy to remove it.
There was a problem hiding this comment.
I don't think btoa is available - it's DOM api (node doesn't have it either).
There was a problem hiding this comment.
@piscisaureus thanks... Yeah I found the one about adding it in Node.js and a bit of discussion which led to essentially what is a one liner in Node.js for btoa. I guess it is already pulled in via source-map-support though and sort of trying to slimline the runtime is something that can be done down the road.
|
@piscisaureus Maybe your symlinks are messed up and it doesn't create //node_modules or //out/debug/node_modules ? Otherwise I'm not sure... |
ry
force-pushed
the
source_map
branch
4 times, most recently
from
e12ab6f
to
4a5fe4b
Compare
Symlinks are fine. Running tsc directly also no problem, looks like something related to either ninja or rollup. Will look into it. |
It's a dependency of js/v8_source_maps.ts.
Switch the order of the snapshot_creator args, in order to allow for optional source map arg.
|
I have removed the asm optimization from this PR. I will submit separately. |
This change increases compile time and size: Compile time: obj/libdeno/from_snapshot.o 44s -> 85s Size: out/debug/obj/libdeno/from_snapshot.o 19M -> 34M out/debug/gen/snapshot_deno.cc 54M -> 97M out/release/deno 32M -> 47M The compile time of from_snapshot.o is problematic and will be dealt with in future commits.
|
closed in favor of #429 |
Depends on #421