Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add generated source map to Karma's file object #37

Closed
wants to merge 1 commit into from
Closed

Add generated source map to Karma's file object #37

wants to merge 1 commit into from

Conversation

Lalem001
Copy link
Contributor

@Lalem001 Lalem001 commented Oct 4, 2018

Found a solution for the misaligned lines/columns.

Fixes #36

Before:

at UserContext.it (main.spec.js:14:17)

After:

at UserContext.it (src/main.spec.js:7:15 <- main.spec.js:14:17)

@Lalem001
Copy link
Contributor Author

Lalem001 commented Oct 4, 2018

@jlmakes

I am not sure how to add tests that would actually test this change.

Also, this would obliterate any sourceMap that exists from a prior preprocessor/plugin if any. Taking prior source maps into account would likely require similar logic to what is in karma-coverage. Don't know if you want to add dependencies, and I thought the 1-liner would be an easier pill to swallow.

@jlmakes jlmakes added the fix label Oct 4, 2018
@jlmakes
Copy link
Owner

jlmakes commented Oct 6, 2018

In your estimation @Lalem001, what other use cases would have upstream preprocessors? It seems like a reasonable fix for most cases to me.

@Lalem001
Copy link
Contributor Author

Lalem001 commented Oct 6, 2018

I cannot think of any. I agree it is a reasonable fix for most cases.

@jlmakes
Copy link
Owner

jlmakes commented Oct 10, 2018

Hey @lephyrus thanks for your contributions! I’m going to make sure to cut a new release shortly, but I was hoping you could also share your opinion on this PR as well.

@jlmakes jlmakes requested review from jlmakes and removed request for jlmakes October 10, 2018 18:47
@lephyrus
Copy link
Contributor

@jlmakes @Lalem001 This is wonderful. For our project, it turns

Expected false to be true.
            at UserContext.<anonymous> (src/specs.ts:92368:70)

into

Expected false to be true.
            at UserContext.<anonymous> (src/platform/browser.spec.ts:29:59 <- src/specs.ts:92160:70)

So that can be massively helpful and I was actually missing that mapping. Luckily, it also doesn't screw up other source mappings (for coverage reports, for in-browser debugging). So a clear 👍 from me.

@jlmakes
Copy link
Owner

jlmakes commented Oct 17, 2018

Merged! Thank you @Lalem001 6.0.1...6.1.0

@jlmakes jlmakes closed this Oct 17, 2018
@Lalem001 Lalem001 deleted the fix-source-maps branch November 27, 2019 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants