denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.82k stars 5.34k forks source link

Allow empty newlines after `//# sourceMappingURL=...` #21988

Open nicolo-ribaudo opened 9 months ago

nicolo-ribaudo commented 9 months ago

Test case:

"use strict";

throw new Error("Hello world!");
//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiaHR0cDovL2xvY2FsaG9zdDo0NTQ1L3J1bi9pbmxpbmVfanNfc291cmNlX21hcF8yLnRzIl0sInNvdXJjZXNDb250ZW50IjpbIjErMTtcbmludGVyZmFjZSBUZXN0IHtcbiAgaGVsbG86IHN0cmluZztcbn1cblxudGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIgYXMgdW5rbm93biBhcyBzdHJpbmcpO1xuIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7QUFBQSxDQUFDLEdBQUMsQ0FBQyxDQUFDO0FBS0osTUFBTSxJQUFJLEtBQUssQ0FBQyxjQUErQixDQUFDLENBQUMifQ==

If you add/remove one or more newlines at the end of the file you'll see the stack trace changing. It doesn't cause a difference in Node.js or in the various browsers.

bartlomieju commented 9 months ago

I'm having trouble reproducing it in Deno v1.39.4. I added several newlines at the end of the file (after the source map) and I always get:

error: Uncaught (in promise) Error: Hello world!
throw new Error("Hello world!");
      ^
    at file:///Users/ib/dev/deno/foo.js:3:7

Are there any other conditions that you've applied to reproduce it?

dsherret commented 9 months ago

I'm able to reproduce. I also remember the code doing what's described here somewhere.

nicolo-ribaudo commented 9 months ago

Apparently this is not just about new lines. There are real-world source maps that also have other comments after //# sourceMappingURL: https://github.com/tc39/source-map-rfc/issues/64#issuecomment-1900043100

lucacasonato commented 9 months ago

We should use https://v8.github.io/api/head/classv8_1_1UnboundModuleScript.html#a9c4f796120c4431c24edfced6e784cee instead of pulling out the source map url ourselves.

bartlomieju commented 9 months ago

We should use https://v8.github.io/api/head/classv8_1_1UnboundModuleScript.html#a9c4f796120c4431c24edfced6e784cee instead of pulling out the source map url ourselves.

This is a good idea, but requires significant effort to give access to that struct in necessary place. I think we should first adjust the code in deno_core to not panic in this situation and refactor it in a follow up.

bartlomieju commented 3 months ago

Adding empty lines after the magic comment now works, but this doesn't:


"use strict";

throw new Error("Hello world!");
//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiaHR0cDovL2xvY2FsaG9zdDo0NTQ1L3J1bi9pbmxpbmVfanNfc291cmNlX21hcF8yLnRzIl0sInNvdXJjZXNDb250ZW50IjpbIjErMTtcbmludGVyZmFjZSBUZXN0IHtcbiAgaGVsbG86IHN0cmluZztcbn1cblxudGhyb3cgbmV3IEVycm9yKFwiSGVsbG8gd29ybGQhXCIgYXMgdW5rbm93biBhcyBzdHJpbmcpO1xuIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7QUFBQSxDQUFDLEdBQUMsQ0FBQyxDQUFDO0FBS0osTUFBTSxJQUFJLEtBQUssQ0FBQyxjQUErQixDQUFDLENBQUMifQ==

asd;

Expected:

error: Uncaught (in promise) Error: Hello world!
    at http://localhost:4545/run/inline_js_source_map_2.ts:6:7

Actual:

error: Uncaught (in promise) Error: Hello world!
throw new Error("Hello world!");
      ^
    at file:///Users/ib/dev/deno/repro.js:3:7
nicolo-ribaudo commented 3 months ago

That's correct, because there is code after the comment. See https://tc39.es/source-map/#extraction-methods-for-javascript-sources.

bartlomieju commented 3 months ago

Waiting on https://github.com/denoland/rusty_v8/pull/1514 to be available in deno_core to start working on a fix.