remap-istanbul icon indicating copy to clipboard operation
remap-istanbul copied to clipboard

Provide only a sourcemaps v3 for processing

Open m-a-r-c-e-l-i-n-o opened this issue 8 years ago • 5 comments

In my use case, using jspm, I have to create two additional files (the transpiled file and source map) just to have remap() find them. In other words, I have a physical source file, which then gets transpiled and a source map is generated, but for the last two (transpiled and source map file ), I have already access to them in memory and they don't need to be a physical file for any other reason. Since sourcemaps v3 supports the sourcesContent property, it would be ideal if I could just pass in an array of v3 source maps to remap() (along with the coverage object), and have remap() granted immediate access to the source map and the source's content (via the sourcesContent property), without them having to be physically on the system. If this sounds reasonable to you guys, I would be happy to help with that effort.

m-a-r-c-e-l-i-n-o avatar Apr 16 '16 13:04 m-a-r-c-e-l-i-n-o

Thanks, but remap-istanbul already supports this (since early December). There were a couple issues in further passing that down the path in the coverage and to the reports, but I think the last of those challenges have been resolved with the release 0.6.3. Are you having issues with release 0.6.3?

kitsonk avatar Apr 16 '16 15:04 kitsonk

@kitsonk I appreciate the quick response. I understand what you're saying and it's not what I'm proposing. At the moment, at least in my usage of remap-istanbul, the sourcesContent property is not usable, and it does indeed cause issues, but for the sake of staying on topic, I won't get into that (I can share my findings later, if you would like). Right now, remap() gets pretty much all of its information from the coverage object ( remap(_covaregaObject_) ), this forces me to have a set of physical files on the system for when it decides to fetch the tranpiled files and their source maps. I'm suggesting this could be avoided if the raw v3SourceMaps were initially passed in like so remap(_covaregaObject_, options, <array>v3SourceMaps) and said v3SourceMaps contained a sourcesContent property with the source file's content. The remap(_covaregaObject_, options, <array>v3SourceMaps) function would then have all the information it needs, and wouldn't have to fetch any additional files in the file system. Let me know if you need me to elaborate a little further. Thanks!

m-a-r-c-e-l-i-n-o avatar Apr 16 '16 16:04 m-a-r-c-e-l-i-n-o

Could you provide an example source map that contains the sources like you suggest?

Typically sourcesContent is associated with an entity that provides locations to that file from some generated file. The covered files are only "needed" to find the source map, as the coverage.json provides the locations to look up in the source map. Currently remap-istanbul supports the code property in the coverage.json to provide the covered files, assumed to be with inline source maps (and potentially sources).

I think what you are suggesting is providing the array of source maps externally, so that remap doesn't have to resolve them (versus actually supporting sourcesContent).

kitsonk avatar Apr 17 '16 07:04 kitsonk

@kitsonk Sorry for the delay. This is no longer a requirement of ours, as keeping everything in memory is not ideal. And yes, I was indeed suggesting what you said:

... I think what you are suggesting is providing the array of source maps externally, so that remap doesn't have to resolve them ...

This feature might still be useful for us in the future, but I'll close this issue, for now :) And, here's the sample SourceMap v3 containing all the necessary information.

{
    version: 3, 
    sources: ['/path/to/file.js'], 
    names: [], 
    mappings: ';;;uBAAe,YAAY;AACvB,YAAI,IAAI,EAAJ,CADmB;AAEvB,YAAK,KAAL,EAAa;AACT,gBAAI,CAAC,CAAD,CADK;SAAb;AAGA,eAAO,CAAP,CALuB;KAAZ', 
    file: '/path/to/transpiled-file.js', 
    sourcesContent: [
        'export default function () {' +
            'var x = 42' +
            'if ( false ) {' +
                'x = -1' +
            '}' +
            'return x' +
        '}' +
    ]
}

m-a-r-c-e-l-i-n-o avatar Apr 28 '16 18:04 m-a-r-c-e-l-i-n-o

I am going to re-open, because it is an enhancement worth considering (even if it is no longer needed by @m-a-r-c-e-l-i-n-o)

kitsonk avatar May 02 '16 08:05 kitsonk