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

fix: use inline source maps when present in js #8995

Merged
merged 10 commits into from
Jan 5, 2021

Conversation

lucacasonato
Copy link
Member

@lucacasonato lucacasonato commented Jan 5, 2021

This PR also fixes source_line mapping, because currently it was making some wrong (but up to now correct) assumptions about where to get the source map info from. This has been corrected. The order of priority for source line info is now 1. the inline source map sourcesContent, 2. the module graph, and 3. the original source_line form the transpiled code.

Towards #4499.

Copy link
Contributor

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good... kind of sorta what I had in mind that I was going to start until you tortured me with Streams. It should be easy to continue to iterate on it.

Still some test errors, so holding off approval.

Copy link
Contributor

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though one stray debug statement

cli/source_maps.rs Outdated Show resolved Hide resolved
@lucacasonato lucacasonato merged commit 39bbbbc into denoland:master Jan 5, 2021
@lucacasonato lucacasonato deleted the use_inline_js_source_maps branch January 5, 2021 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants