Code Monkey home page Code Monkey logo

Comments (15)

twolfson avatar twolfson commented on July 20, 2024

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?

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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)

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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?

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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.

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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

bildschirmfoto 2016-04-25 um 10 18 13

from this TS test file (Helper.ts):
bildschirmfoto 2016-04-25 um 10 18 04

(if I get you correct :))

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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);
            }));
        });
    });
});

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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:

karma-runner/karma#1984

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

from karma-electron.

otbe avatar otbe commented on July 20, 2024

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.

from karma-electron.

twolfson avatar twolfson commented on July 20, 2024

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

from karma-electron.

otbe avatar otbe commented on July 20, 2024

Works very well :)
Thank you!

from karma-electron.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.