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
Do not add sourcemap markings for indentation #14506
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51792/ |
1 similar comment
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51792/ |
if (opts.stdoutContains) { | ||
expect(stdout).toContain(expectStdout); | ||
} else { | ||
fs.writeFileSync(opts.stdoutPath, stdout + "\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was very annoying.
} else if (stderr) { | ||
throw new Error("stderr:\n" + stderr); | ||
} catch (e) { | ||
if (!process.env.OVERWRITE) throw e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tired of manually updating fixtures, so I'm adding these in the places that can use it.
"sourcesContent": [ | ||
"arr.map(x => x * x);" | ||
], | ||
"mappings": "AAAAA,GAAG,CAACC,GAAJ,CAAQ,UAAAC,CAAC;EAAA,OAAIA,CAAC,GAAGA,CAAR;AAAA,CAAT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff here:
- AAAAA,GAAG,CAACC,GAAJ,CAAQ,UAAAC,CAAC;AAAA,SAAIA,CAAC,GAAGA,CAAR;AAAA,CAAT
+ AAAAA,GAAG,CAACC,GAAJ,CAAQ,UAAAC,CAAC;EAAA,OAAIA,CAAC,GAAGA,CAAR;AAAA,CAAT
The changed AAAA,SAAIA
-> EAAA,OAAIA
is the two spaces used for indentation. A
means 0, and E
means 2, so we moved the generated column 2 times when inserting the new marking. The S
-> O
is because markings are deltas, so we subtracted 2 columns from S
giving us O
.
@@ -273,7 +273,7 @@ class Printer { | |||
this.endsWith(charCodes.lineFeed) && | |||
str.charCodeAt(0) !== charCodes.lineFeed | |||
) { | |||
this._buf.queue(this._getIndent()); | |||
this._buf.queueIndentation(this._getIndent()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real change.
/** | ||
* Same as queue, but this indentation will never have a sourcmap marker. | ||
*/ | ||
queueIndentation(str: string): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the real change.
const SPACES_RE = /^[ \t]+$/; | ||
export default class Buffer { | ||
constructor(map?: SourceMap | null) { | ||
this._map = map; | ||
} | ||
|
||
_map: SourceMap = null; | ||
_buf: string = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but this file needs a lot of cleanup.
generated: { line: 2, column: 0 }, | ||
source: "a.js", | ||
original: { line: 1, column: 20 }, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: why was this deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is { line: 2, column: 0 }
of the generated output, which matches with indentation:
function hi(msg) {
console.log(msg);
}
hi('hello');
If you look at the next object, we have { line: 2, column: 2 }
, which matches with the start of console
in the generated output.
Thanks! |
This part 1 of me drastically simplifying the sourcemap generation now that we're off the
source-map
library.