Skip to content

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Mar 24, 2015

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.

@mhegazy
Copy link
Contributor Author

mhegazy commented Mar 24, 2015

@frankwallis you might find this helpful.

@frankwallis
Copy link
Contributor

thanks!

@mhegazy
Copy link
Contributor Author

mhegazy commented Mar 25, 2015

Also pinging @teppeis and @basarat for typescript-simple that can use this change.

Copy link
Member

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use let consistently

@CyrusNajmabadi
Copy link
Contributor

Is the algorithm provided basically saying:

  1. We have a string.
  2. We want to consider the UTF8 encoding of that string.
  3. We want to take that encoding and convert it to Base64?

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:
https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/btoa#Unicode_Strings

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).

@mhegazy
Copy link
Contributor Author

mhegazy commented Mar 25, 2015

mm. let me try this out then. i remember there was a reason why i did not do that :)

@mhegazy mhegazy force-pushed the inlineSourceMaps branch from fda9974 to a998abb Compare April 8, 2015 17:57
@mhegazy
Copy link
Contributor Author

mhegazy commented Apr 8, 2015

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:%s/correctelly/correctly/g

@vladima
Copy link
Contributor

vladima commented Apr 25, 2015

LGTM

@vladima
Copy link
Contributor

vladima commented Apr 25, 2015

Can you also update transpile function to use inlineSources\inlineSourceMaps?

mhegazy added a commit that referenced this pull request Apr 27, 2015
@mhegazy mhegazy merged commit 5673660 into master Apr 27, 2015
@mhegazy mhegazy deleted the inlineSourceMaps branch April 27, 2015 17:47
@mhegazy mhegazy mentioned this pull request Apr 27, 2015
@mhegazy
Copy link
Contributor Author

mhegazy commented Apr 29, 2015

thanks @teppeis. fixed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

8 participants