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

Include sourcesContent in generated source map #85

Closed
bengourley opened this issue Aug 14, 2019 · 7 comments
Closed

Include sourcesContent in generated source map #85

bengourley opened this issue Aug 14, 2019 · 7 comments
Labels
question Further information is requested

Comments

@bengourley
Copy link

When react-native's normal bundle command is executed with --sourcemap-output, e.g:

$ react-native bundle \
    --platform android \
    --dev false \
    --entry-file index.js \
    --bundle-output android-release.bundle \
    --sourcemap-output android-release.bundle.map

… the sourcesContent field of the source map is populated.

The default behaviour in Hermes is to not include the sourcesContent field in the output source map, and I can't see a way to enable it.

Having it on by default, or at least being able to include sources with an option, is a really useful feature that we (at Bugsnag) and our customers rely on. Can you let me know if there are plans to add this, or any objections to adding it?

Thanks!

@motiz88
Copy link
Contributor

motiz88 commented Aug 16, 2019

Hi @bengourley! I think what you're describing is that the React Native Hermes integration doesn't preserve the sourcesContent field when it composes source maps from Metro and Hermes. I'll open a corresponding issue on the React Native repo for further discussion.

@dulinriley
Copy link
Contributor

Let's have the discussion on that other issue

@xljones
Copy link

xljones commented Jun 3, 2020

Looks like facebook/react-native#26086 was marked resolved, but assuming this was just an automatic closure due to the issue going stale. Has there been any progress on this?

@tmikov
Copy link
Contributor

tmikov commented Jun 3, 2020

Out of curiosity, what is the use case for this? Is it for debugging or for something else?

Hermes itself could populate the sourcesContent field, but it wouldn't be very useful because Hermes consumes code that has already been transformed.

Another option is for Hermes to copy the sourceContent field of the input source map it is given. That should be pretty easy to do.

Would any of these two options address the need?

@tmikov tmikov reopened this Jun 3, 2020
@bengourley
Copy link
Author

The latter would be the ideal solution – just preserving the orginal sourcesContent field, as @motiz88 mentioned.

The use case for us (there may be others) is for when users upload their source maps to Bugsnag. We can only show the original code to our users when it is included in the source map – without it we can show the orginal line/col but not what code was there.

@tmikov
Copy link
Contributor

tmikov commented Jun 4, 2020

Copying the sourceContent of the input source map, if present, should be easy.

However, right now in the build process I don't think Hermes is given an input source map at all. So something has to be done in the React Native integration anyway.

motiz88 added a commit to motiz88/metro that referenced this issue Jun 29, 2020
Summary:
* Implements the `sourceContentFor` API from `source-map` in Metro's `Consumer`.
* Copies source contents in `composeSourceMaps` if they exist.

Related issues: facebook/react-native#26086, facebook/hermes#85. We'll be able to close the RN issue once this lands in the version of `metro-source-map` used in RN master.

Differential Revision: D22284159

fbshipit-source-id: 996f485a95190319a6482da518dcf32a4e7a8a96
@motiz88
Copy link
Contributor

motiz88 commented Jun 29, 2020

facebook/metro#574 will fix this on the React Native side.

facebook-github-bot pushed a commit to facebook/metro that referenced this issue Jun 29, 2020
…#574)

Summary:
Pull Request resolved: #574

* Implements the `sourceContentFor` API from `source-map` in Metro's `Consumer`.
* Copies source contents in `composeSourceMaps` if they exist.

Related issues: facebook/react-native#26086, facebook/hermes#85. We'll be able to close the RN issue once this lands in the version of `metro-source-map` used in RN master.

Reviewed By: cpojer

Differential Revision: D22284159

fbshipit-source-id: ac3080ce772f664d7b58559f109b71a9252c325b
mganandraj pushed a commit to mganandraj/hermes that referenced this issue Jun 22, 2022
We don't currently have version information in the dll, which makes debugging and filtering crash dumps difficult.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants