Skip to content

Take comments into account when tracking sourcemap spans#2880

Merged
DanielRosenwasser merged 15 commits into
release-1.5from
sourceMapWithComments-attempt2
Apr 24, 2015
Merged

Take comments into account when tracking sourcemap spans#2880
DanielRosenwasser merged 15 commits into
release-1.5from
sourceMapWithComments-attempt2

Conversation

@DanielRosenwasser
Copy link
Copy Markdown
Member

This PR fixes #1980.

This is an alternative to PR #2014.


Rather than fixing up each use site to choose between how to emit, we create a common path that understands the sourcemap/comment ordering.

We now have 4 functions:

  • emit - emits a node with sourcemap info and comments if appropriate
  • emitNodeWithoutSourceMap - emits a node with only comment info
  • emitVerbatimDeclarationName - emits a node with sourcemaps/comments without renaming if it is an identifier
  • emitNodeWorker - emits a node with comments if appropriate, and takes a flag indicating whether or not to track sourcemaps. This is only called by the above two functions.

@JsonFreeman
Copy link
Copy Markdown
Contributor

👍

@NN---
Copy link
Copy Markdown

NN--- commented Aug 19, 2015

This doesn't fix the issue.
It is easy to reproduce using this simple TS and HTML.

module Q {
    function P() {
        // Test this
        var a = 1;
    }
}

Running tsc --sourceMap produces the following files

var Q;
(function (Q) {
    function P() {
        // Test this
        var a = 1;
    }
})(Q || (Q = {}));
//# sourceMappingURL=a.js.map

a.js.map:

{"version":3,"file":"a.js","sourceRoot":"","sources":["a.ts"],"names":["Q","Q.P"],"mappings":"AAAA,IAAO,CAAC,CAKP;AALD,WAAO,CAAC,EAAC,CAAC;IACNA;QAEIC,AADAA,YAAYA;YACRA,CAACA,GAAGA,CAACA,CAACA;IACdA,CAACA;AACLD,CAACA,EALM,CAAC,KAAD,CAAC,QAKP"}

Now open this HTML in FF or Chrome , and you can still put a breakpoint on the comment.

untitled

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants