feat: add source maps to libraries bundle #144

wawyed · 2017-06-30T14:56:45Z

It would be nice if the karma-typescript-bundle was mapped back to the original files. so when debugging in unit tests is easier to know where the errors come from.

erikbarke · 2017-07-01T05:24:57Z

Hey @wawyed, awesome idea! This shouldn't be too hard to implement, all the pieces are already there.

erikbarke · 2017-08-13T06:09:44Z

There's basic support for source maps for the bundled packages now:

karmaTypescriptConfig: {
    bundlerOptions: {
        sourceMap: true
    }
}

To try it out you can install from the dev branch, npm i github:monounity/karma-typescript#next ☺️

wawyed · 2017-08-13T08:09:51Z

You sir are on fire. I'll give it a try ^^

wawyed · 2017-08-14T10:41:51Z

I just tried it, it works but it doesn't seem to keep the sourcemap comment for the original js. So if it had a map to typescript it doesn't show the original typescript code.

erikbarke · 2017-08-16T20:51:23Z

Source maps for the original code are now loaded (only first level) and it works for packages like es6-promise, rxjs, zone.js, I can step through the original source code. Packages like @angular and diff however, do not work, their source maps seem to be broken beyond belief 😠

erikbarke · 2017-08-22T21:24:23Z

@wawyed, have you tested the last commit, can I close this?

wawyed · 2017-08-22T22:19:43Z

I'm trying to test it but Im getting this when trying to npm install the branch

npm ERR! code 128
npm ERR! Command failed: C:\Program Files\Git\cmd\git.EXE submodule update -q --init --recursive
npm ERR! fatal: No url found for submodule path 'examples/karma-ts-test-by-dts-file' in .gitmodules
npm ERR!

erikbarke · 2017-08-23T05:17:33Z

Oh great, I broke the git repo... I've cleaned up now and the build passes on the ci servers and I'm able to npm i the branch locally, does it work for you too now?

wawyed · 2017-08-23T09:05:39Z

Yes, I got it working, unfortunately even though is mapping from the bundle to the original TS the line that it goes to when I'm setting a breaking point doesn't match the correct one.

erikbarke · 2017-08-24T04:43:57Z

Which package isn't working when you debug, a specific one or all? I banged my head on my desk for days before realizing that some packages simply have incorrect source maps, I verified by loading a few packages directly in Chrome:

<!DOCTYPE html>
<html>
<body>
    <script src="node_modules/@angular/core/bundles/core.umd.js"></script>
    <script src="node_modules/diff/dist/diff.js"></script>
    <script src="node_modules/es6-promise/dist/es6-promise.js"></script>
    <script>
        console.log("try setting a break point");
    </script>
</body>
</html>

Put this html file in the root of, for instance, the angular2 example project and open the html file in Chrome. The angular file will crash and you might have to hit reload before the source maps load but it's still possible to set break points.

Now add import "es6-promise"; somewhere in the exampe project and run the test project and try setting break points.

This is the result I get with Chrome on OSX:

angular v4.3.4, break points are off in both the simple test and in karma-typescript, weird packages paths
diff v3.3.0, seems completely broken in both the simple test and in karma-typescript (src/convert/xml.js??)
es6-promise v4.1.1, works in both the simple test and in karma-typescript

The pattern I see is if it works when loading a source map directly in Chrome, it will work in karma-typescript as well.

It seems some vendors have a broken build chain where they transpile with source maps first and then they add umd wrappers without updating the source map, which throws the line numbers off.

wawyed · 2017-08-24T18:15:25Z

The package I used as a test is one of our private npm package and I can validate that the sourcemaps are correctly generated as they work fine when I run the app.

erikbarke · 2017-08-25T19:55:28Z

New approach: I've modified one of the typescript example projects to add the package karma-typescript-es6-transform (written in Typescript, transpiled with source maps) to the bundle.

Debugging this project works as expected, I can set a breakpoint in src/hello.component.ts and step through the code with correct line mappings.

Does it work if you install from the sourcemap branch, npm i github:monounity/karma-typescript#sourcemaps and debug the typescript-latest project or are the line numbers off? Is this an os/environment issue?

In your project, are the line numbers off by one or by a lot, or does the discrepancy grow as the line numbers grow?

erikbarke · 2017-08-31T07:13:54Z

@trwolfe13 @Salasar @patrickarlt, I would love some testing outside my environment for this, could you guys help me out please?

wawyed · 2017-09-05T15:43:15Z

Hi, sorry about the delay in the reply. I have tried the branch and I'm getting mixed results. Some ts files are getting remapped but others just show an empty file while trying to load the ts file.

erikbarke · 2017-09-05T18:59:13Z

Ok, you should get the same results for your project with the next and the sourcemaps branches, they both contain the same source map logic.

Does the typescript-latest example project on the sourcemaps branch work, are you able to set a breakpoint in src/hello.component.ts and step into the code for karma-typescript-es6-transform? Are the line mappings correct for the (one and only) Typescript file in the karma-typescript-es6-transform package?

wawyed · 2017-09-05T20:53:06Z

Yes, that seems to work correctly. One difference that I didn't mention, my compiled code uses inlineSources in the sourceMap so it doesn't point to a different file, contains the original code inside the sourcemap file.

erikbarke · 2017-09-06T04:58:53Z

Great, so it's not an os/environment issue then... Which leaves us with what? If you look at source-map.ts, there's nothing special going on in my code; file content is fed to combine-source-map which performs the actual magic.

I have yet to find a correct source map that doesn't work in karma-typescript, apart from your source maps, which I can't test since it's a private package.

I'm out of ideas here 😕

wawyed · 2017-09-07T08:30:55Z

I'm going to try to reproduce it in your example.

wawyed · 2017-09-11T10:01:09Z

Okay so I have created an example that you can reproduce :)

You can find it here:

https://github.com/wawyed/karma-typescript/tree/sourcemaps

Hope it helps!

If you look at MhrWindowService you will see that the original ts file is shown as an empty file.

erikbarke · 2017-09-12T17:32:52Z

Awesome!! Your source maps have sources that look like this:

[
    'webpack:///webpack/bootstrap a5a1de700b3a78264108',
    'webpack:///./src/services/index.ts',
    'webpack:///external "@angular/core"',
    'webpack:///./index.ts',
    'webpack:///./src/index.ts',
    'webpack:///./src/services/MhrWindow/MhrWindow.service.ts',
    'webpack:///./src/mhrCommon.module.ts',
    'webpack:///external "@angular/common"'
]

The webpack:///prefix apparently makes combine-source-map create non-working source maps, so I created a fix that simply removes the prefix, you can try it out on the nextbranch, npm i github:monounity/karma-typescript#next 🚀

wawyed · 2017-09-12T18:20:32Z

Hello, good news is that the files are not blank anymore :D. Bad news... the lines seem offset by a few lines.

wawyed · 2017-09-12T18:27:42Z

I found this tool to check sourcemaps visually. Maybe it can help you find out what's wrong:

http://sokra.github.io/source-map-visualization/

Seems to have a limitation on the number of lines it can show

sokra/source-map-visualization#21

so might not be that useful.

erikbarke · 2017-09-12T21:39:13Z

Darn.

I loaded @ mhrwebux/common/index.js and its source map from your tar ball into the visualization tool you linked and inspected row 127-129 of index.js:

function _window() {
    return window;
}

Row 128 (return window;) points back to row 4 in webpack:///src/services/MhrWindow/MhrWindow.service.ts [4->4:2] [10->4:8] [11->4:9] [17->4:15] [18->4:16].

I then launched the typescript-latest project from your fork, with the fix for empty files, and located the function _window() { part in the bundle (around line 22000 in my case). If I set a breakpoint on the return window; row in Chrome, the mapped file MhrWindow.service.ts opens with a breakpoint on row 4 which contains return window;.

Row 162 of index.js, var core_1 = __webpack_require__(1); points back to row 2 in webpack:///src/mhrCommon.module.ts [0->2:0]. If I locate that row in the bundle and set a breakpoint, the mapped file mhrCommon.module.ts opens with a breakpoint on row 2 which contains import { NgModule } from '@angular/core';.

This seems pretty accurate and consistent to me, could you give me an example of a file and a row in your fork where the source mapping is off please?

I suspect there is more webpack magic hidden in your source maps 😕

wawyed · 2017-09-12T21:53:14Z

If I try to put a breakpoint on the getter function it adds it to the class definition, have a look at this:

erikbarke · 2017-09-13T05:07:10Z

Aah, windows line breaks, the gift from the dot-matrix printers era that keeps on giving ☺️

If I hardcode \n as line breaks in the bundler instead of os.EOL (The Right Way™) the source maps seem to work correctly also on windows. This is clearly a CRLF issue, I'll see if I can come up something less hacky.

wawyed · 2017-09-13T08:12:41Z

I was going to say it might be something related to line endings since you didn't seem to have issues.

Could you do something like?

var nl = (process.platform === 'win32' ? '\r\n' : '\n')

erikbarke · 2017-09-13T18:34:44Z

This is exactly what os.EOL does, it returns the correct newline for each platform, ie \r\n on windows. What I did was using a hardcoded \n in the bundler regardless of platform, which made the source maps work correctly, go figure 😕

wawyed · 2017-09-13T20:14:48Z

hmm, from what I can see the sourcemap gets converted into a base64 string that gets read by chrome, so maybe chrome wants linux endings regardless?

erikbarke · 2017-09-14T04:58:04Z

I'm using the fix the hardcoded unix line breaks, it doesn't seem to affect anything else but the source maps, the fix is on the next branch, npm i github:monounity/karma-typescript#next!

wawyed · 2017-09-14T12:45:17Z

So, that fixes the issue :)... unfortunately, it seems that there is some issue with the sourcemaps of the src or test files. I don't seem to be able to set a breakpoint in some statements, specially if I have new lines in between.

The crossed lines are the ones I'm not able to add a breakpoint.

But if i remove the line lines..

Works fine.

Don't know if it's a chrome thing or what.

erikbarke · 2017-09-14T14:43:21Z

This must be a Chrome issue, I get the same behavior on windows with and without the latest line break fix. Also, the new source maps logic only affects the main bundle, the project ts files still use the inline source maps generated by Typescript.

Anyway, I managed to set breakpoints in Chrome on windows by setting them in the compiled file and the breakpoints end on the correct lines, so it's a Chrome issue I guess:

erikbarke · 2017-09-18T07:14:38Z

Ok, case closed then ☺️

wawyed · 2017-09-18T08:01:16Z

Releasing the new version soon? :)

erikbarke · 2017-09-18T19:05:36Z

3.0.6 is on npm now 🚀

erikbarke added the enhancement label Jul 1, 2017

erikbarke added this to the 3.0.5 milestone Jul 1, 2017

erikbarke modified the milestones: 3.0.6, 3.0.5 Aug 1, 2017

erikbarke added a commit that referenced this issue Aug 12, 2017

Major source map overhaul, #144

493060a

erikbarke added a commit that referenced this issue Aug 16, 2017

Load original source maps, #144

bf3196a

erikbarke added a commit that referenced this issue Aug 23, 2017

Removed bad commit, #144

7d04092

erikbarke added a commit that referenced this issue Aug 23, 2017

Removed another broken commit, #144

904a00c

erikbarke added a commit that referenced this issue Aug 25, 2017

Always include sourcesContent, #144

fde750c

erikbarke added a commit that referenced this issue Aug 25, 2017

Testing source maps with karma-typescript-es6-transform", #144

3793287

erikbarke added a commit that referenced this issue Sep 12, 2017

Clean up source map sources with webpack uri, #144

6082e9e

erikbarke added a commit that referenced this issue Sep 14, 2017

Source maps, hardcoded linebreaks, #144

6eae8c9

erikbarke closed this as completed Sep 18, 2017

snyk-bot mentioned this issue Mar 25, 2021

[Snyk] Fix for 1 vulnerabilities Bhanditz/karma-typescript#122

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add source maps to libraries bundle #144

feat: add source maps to libraries bundle #144

wawyed commented Jun 30, 2017

erikbarke commented Jul 1, 2017

erikbarke commented Aug 13, 2017

wawyed commented Aug 13, 2017 •

edited

Loading

wawyed commented Aug 14, 2017

erikbarke commented Aug 16, 2017

erikbarke commented Aug 22, 2017

wawyed commented Aug 22, 2017

erikbarke commented Aug 23, 2017

wawyed commented Aug 23, 2017 •

edited

Loading

erikbarke commented Aug 24, 2017

wawyed commented Aug 24, 2017

erikbarke commented Aug 25, 2017

erikbarke commented Aug 31, 2017

wawyed commented Sep 5, 2017

erikbarke commented Sep 5, 2017

wawyed commented Sep 5, 2017

erikbarke commented Sep 6, 2017

wawyed commented Sep 7, 2017

wawyed commented Sep 11, 2017 •

edited

Loading

erikbarke commented Sep 12, 2017

wawyed commented Sep 12, 2017

wawyed commented Sep 12, 2017 •

edited

Loading

erikbarke commented Sep 12, 2017

wawyed commented Sep 12, 2017

erikbarke commented Sep 13, 2017

wawyed commented Sep 13, 2017

erikbarke commented Sep 13, 2017

wawyed commented Sep 13, 2017 •

edited

Loading

erikbarke commented Sep 14, 2017

wawyed commented Sep 14, 2017

erikbarke commented Sep 14, 2017

erikbarke commented Sep 18, 2017

wawyed commented Sep 18, 2017

erikbarke commented Sep 18, 2017

feat: add source maps to libraries bundle #144

feat: add source maps to libraries bundle #144

Comments

wawyed commented Jun 30, 2017

erikbarke commented Jul 1, 2017

erikbarke commented Aug 13, 2017

wawyed commented Aug 13, 2017 • edited Loading

wawyed commented Aug 14, 2017

erikbarke commented Aug 16, 2017

erikbarke commented Aug 22, 2017

wawyed commented Aug 22, 2017

erikbarke commented Aug 23, 2017

wawyed commented Aug 23, 2017 • edited Loading

erikbarke commented Aug 24, 2017

wawyed commented Aug 24, 2017

erikbarke commented Aug 25, 2017

erikbarke commented Aug 31, 2017

wawyed commented Sep 5, 2017

erikbarke commented Sep 5, 2017

wawyed commented Sep 5, 2017

erikbarke commented Sep 6, 2017

wawyed commented Sep 7, 2017

wawyed commented Sep 11, 2017 • edited Loading

erikbarke commented Sep 12, 2017

wawyed commented Sep 12, 2017

wawyed commented Sep 12, 2017 • edited Loading

erikbarke commented Sep 12, 2017

wawyed commented Sep 12, 2017

erikbarke commented Sep 13, 2017

wawyed commented Sep 13, 2017

erikbarke commented Sep 13, 2017

wawyed commented Sep 13, 2017 • edited Loading

erikbarke commented Sep 14, 2017

wawyed commented Sep 14, 2017

erikbarke commented Sep 14, 2017

erikbarke commented Sep 18, 2017

wawyed commented Sep 18, 2017

erikbarke commented Sep 18, 2017

wawyed commented Aug 13, 2017 •

edited

Loading

wawyed commented Aug 23, 2017 •

edited

Loading

wawyed commented Sep 11, 2017 •

edited

Loading

wawyed commented Sep 12, 2017 •

edited

Loading

wawyed commented Sep 13, 2017 •

edited

Loading