Respect/Merge Input Source Maps With Generated Source Map Data

[RyanThomas73]

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
The transform pipeline does not respect/load/merge existing input source maps with generated source map data.

What is the expected behavior?
If an input module has an existing source map file, especially if the input module includes a source map url for the existing source map file, metro should load that input source map and merge that mapping with the output mapping generated by metro.

Related:
#10
facebook/react-native#12705

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

• Operating System: Windows 10
• Node Version: 9.2.0
• Yarn Version: 1.2.1
• Metro Version(s): Latest

Details
The transform pipeline does not load existing input source maps, and thus does not merge them with the generated source map data.

• If the input module code contains a //# sourceMappingURL= reference to the existing input source map, then the existing input source map file contents should be loaded early in the process, possibly in the Module class where it can be cached with the other module data and should flow through the transform pipeline.

• If an input source map object is present it should be merged with/transform the output source map.
  The transformer should pass the input source map to the babel.transform(..) call via the babelConfig.inputSourceMap property. The post transform logic should merge the input source map with the generated one.

For Reference.
https://github.com/babel/babel/blob/7d37017a5f85260c0c95580731585916e791265c/packages/babel-core/src/transformation/file/index.js#L290

I played around with modified version of transform(..) and postTransform(..) methods. It worked for my very basic proof of concept. I was hoping one of the maintainers familiar with the source map optimizations and recent overhaul of the transform pipeline (e.g. @davidaurelio ) could weight in on what else would be needed for an acceptable PR.

Something like this for example (Module.js):

  _readInputSourceMap(): ?string {
    if(!this._hasCheckedForInputSourceMap) {
      this._hasCheckedForInputSourceMap = true;
      this._inputSourceMapUrl = this._extractInputSourceMapUrl(this._readSourceCode());
      if(this._inputSourceMapUrl != null) {
        // perhaps any manipulations / raw mapping optimizations here ?
        this._inputSourceMap = this._optimizeInputSourceMap(fs.readFileSync(this._inputSourceMapUrl, 'utf8'));
      }
      // if there is not a source mapping url should we look for a matching filename with the `.map` suffix ?
      // if we're passing an input source map through, should we parse any inline source map from the code, at this point as well ?
    }
    return this._inputSourceMap;
  }

  _extractInputSourceMapUrl(sourceCode: string): ?string {
    // some optimized version of this logic perhaps:
    const urlIndex = sourceCode.indexOf('//# sourceMappingURL=');
    if(urlIndex ===  -1) {
        return null;
    }
    var sourceMapUrl = sourceCode.substring(urlIndex + 21);
    if(sourceMapUrl.indexOf('\n') > -1) {
        sourceMapUrl = sourceMapUrl.substring(0, sourceMapUrl.indexOf('\n'));
    }
    return sourceMapUrl;
  }

Pass the input source map data through...

  async transformFile(
    filename: string,
    localPath: LocalPath,
    code: string,
    inputSourceMap: SourceMap
    ....

For my proof of concept with the raw mappings I was doing this:
src/transformer.js

   function buildBabelConfig(filename, options, inputSourceMap?: SourceMap, plugins?: BabelPlugins = []){
    ...
    if(!!inputSourceMap) {
      config.inputSourceMap = inputSourceMap;
    }
    ...
  }

   function transform(filename, options, src, inputSourceMap, plugins): Params) {
        ...
        const babelConfig = buildBabelConfig(filename, options, inputSourceMap, plugins);
        const {ast,map} = babel.transform(src, babelConfig);

        return {ast,map};
        ...
   }

src/JSTransformer/worker/index.js

    function transformCode(...) {

        ....
        return transformResult instanceof Promise
           ? transformResult.then(({ast,map})=> postTransform(...postTransformARgs, ast, map))
            : postTransform(...postTransformArgs, transformResult.ast, transformResult.map);
    }

    function postTransform(...) {
        ....
        var map;
       if(result.rawMappings && inputSourceMap) {
            map = toRawMappings(mergeSourceMaps(inputSourceMap, result.map)).map(compactMapping);
        } else if(result.rawMappings) {
            map = result.rawMappings.map(compactMapping);    
        } else if(inputSourceMap) {
             map = toRawMappings(inputSourceMap).map(compactMapping);
         } else {
             map = [];
         }
         
        return {
            result: {dependencies, code: result.code, map},
            transformFileStartLogEntry,
            transformFileEndLogEntry,
        };
    }

[RyanThomas73]

Hoping this could help serve as a starting point.

https://github.com/facebook/metro/compare/master...RhinobyteSoftware:ryant/master/input-source-map-support?expand=1

Could use some direction on desired way to pass the input source map through the pipeline, optimizations, etc.

[ScottPierce]

I'm also experiencing this issue. This is listed as a feature. I've seen people online talk about how this used to work, but is no longer working. Does anyone know of a way to work around this for now?

[davidaurelio]

@ScottPierce what powered the feature in the past? Metro never had code for this.

@RyanThomas73 Last time I checked that code in babel it did not work for all cases, but I can’t recall the details.

In any case, supporting that would add considerable performance overhead, as we would have to parse all mappings in input source mappings into our internal format. I am not sure this should be the default.

Are there any example packages you can point us at?

[ScottPierce]

@davidaurelio I've been reading that after a specific version it stopped working. Perhaps I'm wrong?

Here is a package that does this and is kept up-to-date specifically for typescript: https://github.com/ds300/react-native-typescript-transformer

Here is a package that is no longer up-to-date, but did this for all js files with .map files.
https://github.com/mhzed/react-native-sm-transformer

[RyanThomas73]

@davidaurelio The performance overhead is definitely a concern that will have to be figured out; I don't think you would want to parse them into the compact format used by metro. The compact format does not support tracking the original source file, unless you're going to update that part to include it as an additional element in the 'tuple'.

Metro's structure has changed somewhat since I first entered this ticket; with metro-source-map now being a separate package. From what I know of the current metro pipeline now there are two ways this could be achieved:

• Loading the input source maps at some point so they're available in the addMappingsForFile(..) call. Update the generator logic so it is possible to merge/translate the compact mapping and the input source map while generating the output source map.
  https://github.com/facebook/metro/blob/master/packages/metro-source-map/src/source-map.js

• ALTERNATIVELY, leave the generator logic untouched. Translate the completed output source map using the input source map as a last step.

Since the full input source map requires considerable overhead it would be ideal to delay reading/parsing it until as close to the merge site as possible but, to my knowledge, these later stage call sites in the pipeline aren't async and I think they're outside of the transformer workers.

@ScottPierce As David mentioned metro has never had code for applying input source maps. I'm guessing the issues you reading were related to the creation of output source maps generated by metro not working.

[fbartho]

Damn, I'm running into this as well. I'd love to hear if there are any updates about this.

My situation:

1. Using tsc to compile typescript (we are using features not supported under babel-typescript),
2. Even though sourcemaps are included in the dist/ directory that is passed to react-native's preprocessor that calls metro, babel-jest is emitting code-coverage with the dist/-directory paths.

[thheller]

I'm working on shadow-cljs which is a build tool for ClojureScript and I'm adding proper support for react-native. This is all working fine already with the exception of source maps. If we want proper source maps we currently have to bypass metro completely and instead load the code dynamically at runtime. Even then it only works when running in Chrome. All we need really is for metro to read the input source maps we generated when translating CLJS->JS, as others have suggested in the past.

Any plans to make this a possibility?

[aleclarson]

Bump. This deserves more attention from the Metro team.

What does Facebook do internally for this?

[raejin]

I don't think Metro can easily handle this if babel/generator does not support merging inputSourceMap by default. This was submitted as an issue

Before babel/generator makes this easier for us, I think we can perhaps pursuit a workaround by leveraging this source map merging function in babel: https://github.com/babel/babel/blob/fced5cea430cc00e916876b663a8d2a84a5dad1f/packages/babel-core/src/transformation/file/merge-map.js

Note that the above function expects two JSON source map form, however Metro uses the rawMappings constructed by babel. So we would have to fiddle a bit with the merging logic to work with rawMappings:

metro/packages/metro/src/JSTransformer/worker.js

Line 305 in 69afb2a

let map = result.rawMappings ? result.rawMappings.map(toSegmentTuple) : [];

[fbartho]

That sounds like a great workaround @raejin -- what would it take to integrate that into Metro, since Babel doesn't seem super interested in this fix/feature?

It feels like beyond my level of understanding of either tool, but I'd be happy to beta metro to see if it works for my project?

[fbartho]

@raejin I hesitate to resurrect a potentially dead ticket, but we’re finding even more reasons why we’d love this feature to exist. Is there any hope?

[fbartho]

I noticed in this Pull Request (facebook/react-native#25613) there was a nifty new file: https://github.com/facebook/react-native/blob/master/scripts/compose-source-maps.js

I'm going to try to mess with this script and see if I can put anything together.

Ideally, metro would automatically consume source maps and produce a merged output file internally, however.

[raejin]

@fbartho We pursued using babel to transform our TypeScript files completely so this isn't an issue for us anymore. Sorry about that! I hope this carries out soon.