Use Vue.js source-map technique for Svelte source code

[uglow]

This issue relates to sveltejs/svelte#286, and having done some detective work, I think there is a simple solution.

Current Behaviour

When using Webpack 2 + svelte + svelte-loader + babel, everything is fine. Here's the relevant Webpack config that I'm using:

let htmlLoader = {
    test: /\.html?$/,
    use:['babel-loader', 'svelte-loader'],
    exclude: /index-template.html$/
  };
config.module.rules.push(htmlLoader);

However, when it comes to generating code coverage and instrumentation (using Karma and karma-coverage (which uses Istanbul)), I keep running into this error:

ERROR in ./src/modules/common/marketingPanel/marketingPanel.svlt
Module build failed: Error: /mydev/code/src/modules/common/marketingPanel/marketingPanel.svlt: don't know how to turn this value into a node
at Object.valueToNode (/mydev/code/node_modules/babel-types/lib/converters.js:337:9)
at Object.valueToNode (/mydev/code/node_modules/babel-types/lib/converters.js:332:46)
at Object.exit (/mydev/code/node_modules/istanbul-lib-instrument/dist/visitor.js:520:34)
at PluginPass.exit (/mydev/code/node_modules/babel-plugin-istanbul/lib/index.js:73:23)
at newFn (/mydev/code/node_modules/babel-traverse/lib/visitors.js:276:21)
at NodePath._call (/mydev/code/node_modules/babel-traverse/lib/path/context.js:76:18)
at NodePath.call (/mydev/code/node_modules/babel-traverse/lib/path/context.js:48:17)
at NodePath.visit (/mydev/code/node_modules/babel-traverse/lib/path/context.js:117:8)
at TraversalContext.visitQueue (/mydev/code/node_modules/babel-traverse/lib/context.js:150:16)
at TraversalContext.visitSingle (/mydev/code/node_modules/babel-traverse/lib/context.js:108:19)
at TraversalContext.visit (/mydev/code/node_modules/babel-traverse/lib/context.js:192:19)
at Function.traverse.node (/mydev/code/node_modules/babel-traverse/lib/index.js:114:17)
at traverse (/mydev/code/node_modules/babel-traverse/lib/index.js:79:12)
at File.transform (/mydev/code/node_modules/babel-core/lib/transformation/file/index.js:558:35)
at /mydev/code/node_modules/babel-core/lib/transformation/pipeline.js:50:19
at File.wrap (/mydev/code/node_modules/babel-core/lib/transformation/file/index.js:574:16)
at Pipeline.transform (/mydev/code/node_modules/babel-core/lib/transformation/pipeline.js:47:17)
at transpile (/mydev/code/node_modules/babel-loader/lib/index.js:38:20)
at Object.module.exports (/mydev/code/node_modules/babel-loader/lib/index.js:133:12)
@ ./src/modules/common/marketingPanel/unitTest/marketingPanel.spec.js 3:0-52
@ ./src/modules unitTest\/.*spec\.(js|jsx)$
@ ./config/testUnit/test.files.js
webpack: Failed to compile.

---

The root cause seems to be here: https://github.com/sveltejs/svelte-loader/blob/master/index.js#L30

try {
    let { code, map } = compile(source, options);
    // "map" is not a "plain object" according to Lodash, as it has toString() and toUrl() methods on the prototype
    this.callback(null, code, map);
  } 

Babel(-plugin-istanbul) is parsing the map object, which it expects is a "plain object" (according to Lodash' definition). In this case, map has a 2 prototype methods: toString() and toUrl().

When I replace the above code with the following code, the error disappears:

try {
    let { code, map } = compile(source, options);

     // Remove the toString() and toUrl() methods, which should mean isPlainObject() returns true.
+    let newMap = {
+      code: code.split(/(?:\r?\n)|\r/),  // For the HTML writer
+      file: map.file,
+      mappings: map.mappings,
+      names: map.names,
+      sources: map.sources,
+      sourcesContent: code,   // Change this to the compiled code, not the original code, for the HTML Writer
+      version: map.version
+    };

+    this.callback(null, code, newMap);
-    this.callback(null, code, map);
  }

There are two additional changes which are necessary to support Istanbul with HTML reports:

1. The code property is an array of the lines of generated-by-svelte code. This property can be used within the karma.config.js file (see below) to cause Istanbul to correctly show code coverage for the generated code. Istanbul CANNOT use the original source code with HTML elements in it.
2. The sourcesContent property is changed to be the generated-by-svelte code. Again, this is because Istanbul is expecting JS as the source code, not a mixture of HTML and JS.

Combined with the following karma-coverage config in karma.config.js, we now get accurate source maps for the generated code!

// karma.config.js
...
reporters: ['progress', 'junit', 'coverage', 'threshold'],

  coverageReporter: {
    dir: 'reports/coverage/',
    reporters: [
      { type: 'cobertura', subdir: 'cobertura' },
      { type: 'lcovonly', subdir: 'lcov' },
      { type: 'html', subdir: 'html' },
      { type: 'json', subdir: 'json' },
      { type: 'text' }
    ],
    // A custom report collector for Svelte HTML components, so that source maps display semi-correctly
    _onWriteReport: function(collector) {
      // Iterate over the keys of the collector.
      // If the collector key is a HTML file, it should have an inputSourceMap with a code property
      var store = collector.store;
      var keys = store.keys().filter(function(key) {
        return /\.html$/.test(key);
      });

      keys.forEach(function(key) {
        let coverage = store.getObject(key);

        if (coverage.inputSourceMap && coverage.inputSourceMap.code) {
          // Replace the existing key with one that has the code property
          coverage.code = coverage.inputSourceMap.code;
          store.set(key, JSON.stringify(coverage))
        }
      });
      
      return collector;
    }
  },
...

[uglow]

Would you like a PR?

[Rich-Harris]

Thanks for raising this and digging into it. I'm a little confused by some parts of it — bear with me:

• I'd consider it a bug that babel-plugin-istanbul won't accept an object that has (non-enumerable) convenience methods, if it's a valid sourcemap. Maybe that needs to be raised elsewhere?
• I don't quite understand what's going on with the code property — that's not part of a sourcemap object. It seems very odd to add a non-sourcemap property to the sourcemap we give back to webpack just for the sake of karma — is that what other loaders do? Feels weird!
• sourcesContent should be the source, not the generated content. Again, this sounds like a bug elsewhere — changing sourcesContent to be the wrong thing seems like a really strange way to fix it!

Sorry if this sounds unhelpful — it's possible I've misunderstood something.

[uglow]

Hi @Rich-Harris ,

Yeah, I know this is a bit weird. There seems two be reasons why it doesn't work out of the box:

1. The bug in babel-plugin-istanbul that expects the plain object (as you pointed out). Perhaps this should be fixed there, but the reason for their constraint may be valid. In which case, the problem would remain. If the constraint isn't valid, they can change the code - great!

However, given that this issue hasn't surfaced in Angular 1 code, Angular "new" code or React code before in my experience, I would conclude that the problem seems related to the source map produce by Svelte. Additionally, there seemed to be no harm caused (in my test case) by removing those prototype functions. Perhaps this behaviour could be configured by a loader option?

2. The second problem is that when Istanbul is generating code coverage, it tries to cover JS nodes from an abstract syntax tree to determine whether (for example) the branches in an if-else condition have been executed. By design, it is expecting the source-code to be JS, so that it can produce (or be provided) an AST to process. Given that Svelte's source is really a HTML fragment, Istanbul crashes UNLESS you trick it into thinking that the Svelte-generated code is the sourcesContent.

The code property on the svelte-loader sourceMap is actually redundant as the Karma config could compute it from inputSourceMap.sourcesContent.split(/(?:\r?\n)|\r/). But it is necessary to add the coverage.code property in the Karma config so that I can tell Istanbul NOT to try and read the source code from the HTML file path, but to use the code array instead. Then Istanbul is dealing with a JS file as source code, and reporting coverage on the same JS code.

https://github.com/gotwarlost/istanbul/blob/master/lib/report/html.js#L410:

writeDetailPage: function (writer, node, fileCoverage) {
        var opts = this.opts,
            sourceStore = opts.sourceStore,
            templateData = opts.templateData,
// Next line is the key line ---->
            sourceText = fileCoverage.code && Array.isArray(fileCoverage.code) ?
                fileCoverage.code.join('\n') + '\n' : sourceStore.get(fileCoverage.path),
            code = sourceText.split(/(?:\r?\n)|\r/),
            count = 0,
            structured = code.map(function (str) { count += 1; return { line: count, covered: null, text: new InsertionText(str, true) }; }),
            context

Does that make more sense? If there is an easier way to get this working, I'd love to know! :)

[uglow]

@Rich-Harris would you like a PR which adds an option - something like "useGeneratedSourceForMap" - to the loader? I'm really keen to get this solved so that I can take this to the rest of my company and clients.

[Rich-Harris]

I just don't think the fixes are appropriate, given that they're so specific to this particular setup. The useGeneratedSourceForMap in particular sounds like absolutely the wrong solution. How does Istanbul deal with e.g. CoffeeScript? It must have the same problem, no?

The sourcemap is generated by magic-string — you can see the class here. In the case of this loader we could generate a new POJO with the same properties, but I'd be curious to see if Istanbul has a valid case against accepting these objects.

What about forking this project so that you can work around these issues, is that an option?

[uglow]

Ok, so I've done some further research on how languages like CoffeeScript and Vue.js are working:

CoffeeScript

Karma-coverage has CoffeeScript examples. These examples are using an instrumenter called ibrik to do the CoffeeScript source code coverage.

This line is using Istanbul's matcherFor() method to basically do their own instrumentation and coverage reporting for CoffeeScript files. In otherwords, they are bypassing Istanbul's instrumenter and using their own.

Vue.js

Vue.js appears to use a similar setup to what I'm trying to get working. I followed these instructions for setting up a basic Vue.js project. As I investigated further, I found some differences between Vue and Svelte:

• The vue-loader (which processes .vue files) does not need to be used with babel-loader. It appears to understand (some) ES6 syntax (like object-method syntax shortcut e.g. {data() { }, }) without the need for Babel. It also creates "parts" for the script, css and template(HTML) parts of a component, and sourcemaps for each part.
• When the sourcemap is returned for the "script" part, the "code" that is returned in the callback is actually padded with //\n for the HTML-lines that were in the source code.

https://github.com/vuejs/vue-loader/blob/master/lib/selector.js#L18 :

module.exports = function (content) {
  this.cacheable()
  var query = loaderUtils.getOptions(this) || {}
  var filename = path.basename(this.resourcePath)
  var parts = parse(content, filename, this.sourceMap)
  var part = parts[query.type]
  if (Array.isArray(part)) {
    part = part[query.index]
  }
  this.callback(null, part.content, part.map)
}

... where part.content is:

//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//
//

export default {
  name: 'hello',
  data () {
    return {
      msg: 'Welcome to Your Vue.js App'
    }
  }
}
"

This must preserve the mapping between the line numbers in the sourceMap and the source code.
The comment lines are where the HTML code used to be.

Does that give you more to work with, @Rich-Harris ?

[benmccann]

Svelte has added much better source map support recently though there's some work left to do in svelte-loader to make it match rollup-plugin-svelte. Closing this in favor of #171