[Request] add source map support

[otbe]

Hi,

would it be possible to add source map support? Played around for a while and got a working version. Don't know if it fits every edge case.

https://github.com/otbe/karma-electron/tree/source-map-support

Thanks.

[twolfson]

It looks reasonable. There are a few minor one-offs like ~ vs ^ and consolidating jscs:disable but overall looks good. Can you give us more background on the edge case this covers?

[otbe]

In general this is no edge case. In every case the generated sourcemaps annotation (independent of type) will be placed alongside the content within your wrapper in node-integration-iframe.mustache.js. In my opinion there are two problems with that behavior:

• a missing line break at the end of the content leads to unexpected end of input because the last line will be:

//# sourceMappingURL... }());

• sourcemaps annotation should be at the end of the source file otherwise chrome dev tools won't recognize it (my observation)

[twolfson]

I think we talked past each other. The current implementation (i.e. no source maps) should only affect errors generated on line 1 (e.g. there will be significant column shifting in stack traces) but all lines after that should be fine. What does adding source map support accomplish?

[otbe]

Sorry misunderstood you :)

Without adding source map support in this way, I cant get karma-electron working at all. Because I run into unexpected end of input all the time.

My setup:

• TS test and source files
• karma-webpack for preprocessing this files (TS -(ts-loader)> ES6 -(babel-loader)> ES3)

This will generate the following typical content:

/******/ (function(modules) { // webpackBootstrap
/******/    // The module cache
/******/    var installedModules = {};
... content...
/***/ }
/******/ ]);
//# sourceMappingURL=inlined|external sourcemap

And within your wrapper it looks like this (note the last line):

...
  __parentWindow = undefined;

/******/ (function(modules) { // webpackBootstrap
/******/    // The module cache
/******/    var installedModules = {};
... content...
/***/ }
/******/ ]);
//# sourceMappingURL=inlined|external sourcemap }());

I tried to add a line break before }());, but like I said before it seems to me that chrome dev tools wont pick up the source maps if the annotation is not at the end of the file.

[twolfson]

Ahh, that makes sense now. Nice catch =) Do you know if preserving the sourceMappingURL comment actually corrects the stack trace output by Karma (e.g. a failing assertion will raise on the TS line/column not the JS line/column)?

Note to self: There are 2 edge cases to handle (1) preserving sourceMappingURL as last line, (2) allowing any // comment on the last line of an input file

[otbe]

With https://github.com/otbe/karma-electron/tree/source-map-support the output looks like this:

from this TS test file (Helper.ts):

(if I get you correct :))

[twolfson]

Ah, nice. It appears to be using the source map. As a sanity check, I transcribed the content and if there would be no source map support, then it would be more like line 23:

import expect from 'expect';

function sleep() {
    return new Promise(resolve => setTimeout(resolve, 1000));
}

describe('Helper', () => {
    describe('normalizePort', () => {
        it('test', async () => {
            await sleep();
            // await sleep();
            // throw new Error('Test')
            expect(2).toBe(3);
        });
    });
});

Compiled via https://www.typescriptlang.org/play/ :

var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
    return new (P || (P = Promise))(function (resolve, reject) {
        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
        function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
        function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
        step((generator = generator.apply(thisArg, _arguments)).next());
    });
};
define(["require", "exports", 'expect'], function (require, exports, expect_1) {
    "use strict";
    function sleep() {
        return new Promise(function (resolve) { return setTimeout(resolve, 1000); });
    }
    describe('Helper', function () {
        describe('normalizePort', function () {
            it('test', function () __awaiter(this, void 0, void 0, function* () {
                yield sleep();
                // await sleep();
                // throw new Error('Test')
                expect_1.default(2).toBe(3);
            }));
        });
    });
});

[otbe]

In my use case it would be line 10000 or so :) babel-polyfill and some other modules gets bundled too.

[twolfson]

Can you open a PR with your work so far? As a pre-emptive request, here are some notes:

• Adjust semver from ^ to ~
• Consolidate the jscs and jshint inline comments with the adjacent ones

Optional items we will need to take care of:

• Add a test for preserving source map (might be too hard to test)
  • We could have one that verifies we can load source map JS but that's superceded by the following bullets
• Move from bespoke minification to something like JSMin or UglifyJS so we can add double source map support
  • Or maybe remove minification altogether but get double source mapping working
• Add support for // comments
• Add test for handling // comments at end of files

[otbe]

Will prepare a PR this afternoon, but Im not sure how to merge jshint comments properly. jshint works scoped by block, don't it?

[twolfson]

I think they work by lines, not by blocks. I guess try consolidating them, running npm run lint, and if that fails, then leave them as is.

Sounds good, I'm prob logging off soon but I will give you an ETA when I receive the PR

[twolfson]

I should also mention that this package currently doesn't fully support submodules (i.e. require('./abc') won't see the samewindow). We have repaired it in https://github.com/twolfson/karma-electron/tree/dev/fix.nested.context but that requires a patch fromkarma` which is being reviewed:

https://github.com/karma-runner/karma/pull/1984

When the PR is landed, I will update that dev/fix.nested.context branch as well.

[otbe]

That would explain why async/await fails sometime with _regeneratorRuntime is not defined. Therefore I have to bundle all deps instead of requiring them properly.

[twolfson]

This has been resolved via @otbe in #4 and released in 3.1.0.

[otbe]

Works very well :) Thank you!