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

Release optimization pass doesn't respect input source maps? #10

Closed
ds300 opened this Issue Jun 14, 2017 · 2 comments

Comments

Projects
None yet
4 participants
@ds300

ds300 commented Jun 14, 2017

Hey peeps! Nice work splitting this off from react-native :)

It seems that, despite best intentions, input source maps get ignored when bundling for release. Here is the culprit: https://github.com/facebook/metro-bundler/blob/e87a8205d8ca070df0fcef67dbe4f02389770b95/packages/metro-bundler/src/JSTransformer/worker/index.js#L126

The transform steps done inside both of those optimization passes seem to ignore input source maps, regardless of whether they are passed inline or not. This may well be an issue with Babel rather than Metro. Indeed, it may even be desired behaviour in babel, but metro should probably be mapping between compiled and input code properly :)

The first place that the input source map gets left behind is during the inline operation. Notice that it transforms the input with the babel option source set to false - When you do that, babel never outputs a map property during the File#generate method because it ends up calling File#makeResult in such a way that it is impossible for a map to be emitted.

However, even if you patch inline to set code: true, the emitted map appears to be based only on the code passed to the transform step, completely ignoring the inputSourceMap property.

I've done a few hours trying to debug this but gotten nowhere. Maybe one of you can look at it?

I made a failing test over here: https://github.com/ds300/metro-bundler/blob/aadd5d8303ac99c8d5e5918ac752ca90b1c2fbdb/packages/metro-bundler/src/JSTransformer/worker/__tests__/source-mapping-test.js

Thanks ❤️

@tomauty

This comment has been minimized.

Show comment
Hide comment
@tomauty

tomauty Aug 23, 2017

Any chance this will get looked at?

tomauty commented Aug 23, 2017

Any chance this will get looked at?

@rafeca

This comment has been minimized.

Show comment
Hide comment
@rafeca

rafeca Dec 4, 2017

Member

Hey! We've completely rewritten the transforming pipeline, the new logic is available from v0.21.0 and should fix any sourcemap issue 😄

Member

rafeca commented Dec 4, 2017

Hey! We've completely rewritten the transforming pipeline, the new logic is available from v0.21.0 and should fix any sourcemap issue 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment