Fork me on GitHub
#cljs-dev
<
2018-02-08
>
richiardiandrea00:02:46

@dnolen in self-host we build source maps inline with ?rel=number. Would it make sense to have a path that gets rid of that only if target is node? I have basically discovered that cljs.stacktrace/remove-ext does not work if ?rel=... is in there, I opened https://dev.clojure.org/jira/browse/CLJS-2493

richiardiandrea00:02:11

(unless I am missing some normalization step along the way)

richiardiandrea02:02:52

Ok I opened a couple of issues and sent a patch for https://dev.clojure.org/jira/browse/CLJS-2492. Now lumo has working stacktrace mapping 😄 🎉

dnolen09:02:05

@richiardiandrea thanks will take a look

dnolen09:02:30

@martinklepsch yeah that was an old conversation, enough people have stated they have legitimate reasons for whitespace - it should be supported

dnolen09:02:29

there’s probably some funny cases, since it’s a bit less common, but if it doesn’t work and could be made to work - that should be considered a defect

juhoteperi16:02:38

Closure february release is available, I'll try to check soon if that would happen to fix the problems my patch had

richiardiandrea17:02:44

data point, I updated lumo and had a test failure in an simple :npm-test.

juhoteperi20:02:48

So, Closure update doesn't fix the problem, but I think I finally might have found the cause

juhoteperi20:02:24

script/test has one AMD module, so that will enable .setTransformAMDToCJSModules for Closure Compiler

juhoteperi20:02:12

Other tests probably don't try AMD modules (at least with Node packages)

juhoteperi20:02:48

I think this is bug in Closure (it tries to process all files as AMD modules before json files are converted) but as AMD support has been deprecated, maybe it would be just bast to remove the AMD support from our side and remove the tests

juhoteperi20:02:41

Still getting one test failure:

FAIL in (test-printing) (core-advanced-test.js:1262:83)
Testing pr-str
expected: (#{"[1 true {:b #\"x\\\"y\", :a 2} #js [3 4]]" "[1 true {:a 2, :b #\"x\\\"y\"} #js [3 4]]"} (pr-str [1 true {:a 2, :b #"x"y"} (array 3 4)]))
  actual: nil

juhoteperi20:02:46

Seems strange that this would be broken by Closure-compiler update? The problem is that #"x\"y" is printed as #\"x\"y\", instead of as #\"x\\\"y\".

juhoteperi21:02:08

Or maybe not? After Cljs compilation JS looks like /x\"y/ but after optimization it is /x"y/

juhoteperi21:02:18

Running bisect on Closure to find the change, but seems like this has changed between v20170806 and v20171112

juhoteperi21:02:22

The optimization might be correct here, it is not necessary to escape " inside JS regex literal, the regex pattern will work correctly

juhoteperi21:02:46

But this will affect cases where the regex is converted back to string (e.g. printing)

juhoteperi21:02:04

These should be ready now: https://dev.clojure.org/jira/browse/CLJS-2375 Remove AMD Module support and tests https://dev.clojure.org/jira/browse/CLJS-2389 Closure Update, tests pass after CLJS-2375 https://dev.clojure.org/jira/browse/CLJS-2494 :npm-deps false to disable indexing node_modules https://dev.clojure.org/jira/browse/CLJS-2495 Throw if Closure reports errors

anmonteiro22:02:23

wow. 🎉 great work @juhoteperi