Skip to content

Add sourceMapCallback option to enable advanced mapping use cases#69

Merged
ezolenko merged 2 commits into
ezolenko:masterfrom
lephyrus:master
Mar 28, 2018
Merged

Add sourceMapCallback option to enable advanced mapping use cases#69
ezolenko merged 2 commits into
ezolenko:masterfrom
lephyrus:master

Conversation

@lephyrus

Copy link
Copy Markdown
Contributor

Motivation
I am trying to do a Rollup build that transpiles my Typescript sources and specs for a Karma run, while also instrumenting the sources in a way that maps back to the original Typescript, without writing intermediate files to disk. I also want the final Rollup bundle to have correct source maps for in-browser debugging.

Problem
This plugin provides Rollup with the maps for the transformation from Typescript to Javascript, which correctly maps the resulting bundle to the original sources. When instrumenting the code with Istanbul as part of the process, the coverage info will not correctly apply to the original typescript, however. One approach is to use remap-istanbul (e.g. via karma-remap-coverage), but for that you need the source maps for the Typescript > Javascript transformation. These are not accessible fior a Rollup plugin, unless written to disk, which is not what this plugin does (for good reason). If I set inlineSourceMap to true in the Typescript compiler options, the Istanbul instrumenter correctly sees this as an "input sourcemap" and the coverage maps back to the Typescript sources perfectly. With such a configuration, rollup-plugin-typescript2 on the other hand does not "see" the sourcemap and does not pass it back to Rollup, which leads to incorrect mappings for the generated bunde. The other way to solve this is to "manually" supply the Istanbul instrumenter with an input source map. This is what I got working.

Solution
With the help of the modification proposed in this PR and a custom instrumenter plugin, I got the coverage report to match the Typescript sources while also having the correct source map for the bundle produces by Rollup. 🎉

// karma.conf.js
// ...
  config.set({
    // ...
    files: [
      // ...
      { pattern: 'specs.ts', watched: false }
    ],
    preprocessors: { 'specs.ts': ['rollup'] },
    reporters: ['coverage-istanbul'],
    mime: { 'text/x-typescript': ['ts','tsx'] },
};
// rollup.config.js
// ...
let typescriptSourceMaps = new Map();

module.exports = {
  plugins: [
    resolve(),
    typescript(
      tsconfigOverride: { include: 'specs.ts' },
      clean: true,
      sourceMapCallback: (id, map) => typescriptSourceMaps.set(id, map)
    }),
    commonjs(),
    instrument({
      include: 'src/**/*.ts',
      exclude: '**/*.spec.ts',
      getInputSourceMap: (id) => typescriptSourceMaps.get(id)
    }),
  ],
  output: {
    format: 'iife',
    sourcemap: 'inline'
  },
};
// rollup-plugin-instrument.js
import { createInstrumenter } from 'istanbul-lib-instrument';
import { createFilter } from 'rollup-pluginutils';

export default function instrument({ include, exclude, getInputSourceMap, debug } = {}) {
  const filter = createFilter(include, exclude);
  const instrumenter = createInstrumenter({
    esModules: true,
    produceSourceMap: true
  });
  getInputSourceMap = getInputSourceMap || (() => undefined);

  return {
    name: 'instrument',

    transform(code, id) {
      if (!filter(id)) return;

      let inputMap;
      try {
        inputMap = JSON.parse(getInputSourceMap(id));
      } catch (error) {
        inputMap = undefined;
      }

      const instrumentedCode = instrumenter.instrumentSync(code, id, inputMap);
      const outputMap = instrumenter.lastSourceMap();

      return { code: instrumentedCode, map: outputMap };
    }
  };
}

Notes

  • Sorry, it's a lot of explanation for a very small change. But man, this stuff was driving me crazy.
  • I think omitting the clean: true option leads to problems with this approach, not sure why.
  • Possibly related issue: Problems running with typescript artberri/rollup-plugin-istanbul#13,
  • By the way, I could not figure out how to limit all these changes to the dist files. I think they are caused by new versions of dependencies. I don't understand how package.lock.json is supposed to work. I really don't. 🤷‍♂️

@ezolenko

Copy link
Copy Markdown
Owner

Don't worry about changes in dist, I'll rebuild it anyway.

Looks good aside from breaking on cache (see comment inline)

Comment thread src/index.ts Outdated
@ezolenko ezolenko self-assigned this Mar 23, 2018
Note that the reason for passing a JSON string rather than an object
reference is that Rollup mutates the object, leading to unpredictable
behaviour.
@lephyrus

Copy link
Copy Markdown
Contributor Author

I still don't fully get how caching works, but it looks to me like the sourcemap JSON will still only be parsed once (unless a file changes) with the how it's done now.

@ezolenko ezolenko merged commit ffb2c0d into ezolenko:master Mar 28, 2018
@ezolenko

Copy link
Copy Markdown
Owner

Ok, I fixed some formatting and I think there was a bug in the way I was returning maps from cache, but rollup might have been covering for it.

@lephyrus Could you check if current version works for you both clean and from cache?

@lephyrus

Copy link
Copy Markdown
Contributor Author

@ezolenko Yep, seems to work perfectly either way. Thanks for incorporating this feature so quickly!

@lephyrus

Copy link
Copy Markdown
Contributor Author

@ezolenko Can I ask if you're planning to do a release soon?

@ezolenko

Copy link
Copy Markdown
Owner

Yep, most likely on the weekend sometime. I'm waiting on confirmation for one other issue, but that's not critical.

@agilgur5 agilgur5 changed the title Add 'sourceMapCallback' option to enable advanced mapping use cases Add sourceMapCallback option to enable advanced mapping use cases Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants