Add getCombinedSourceMap in transform plugin context (#2983)

[billowz]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

Resolves #2983

Description

• Add this.getCombinedSourceMap() => SourceMap in transform plugin context

  When the sourcemap chain is not not empty, replace the original source map with the combined source source map and clear the sourcemap chain

• Refactor collapseSourcemaps

[lukastaegert]

Awesome for looking into this! Did you already try if this works to generate proper instrumentation for coverage? I will probably not have time today for actually running your code but I hope we can merge this soon. Just some stylistic remarks you may or may not address.

[billowz]

Awesome for looking into this! Did you already try if this works to generate proper instrumentation for coverage? I will probably not have time today for actually running your code but I hope we can merge this soon. Just some stylistic remarks you may or may not address.

I have tested it with karma-cover-reporter, and the generated coverage report is correct

[billowz]

Awesome for looking into this! Did you already try if this works to generate proper instrumentation for coverage? I will probably not have time today for actually running your code but I hope we can merge this soon. Just some stylistic remarks you may or may not address.

Here is the test configuration:

• rollup-plugin-istanbul.js

  import { createFilter } from 'rollup-pluginutils';
  import istanbul from 'istanbul-lib-instrument';
  
  export default function (options = {}) {
    const instrumenter = new istanbul.createInstrumenter({
      produceSourceMap: sourceMap,
      esModules: true
    });
    return {
      name: 'istanbul',
      transform (code, id) {
        // getCombinedSourceMap: https://github.com/rollup/rollup/issues/2983
        const { version, sources, sourcesContent, names, mappings } = this.getCombinedSourceMap()
        code = instrumenter.instrumentSync(code, id, { version, sources, sourcesContent, names, mappings })
  
        return { code, map: instrumenter.lastSourceMap() };
      }
    };
  }

• karma-config.js

  const jscc = require('rollup-plugin-jscc'),
    typescript = require('rollup-plugin-typescript'),
    istanbul = require('./rollup-plugin-istanbul')
    
    
  module.exports = function(config) {
    config.set({
      browsers: ['Chrome'],
      transports: ['websocket', 'polling', 'jsonp-polling'],
      frameworks: ['mocha'],
      reporters: ['coverage-istanbul'],
      basePath: path.join(__dirname, '../'),
      files: ['test/**/*.ts'],
      preprocessors: {
        'test/**/*.ts': ['rollup']
      },
      rollupPreprocessor: {
        options: {
          input: 'src/index.ts',
          output: {
            format: 'iife',
            name: 'test'
          },
          plugins: [
            jscc({ values: { _DEBUG: true } }),
            typescript({ module: 'ESNext' }),
            istanbul()
          ]
        },
        transformPath(filepath) {
          return filepath.replace(/\.ts$/, '.js')
        }
      },
      coverageIstanbulReporter: {
        dir: 'coverage/%browser%',
        reports: ['html', 'lcov'],
        combineBrowserReports: false,
        skipFilesWithNoCoverage: false
      },
      plugins: [
        'karma-chrome-launcher',
        'karma-mocha',
        'karma-rollup',
        'karma-coverage-istanbul-reporter'
      ]
    })
  }

[lukastaegert]

Finally managed to get it to run with Mocha and nyc (pro tip: By default, nyc does not like "ts" extensions...). Works really well!

One last thing I noticed: The spelling of "SourceMap" in the context function differs from our spelling of "Sourcemap" in all options so I would like to change the name of the function to "getCombinedSourcemap".

[billowz]

Finally managed to get it to run with Mocha and nyc (pro tip: By default, nyc does not like "ts" extensions...). Works really well!

One last thing I noticed: The spelling of "SourceMap" in the context function differs from our spelling of "Sourcemap" in all options so I would like to change the name of the function to "getCombinedSourcemap".

I will fix it asap

[lukastaegert]

👍