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

Remap diagnostics and other source locations using input source map #6288

Merged
merged 4 commits into from
May 16, 2021

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented May 15, 2021

Based on #6286 and #6238.

Fixes T-1063

This attempts to remap source locations used in diagnostics, dependencies, and symbols using the input source map. This could come from Babel or another transformer that runs before SWC, or an original source map e.g. a library with a source map. This second one is a bit questionable potentially?

Question: should this be done in core so it applies to all transformers?

@height
Copy link

height bot commented May 15, 2021

This pull request has been linked to and will mark 1 task as "Done" when merged:

💡Tip: You can link multiple Height tasks to a pull request.

@parcel-benchmark
Copy link

parcel-benchmark commented May 15, 2021

Benchmark Results

Kitchen Sink 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

React HackerNews 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Click here to view a detailed benchmark overview.

@mischnic
Copy link
Member

mischnic commented May 16, 2021

One variant to move this into core:

  1. call the translation in core in asset.add(URL)Dependency and dep.symbols.set (these already have access to the asset)
  2. for ThrowableDiagnostic, there'll have to be a util of some kind (either like convertLoc(asset, loc) or asset.throwDiagnostic(diagnostic)) because it doesn't know about the asset

@DeMoorJasper DeMoorJasper merged commit 4dcbbdc into sourcemap-cleanup May 16, 2021
@DeMoorJasper DeMoorJasper deleted the loc-sourcemap branch May 16, 2021 19:01
DeMoorJasper pushed a commit that referenced this pull request May 16, 2021
* Simplify source content handling

* Copy over source content from original map in babel transformer

* Don't store a source map in the graph

* Remap diagnostics and other source locations using input source map (#6288)

* Remap diagnostics and other source locations using input source map

* Fix flow

* Ignore test in flow config

* Move to util, apply in CSS transformer as well
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.

None yet

4 participants