[Miniflare 3] Serve linked source maps from loopback server (enable breakpoint debugging for TypeScript) #660
Conversation
With cloudflare/workerd#710, `workerd` supports breakpoint debugging! Support for this in Miniflare just worked, assuming you were using a plain JavaScript worker, or you had inline source maps. `workerd` doesn't know where workers are located on disk, it just knows files' locations relative to each other. This means it's unable to resolve locations of corresponding linked `.map` files in `sourceMappingURL` comments. Miniflare _does_ have this information though. This change detects linked source maps and rewrites `sourceMappingURL` comments to `http` URLs pointing to Miniflare's loopback server. This then looks for the source map relative to the known on-disk source location. Source maps' `sourceRoot` attributes are updated to ensure correct locations are displayed in DevTools. **This enables breakpoint debugging for compiled TypeScript with linked source maps!** 🎉 Closes DEVX-872
|
| } | ||
| let map: RawSourceMap; | ||
| try { | ||
| map = JSON.parse(contents); |
There was a problem hiding this comment.
This could become a bottleneck. Sourcemap files can get quite large. JSON.parse can become expensive in those cases. Not only is this code loading the whole file into memory, it then parses the object into memory, and then stringifies it again, and then garbage collection might also take longer than normal.
Ideally we would stream the file from disk to the response.
JSON allows keys to be set twice and the last wins so I think we can replace the last } with , sourceRoot: "..." } – ideally in a TransformStream
There was a problem hiding this comment.
Makes sense! 👍 One thing that might complicate this is the new sourceRoot will depend on the existing sourceRoot if it's defined, so we'd also need to "parse" part of the JSON in a streaming fashion. What we're doing here certainly isn't ideal, but it's not on the hot path, and will only be called lazily as DevTools requests source maps. 🤔
There was a problem hiding this comment.
Yeah it should be possible to extract a top-level prop from a stream but that could complicate the transform. I'll leave the decision to you whether we optimise it now or follow up with it later
There was a problem hiding this comment.
I think I'm going to leave this as is for now. With the sourcesContent field in source maps containing arbitrary user code, we would effectively need a full streaming JSON parser to do this. https://www.npmjs.com/package/stream-json and https://www.npmjs.com/package/@streamparser/json look like good options, but I suspect this might be less efficient in the case of small-ish source files/maps. JSON.parse() is highly optimised. One other thing to consider is the limited size of Workers in general, currently up to 10MB compressed for paid accounts. This means the source maps we get are unlikely to be massive. If this becomes a problem though, we should definitely revisit this. 👍
Hey! 👋 With cloudflare/workerd#710,
workerdsupports breakpoint debugging! Support for this in Miniflare just worked, assuming you were using a plain JavaScript worker, or you had inline source maps.workerddoesn't know where workers are located on disk, it just knows files' locations relative to each other. This means it's unable to resolve locations of corresponding linked.mapfiles insourceMappingURLcomments.Miniflare does have this information though. This PR detects linked source maps and rewrites
sourceMappingURLcomments tohttpURLs pointing to Miniflare's loopback server. This then looks for the source map relative to the known on-disk source location. Source maps'sourceRootattributes are updated to ensure correct locations are displayed in DevTools.This enables breakpoint debugging for compiled TypeScript with linked source maps! 🎉
Closes DEVX-872