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

feat: Support Hermes (react-native) SourceMaps #22

Merged
merged 13 commits into from
Feb 5, 2020
Merged

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Jan 30, 2020

fixes #21

This exposes a new SourceMapHermes type, which can look up the scope name via a bytecode offset.

@Swatinem Swatinem requested a review from mitsuhiko January 30, 2020 18:10
@mitsuhiko
Copy link
Member

Yeah. This is all a bit meh. I think one way we could go forward is instead of exposing the source map as the primary API to have view objects over them that provide the most common operations.

We already effectively do something like this in symbolic where a source map often only makes sense when paired with the source view of the minified code.

Might be worth looking into the firefox developer tools. They might have found better abstractions.

src/hermes.rs Outdated Show resolved Hide resolved

let mut nums = parse_vlq_segment(mapping).ok()?.into_iter();

column = (i64::from(column) + nums.next()?) as u32;
Copy link
Member

Choose a reason for hiding this comment

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

wonder if we want to deal with the overflow explicitly here.

Copy link
Member Author

Choose a reason for hiding this comment

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

hm, the regular sourcemap code doesn’t either.

src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/hermes.rs Outdated Show resolved Hide resolved
@Swatinem Swatinem requested a review from mitsuhiko February 4, 2020 17:12
@HazAT
Copy link
Member

HazAT commented Feb 5, 2020

Confirmed, just tested it end to end, it works!

@Swatinem Swatinem merged commit eb0521d into master Feb 5, 2020
@Swatinem Swatinem deleted the hermes-sourcemaps branch February 5, 2020 13:42
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.

Implement hermes source maps
4 participants