-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Inline source maps #2484
Inline source maps #2484
Conversation
@frankwallis you might find this helpful. |
thanks! |
@@ -291,7 +293,7 @@ module ts { | |||
function base64VLQFormatEncode(inValue: number) { | |||
function base64FormatEncode(inValue: number) { | |||
if (inValue < 64) { | |||
return 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'.charAt(inValue); | |||
return base64Digits.charAt(inValue); | |||
} | |||
throw TypeError(inValue + ": not a 64 based value"); |
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.
Maybe make this a Debug.fail
var charIndex = 0; | ||
|
||
var length = input.length; | ||
var i = 0; |
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.
Use let
consistently
Is the algorithm provided basically saying:
If so, i would personally prefer breaking out the two parts of this process. The UTF8 encoding, and the Base64 conversion. Note: MDN already covers a good way to do this: function utf8_to_b64( str ) {
return window.btoa(unescape(encodeURIComponent( str )));
}
or
function b64EncodeUnicode(str) {
return btoa(encodeURIComponent(str).replace(/%([0-9A-F]{2})/g, function(match, p1) {
return String.fromCharCode('0x' + p1);
}));
} Now, if we don't have access to these functions in Node or Chakra, we could still do the above steps, just individually. i.e. break up a string into utf8 bytes. Then convert those bytes to Base64. I think this would be far easier to understand (and verify). |
mm. let me try this out then. i remember there was a reason why i did not do that :) |
fda9974
to
a998abb
Compare
So btoa is not available on node, and escape is not available on chakra :), so there is not an easy way to use these. I did split them into two functions, and added unit tests as well. I have also handled --inlineSourceMap to be a parallel to --sourceMap instead of a dependent. The reason covertToBase64 is in utilities is to allow for targeted unit tests. I have limited compiler baselines to limt the noise in .js files as the map is actually included in the .js file. |
assert.equal(actual, expected, "Encoded string using convertToBase64 does not match buffer.toString('base64')"); | ||
} | ||
|
||
it("Converts ASCII charaters correctelly", () => { |
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.
:%s/correctelly/correctly/g
LGTM |
Can you also update |
thanks @teppeis. fixed |
Support for inline source maps. As referenced in #2233, this change allows us to emit single file with source maps instead of having a separate file.