Skip to content
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

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

Open
nicolo-ribaudo opened this issue Jan 18, 2024 · 8 comments
Open

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

nicolo-ribaudo opened this issue Jan 18, 2024 · 8 comments
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed

Comments

@nicolo-ribaudo
Copy link

nicolo-ribaudo commented Jan 18, 2024

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.

@nicolo-ribaudo nicolo-ribaudo changed the title Allow a newline character be allowed after //# sourceMappingURL=... Allow a newline character after //# sourceMappingURL=... Jan 18, 2024
@nicolo-ribaudo nicolo-ribaudo changed the title Allow a newline character after //# sourceMappingURL=... Allow empty newlines after //# sourceMappingURL=... Jan 18, 2024
@lucacasonato lucacasonato added the bug Something isn't working correctly label Jan 18, 2024
@bartlomieju
Copy link
Member

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
Copy link
Member

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

@nicolo-ribaudo
Copy link
Author

Apparently this is not just about new lines. There are real-world source maps that also have other comments after //# sourceMappingURL: tc39/source-map#64 (comment)

@lucacasonato
Copy link
Member

lucacasonato commented Jan 19, 2024

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

@bartlomieju
Copy link
Member

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.

@lucacasonato lucacasonato added the deno_core Changes in "deno_core" crate are needed label Jun 18, 2024
@bartlomieju
Copy link
Member

bartlomieju commented Jul 4, 2024

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
Copy link
Author

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

@bartlomieju
Copy link
Member

Waiting on denoland/rusty_v8#1514 to be available in deno_core to start working on a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly deno_core Changes in "deno_core" crate are needed
Projects
None yet
Development

No branches or pull requests

4 participants