ClojureScript

Add support for compiler option :inline-source-maps

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

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

  1. CLJS-1902.patch
    16/Jun/17 2:25 PM
    14 kB
    Antonin Hildebrand
  2. CLJS-1902-2.patch
    25/Apr/19 10:07 AM
    14 kB
    Antonin Hildebrand

Activity

Hide
Antonin Hildebrand added a comment -

Full review: https://github.com/clojure/clojurescript/compare/darwin:inline-source-maps~3...darwin:inline-source-maps

Also please note that the first patch testing original functionality fails in one test because there was a bug in timestamp formatting in :source-map-url case:
https://github.com/clojure/clojurescript/compare/master...darwin:inline-source-maps#diff-55b85385d2d0bfb6dc20d59ed982d5c8L1239

Show
Antonin Hildebrand added a comment - Full review: https://github.com/clojure/clojurescript/compare/darwin:inline-source-maps~3...darwin:inline-source-maps Also please note that the first patch testing original functionality fails in one test because there was a bug in timestamp formatting in :source-map-url case: https://github.com/clojure/clojurescript/compare/master...darwin:inline-source-maps#diff-55b85385d2d0bfb6dc20d59ed982d5c8L1239
Hide
Antonin Hildebrand added a comment -

Today when testing Dirac I realised we need to embed sources contents as well.

Additional patch:
https://github.com/darwin/clojurescript/commit/c1df38f14a33d02fe2d421f80db0b421b17286bb.patch

New review URL: https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps

Tested in DevTools and works like a charm.

Show
Antonin Hildebrand added a comment - Today when testing Dirac I realised we need to embed sources contents as well. Additional patch: https://github.com/darwin/clojurescript/commit/c1df38f14a33d02fe2d421f80db0b421b17286bb.patch New review URL: https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps Tested in DevTools and works like a charm.
Hide
Dusan Maliarik added a comment -

This would be helpful for us as well.

Show
Dusan Maliarik added a comment - This would be helpful for us as well.
Hide
Andrea Richiardi added a comment -

I have run across this one as well by following this tutorial.

Without either this patch or Dirac's complicated setup it is not currently possible to use node --inspect and debug correctly. The symptom I see on our side is that source maps are detected but for some reason Chrome DevTools does not show them in the Tree View.

The content of one of it is:

{"version":3,"file":"\/Users\/user\/cqrs-engine-cljs\/out\/cqrs\/event_store.js","sources":["event_store.cljs"], ...
Show
Andrea Richiardi added a comment - I have run across this one as well by following this tutorial. Without either this patch or Dirac's complicated setup it is not currently possible to use node --inspect and debug correctly. The symptom I see on our side is that source maps are detected but for some reason Chrome DevTools does not show them in the Tree View. The content of one of it is:
{"version":3,"file":"\/Users\/user\/cqrs-engine-cljs\/out\/cqrs\/event_store.js","sources":["event_store.cljs"], ...
Hide
David Nolen added a comment -

Linking to patches outside of JIRA is not proper for tickets. Please add a single squashed patch to this ticket directly.

Show
David Nolen added a comment - Linking to patches outside of JIRA is not proper for tickets. Please add a single squashed patch to this ticket directly.
Hide
Antonin Hildebrand added a comment -

Attached it as a patch file.

Took https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps.diff and applied it to current master. It applied cleanly without conflicts. Tests are still passing on my machine.

Show
Antonin Hildebrand added a comment - Attached it as a patch file. Took https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps.diff and applied it to current master. It applied cleanly without conflicts. Tests are still passing on my machine.
Hide
Mike Fikes added a comment -

Patch no longer applies; needs re-baseline.

Show
Mike Fikes added a comment - Patch no longer applies; needs re-baseline.
Hide
Antonin Hildebrand added a comment -

Newly when developing chrome extensions, source-maps got broken again[1]. Probably due to some added security restrictions in what Chrome allows Chrome DevTools to "see".

Some people on the interwebs claim that inlined source-maps are a possible workaround:
https://stackoverflow.com/a/54761431/84283

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=931675

Show
Antonin Hildebrand added a comment - Newly when developing chrome extensions, source-maps got broken again[1]. Probably due to some added security restrictions in what Chrome allows Chrome DevTools to "see". Some people on the interwebs claim that inlined source-maps are a possible workaround: https://stackoverflow.com/a/54761431/84283 [1] https://bugs.chromium.org/p/chromium/issues/detail?id=931675
Hide
David Nolen added a comment -

Just noting that I'm ok with what's proposed. Please re-baseline the patch and I can review in the near future.

Show
David Nolen added a comment - Just noting that I'm ok with what's proposed. Please re-baseline the patch and I can review in the near future.
Hide
Antonin Hildebrand added a comment -

Thanks. I will try to look into it during this week.

Show
Antonin Hildebrand added a comment - Thanks. I will try to look into it during this week.
Hide
Antonin Hildebrand added a comment -
Show
Antonin Hildebrand added a comment -

People

Vote (6)
Watch (3)

Dates

  • Created:
    Updated: