Add support for compiler option :inline-source-maps

Description

1. refactor `emit-source-map` and break it into multiple functions
2. fix logic for relative path computation (see `strip-prefix-path`)
3. add support for `:inline-source-maps` option
4. add tests

Related: CLJS-1402, CLJS-1901

Environment

None

Attachments

4

Activity

David Nolen 
January 22, 2020 at 9:42 PM

Would like to see test status - Mike how are you dropping that in here? I can review after that.

Antonin Hildebrand 
January 13, 2020 at 7:49 PM

Added patch #4 - remove forgotten printing of debug info.

Antonin Hildebrand 
January 13, 2020 at 7:11 PM

Attached third patch.

Additionally I’ve done following changes (based on our discussion in #cljs-dev):

  1. using url-path instead of util/path so we get consistent behaviour with forward slashes regardless of OS

  2. wrapped cached-core in prepare-cached-core-if-needed and test :inline-source-maps there

  3. added :inline-source-maps into build-affecting-options

Bellow is transcript of our conversation:

 

Antonin Hildebrand 
January 11, 2020 at 2:46 PM

I would like to get this done. Can you please make the decision how to move forward? Thanks.

Antonin Hildebrand 
May 12, 2019 at 2:24 PM

I looked into that windows build and sourceMappingURL in failed tests looks like this (note mixed slashes):

The problem is in existing code using util/path, which is OS-dependent and produces back slashes under Windows, my patch didn't modify that code branch:
https://github.com/clojure/clojurescript/blob/47386d7c03e6fc36dc4f0145bd62377802ac1c02/src/main/clojure/cljs/compiler.cljc#L1473

I guess we have two possible resolutions:
1. leave existing behavior as is and make my tests accept mixed slashes
2. newly use forward slashes only, leave my tests intact and announce it as a potentially breaking change in next release

Solution 2 is better way forward IMO. I believe mixed slashes work because browsers are forgiving and treat backslashes as forward slashes in source mapping resolution. This change would have some tiny chance of breaking existing code with would depend on specific way how clojurescript generated sourceMappingURL with backslashes up until now.

I will leave it for your consideration. And then move forward with your decision.

Details

Assignee

Reporter

Patch

Priority

Created January 25, 2017 at 2:01 AM
Updated January 22, 2020 at 9:42 PM