<< Back to previous view

[CLJS-1948] Possible race condition in compiler w/ parallel-build true Created: 21/Feb/17  Updated: 22/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Nikita Prokopov Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Zip Archive datascript_compiler_race.zip    

 Description   

I'm seeing errors trying to compile DataScript on 1.9.473. Judging from output, it looks like a race error in ClojureScript compiler: output JS files are damaged, removing parallel-build true fixes the problem.

Due to the nature of error, it might not be 100% reproducible. I get it 100% times on my machine though, but reported syntax errors in JS are always different.

The problem was probably introduced in 1.9.229. I'm not getting any errors compiling DataScript in earlier versions (1.9.227 and before)

HOW TO REPRODUCE:

unzip datascript_compiler_race.zip
cd datascript_compiler_race
wget https://github.com/clojure/clojurescript/releases/download/r1.9.473/cljs.jar
rm -rf target && java -cp cljs.jar:src clojure.main build.clj

As this is a race condition, it might not reproduce perfectly every time.

Examples of what I get on my MacBook Pro Retina Mid 2012 (Core i7-3615QM, 4 cores):

[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: parse-binding at line 215 is being replaced at line 215 src/datascript/parser.cljc
WARNING: collect-vars-acc at line 470 is being replaced at line 470 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
Feb 21, 2017 1:07:13 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:140: ERROR - Parse error. Semi-colon expected
datascript.parser.collect.cljsdatascript.parser.collect.cljs$lang$mavar G__16546 = cljs.datascript.parser.collvar seq16545__$1 = cljs.core.next(seq16545);
                                                                        ^

Feb 21, 2017 1:07:13 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:1676: ERROR - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
return faif(cljs.coredatascript.parser.RuleVars.prototype.cljs$core$IMap$_dissoc$arity$2 = (function (this__7667__auto__,k__7668return ((this__7654__auto____$1.constructor === other__7655__autoif(cljs.core.contains_QMARK_(new cljs.core.PersistentHashSet(null, new cljs.core.PersistentArrayMap(null, 2, [cljs.core.cst$kw$free,return true;
                                                                                                                                                                                                                                                                                                                                    ^

Feb 21, 2017 1:07:13 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 2 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. Semi-colon expected at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 140 : 72
ERROR: JSC_TRAILING_COMMA. Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option. at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 1676 : 324
[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: parse-binding at line 215 is being replaced at line 215 src/datascript/parser.cljc
WARNING: parse-clause at line 598 is being replaced at line 598 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
WARNING: parse-clauses at line 611 is being replaced at line 611 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
Feb 21, 2017 1:05:50 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:458: ERROR - Parse error. Semi-colon expected
datasreturn cljs.core.pr_sequential_writer(writer__7673__auto__,pr_pair__7675__auto__,"#datascript.parser.Variable{",", ","}",opts__7674__auto__,cljs.core.concat.cljs$core$IFn$_invoke$arity$2(new cljs.core.PersistentVector(null, 1, 5return (new cljs.core.RecordIter((0),G__16721__$1,1,new cljs.core.PersistentVector(null, 1, 5, cljs.core.PersistentVector.EMPTY_NODE, [cljs.core.cst$kw$symbol], null),(cljs.core.truth_(self__.__extmap)?cljs.core._iterator(self__.__extmap):cljs.core.nil_iter())));
            ^

Feb 21, 2017 1:05:50 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:4080: ERROR - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
throw cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2([cljs.core.str.cljs$cothrow cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2([cljs.core.str.cljs$core$IFn$_invoke$arity$1("Cannot parse pull expression, expect ['pull' src-var? variable (constant | variable | plain-symbol)]")].join(''),new cljs.core.PersistentArrayMap(null, 2, [cljs.core.cst$kw$error,}
                                                                                                                                                                                                                                                                                                                                                                  ^

Feb 21, 2017 1:05:50 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 2 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. Semi-colon expected at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 458 : 12
ERROR: JSC_TRAILING_COMMA. Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option. at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 4080 : 354
[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: parse-binding at line 215 is being replaced at line 215 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
Feb 21, 2017 1:02:55 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:4507: ERROR - Parse error. ',' expected
throw cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2([cljs.core.str.cljs$core$IFn$_invoke$arity$1("Cannot parse :find, expected: (find-rel | find-coll | find-tuple | find-scalar)")].join(''),new cljs.core.PersistentArrayMap(null, 2, [cljs.core.cst$kw$error,cljs.core.cst$kw$parser_SLASH_find,cljs.core.cst$kw$fragment,forvar or__6983__}
                                                                                                                                                                                                                                                                                                                                      ^

Feb 21, 2017 1:02:55 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. ',' expected at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 4507 : 326
[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: collect-vars-acc at line 470 is being replaced at line 470 src/datascript/parser.cljc
WARNING: parse-clause at line 598 is being replaced at line 598 src/datascript/parser.cljc
WARNING: parse-clauses at line 611 is being replaced at line 611 src/datascript/parser.cljc
Feb 21, 2017 1:02:00 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/target/datascript/parser.js:227: ERROR - Parse error. ',' expected
this.datascript.parser.Placeholder.prototype.cljs$core$ILookup$_lookup$arity$2 = (function (this__7658_this.cljs$lang$protocol_mask$partition1$ = 81var thidatascript.parser.Placeholdereturn this__7658__auto____$1.cljs$core$ILookup$_lookup$arity$3(null,k__7659__auto__,null);
                                                                                                           ^

Feb 21, 2017 1:02:00 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. ',' expected at /Users/prokopov/datascript_compiler_race/target/datascript/parser.js line 227 : 107
[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: parse-binding at line 215 is being replaced at line 215 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
Feb 21, 2017 1:00:46 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/out/datascript/parser.js:215: ERROR - Parse error. Character '@' (U+0040) is not a valid identifier start char
datascript.parser.Placeholder = (functio * @implements {cljs.core.IAthis.__meta  * @implemthis.__extmap = __extm * @this.__hash = __hash;
                                           ^

Feb 21, 2017 1:00:46 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. Character '@' (U+0040) is not a valid identifier start char at /Users/prokopov/datascript_compiler_race/out/datascript/parser.js line 215 : 43
[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj
WARNING: collect-vars-acc at line 470 is being replaced at line 470 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
WARNING: parse-clause at line 598 is being replaced at line 598 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
WARNING: parse-clauses at line 611 is being replaced at line 611 /Users/prokopov/datascript_compiler_race/src/datascript/parser.cljc
Feb 21, 2017 12:59:33 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: /Users/prokopov/datascript_compiler_race/out/datascript/parser.js:7639: ERROR - Parse error. '}' expected

Feb 21, 2017 12:59:33 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_PARSE_ERROR. Parse error. '}' expected at /Users/prokopov/datascript_compiler_race/out/datascript/parser.js line 7639 : 0

Note how output JS is not even syntactically correct:

functio * @implements
__meta  * @implemthis.__extmap
,forvar or__6983__}

This is a clear sign that two or more threads are writing to the same file without coordination.

Also note that with parallel build enabled, compiler confuses declare with function defenition and warns about collect-vars-acc at line 470 is being replaced at line 470. In fact, the function is first decalre-d, then defn-ed just once in the same file, which, under normal circumstances, should not trigger any warnings.

Also note that setting :parallel-build false leads to 100% stable, error-less builds:

[~/datascript_compiler_race] rm -rf target && java -cp cljs.jar:src clojure.main build.clj --no-parallel
[~/datascript_compiler_race]


 Comments   
Comment by David Nolen [ 22/Feb/17 6:50 AM ]

This issue needs work before it will be considered. Do not link outside to minimal cases, you need to provide all the information in the ticket.

Comment by Nikita Prokopov [ 22/Feb/17 7:12 AM ]

David, thanks for the heads up! I’ve updated the issue with standalone zip containing test case and instructions on how to build it. Unfortunately I cannot include cljs.jar in it because it’s too big (24 Mb, while JIRA only allows for 10 Mb attachments max). But I’m sure you have one somewhere on your machine





[CLJS-1937] Self-host: undeclared cljs.core$macros/mod when compiling cljs/core.cljs Created: 12/Feb/17  Updated: 22/Feb/17  Resolved: 22/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1937.patch    
Patch: Code

 Comments   
Comment by António Nuno Monteiro [ 12/Feb/17 11:32 PM ]

Attached patch with fix.

Comment by David Nolen [ 22/Feb/17 6:59 AM ]

fixed https://github.com/clojure/clojurescript/commit/2aa8aae2d8499d1b09a25bfa0b447c46d230cfe0





[CLJS-1936] cljs.analyzer declares vars which are only used in Clojure Created: 11/Feb/17  Updated: 22/Feb/17  Resolved: 22/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1936.patch    
Patch: Code

 Description   

We shouldn't be declaring these vars on the ClojureScript side, as they can lead to runtime errors when trying to call those functions from the self-hosted compiler. Putting them under reader conditionals enables emitting warnings when they are used.



 Comments   
Comment by David Nolen [ 22/Feb/17 6:58 AM ]

fixed https://github.com/clojure/clojurescript/commit/a3a242236e7757a179fd16dd9b86767a73a8cb5d





[CLJS-1949] Self-host: cljs.compiler/munge doesn't preserve JVM compiler semantics Created: 21/Feb/17  Updated: 22/Feb/17  Resolved: 22/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1949.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 21/Feb/17 5:17 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 21/Feb/17 5:30 PM ]

FWIW, I tried the attached patch downstream with Planck and it worked. Prior to the patch, I observed this behavior:

cljs.user=> (.catch (js/Promise. #(%2 "x")) #(println %))
(new Promise((function (p1__20_SHARP_,p2__19_SHARP_){
return p2__19_SHARP_.call(null,"x");
}))).catch$ is not a function. (In '(new Promise((function (p1__20_SHARP_,p2__19_SHARP_){
return p2__19_SHARP_.call(null,"x");
}))).catch$((function (p1__21_SHARP_){
return cljs.core.println.call(null,p1__21_SHARP_);
}))', '(new Promise((function (p1__20_SHARP_,p2__19_SHARP_){
return p2__19_SHARP_.call(null,"x");
}))).catch$' is undefined)

And with the patch:

cljs.user=> (.catch (js/Promise. #(%2 "x")) #(println %))
x
#object[Promise [object Promise]]
Comment by David Nolen [ 22/Feb/17 6:56 AM ]

fixed https://github.com/clojure/clojurescript/commit/025dd9d3e57f1955fd442b9f69823c892a38cb58





[CLJS-1950] Eliminate instances of #^ Created: 21/Feb/17  Updated: 22/Feb/17  Resolved: 22/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1950.patch    
Patch: Code

 Description   

With CLJS-1636 I had, without thinking, blindly followed the existing pattern used to mark def s as private, employing

#^:private

This ticket is to remove those new and the existing instance of this older syntax.



 Comments   
Comment by David Nolen [ 22/Feb/17 6:52 AM ]

fixed https://github.com/clojure/clojurescript/commit/6325a29612b6266ee786ce49a83f12566708e696





[CLJS-1943] Self-host: `cljs.pprint`'s macros can't be compiled Created: 15/Feb/17  Updated: 20/Feb/17  Resolved: 20/Feb/17

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1943-2.patch     Text File CLJS-1943-3.patch     Text File CLJS-1943.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 15/Feb/17 6:46 PM ]

Attached patch with fix and enabled running the cljs.pprint tests when running the self-parity tests.

Comment by David Nolen [ 20/Feb/17 1:50 PM ]

fixed https://github.com/clojure/clojurescript/commit/6c5fb679bf843bfbd065d0576e1a3d350f11d4b0





[CLJS-1945] cljs.spec/every-impl kind-fn kind-form dead code Created: 18/Feb/17  Updated: 20/Feb/17  Resolved: 20/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1945.patch    
Patch: Code

 Description   

With Clojure 386e7e6 a [kindfn kindform] let was removed here

https://github.com/clojure/clojure/commit/386e7e63485bcb7bed050df2c2b54a6ceca05e5f#diff-b0f91f319d0c76aadf667da929b08d37L1031

this upstream revision was not copied in ClojureScript 1a297c5 around here:

https://github.com/clojure/clojurescript/commit/1a297c52d958520065335815ffb3dee07c314aa7#diff-5c48ec6d047e1ea6fdcdd00b4f09f45eR729

and is currently dead code that could be removed.



 Comments   
Comment by David Nolen [ 20/Feb/17 11:01 AM ]

fixed https://github.com/clojure/clojurescript/commit/7326d79001f2fbbb2ee4086b1f645cab374c0e58





[CLJS-1944] Can't spec generate non-vector collections Created: 18/Feb/17  Updated: 20/Feb/17  Resolved: 20/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1944.patch    
Patch: Code and Test

 Description   

If you use cljs.spec/coll-of with non-vector :kind, you can't exercise the spec.

Repro:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 58736
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.spec :as s] 'clojure.test.check.generators)
true
cljs.user=> (s/exercise (s/coll-of string? :kind set?))
Error: Couldn't satisfy such-that predicate after 100 tries.
...


 Comments   
Comment by Mike Fikes [ 18/Feb/17 11:48 AM ]

This is a cut-n-dry defect simply involving missing this Clojure deletion:

https://github.com/clojure/clojure/commit/23e3ec3f8bceeedee70beed7a17846c25eba05a6#diff-b0f91f319d0c76aadf667da929b08d37L1208

in this ClojureScript tracking change here:

https://github.com/clojure/clojurescript/commit/cd43ec9bf40605e230dc8858c2855f9ad85b39d8#diff-5c48ec6d047e1ea6fdcdd00b4f09f45eL702

The consequence is that gen-into is always non-nil (set to []) and then kind is not made use of here: https://github.com/clojure/clojurescript/blob/627f7fd513d928531db48392d8f52142fea5eb38/src/main/cljs/cljs/spec.cljs#L890-L891

Comment by David Nolen [ 20/Feb/17 10:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/77d9077c04f61a19d86e67d18bd7c93217e88aa3





[CLJS-1946] Self-hosted: don't emit `goog.require` calls for foreign libs if optimizations different than `:none` Created: 18/Feb/17  Updated: 20/Feb/17  Resolved: 20/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1946.patch    
Patch: Code

 Description   

This is done in JVM CLJS but not in self-hosted.



 Comments   
Comment by David Nolen [ 20/Feb/17 10:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/23beecb7e0b752c98cacebfe0aff3879eab0f0e1





[CLJS-1947] cljs.spec "and" passes :cljs.spec/invalid in next spec after failed if there are 4+ sub-specs Created: 19/Feb/17  Updated: 20/Feb/17  Resolved: 20/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Vlad Protsenko Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: bug, clojurescript, spec


 Description   

This bug affects version 1.9.473, but I could not select it in select box, and I did not test it on other versions.

;; This code will print :cljs.spec/invalid once:
(s/conform (s/and pos? println println println) 0)
:cljs.spec/invalid
=> :cljs.spec/invalid

;; This code does not print anything
(s/conform (s/and pos? println println) 0)
=> :cljs.spec/invalid


 Comments   
Comment by Mike Fikes [ 19/Feb/17 9:26 PM ]

Duplicate of CLJS-1935.

Confirmed via git bisect that https://github.com/clojure/clojurescript/commit/ff0b11c123bf46d9e0efff164a7327fb269d2262 fixed the issue reported in this ticket.





[CLJS-536] clj->js trims the namespace prefix from keywords while writing them to string Created: 12/Jul/13  Updated: 19/Feb/17  Resolved: 02/Dec/13

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Vasile Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: clj->js

Attachments: Text File 0001-CLJS-536-Add-support-for-namespaced-keywords.patch     Text File 0001-CLJS-536-Add-support-for-namespaced-keywords.patch    

 Description   

The following behavior was observed and confirmed from the code:

(clj->js :ns/n) => "n"

I believe this is a limitation and the namespace of the keyword should be kept while writing it to string.
The code in core.js does this while handling keywords:

(keyword? x) (name x)

while it should do this (or something similar):

(keyword? x) (str (namespace x) "/" (name x))



 Comments   
Comment by Vasile [ 12/Jul/13 12:03 PM ]

a better (working) fix: (keyword? x) (str (if (namespace x) (str (namespace x) "/")) (name x))

Comment by David Nolen [ 16/Jul/13 6:22 AM ]

I'd be willing to take a patch that allows this behavior to be configured.

Comment by Niklas Närhinen [ 01/Nov/13 7:33 AM ]

Proposed fix

Comment by Niklas Närhinen [ 01/Nov/13 7:37 AM ]

Fixed version of patch

Comment by David Nolen [ 01/Nov/13 9:23 AM ]

Excellent, Niklas I don't see you on the list of contributors, please send in your Contributor Agreement, http://clojure.org/contributing, so we can apply the patch.

Comment by David Nolen [ 02/Dec/13 8:52 PM ]

I looked more closely at the clj->js source, the system is already customizable. We can't determine ahead of time how you might want to emit keywords and symbols - thus you can extend Keyword and Symbol to IEncodeJS yourself and get the desired behavior.

Comment by Andrea Richiardi [ 17/Jan/17 2:52 PM ]

I have just stumbled across this one, shall we at least say it in the docstring of clj->js that we are losing the namespace part?

Comment by Jozef Wagner [ 19/Feb/17 4:11 AM ]

With the introduction of specs, the namespaced keywords are being used more and more. This issue prevents streamlined edn->json->edn transformation. I think it should be reopened. IMO the 'lossy' method should never be a default one.

Comment by Paulus Esterhazy [ 19/Feb/17 5:53 AM ]

Unless we are willing to break existing code, I don't think it will be possible to change the default behavior.

I'm also not sure that extending IEncodeJS is the best solution, as it affects every call to clj->js, including calls to libraries which may rely on the ns-stripping behavior.

However, the attached patch allows you to make the decision on a per-call basis.

One quibble with the patch: perhaps it would be better to use kwargs style `(clj->js v :preserve-namespaces true)` in line with `js->clj`?





[CLJS-1935] When calling cljs.spec/valid?, subsequent predicates of cljs.spec/and are evaluated even when early predicate is unsatisfied Created: 11/Feb/17  Updated: 16/Feb/17  Resolved: 15/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Henrik Lundahl Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: clojure.spec

Attachments: Text File CLJS-1935.patch    

 Description   

Affects 1.9.456 and 1.9.473.

The problem is that subsequent predicates of cljs.spec/and are evaluated even when an early predicate is unsatisfied when calling cljs.spec/valid?.

See https://github.com/henriklundahl/minimal-cljs-spec-problem for a minimal example demonstrating the problem.



 Comments   
Comment by David Nolen [ 11/Feb/17 10:49 AM ]

Please do not link outside. Minimal examples need to be in the ticket. This means no project. Just code demonstrating the problem. Thanks.

Comment by Henrik Lundahl [ 12/Feb/17 7:59 AM ]

Since I'm not allowed to edit the description I'll just add the details here.

The following code renders an exception:

(ns asdf
(:require [clojure.spec :as s]))

(defn pred-1? [s]
(> (count s) 100))

(defn pred-2? [s]
(> (count s) 100))

(defn pred-3? [s]
(> (count s) 100))

(s/valid? (s/and string? pred-1? pred-2? pred-3?) "a")

The exception is something like:

Uncaught Error: No protocol method ICounted.-count defined for type cljs.core/Keyword: :cljs.spec/invalid

This works in both Clojure 1.9.0-alpha14 and ClojureScript 1.9.293. It does not work in neither ClojureScript 1.9.456 nor ClojureScript 1.9.473.

Comment by David Nolen [ 15/Feb/17 5:38 PM ]

fixed https://github.com/clojure/clojurescript/commit/ff0b11c123bf46d9e0efff164a7327fb269d2262

Comment by Henrik Lundahl [ 16/Feb/17 12:47 AM ]

Thanks!





[CLJS-1636] Mark some symbols in core macros ns as private Created: 27/Apr/16  Updated: 15/Feb/17  Resolved: 15/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1636-2.patch     Text File CLJS-1636-3.patch     Text File CLJS-1636.patch    
Patch: Code

 Description   

There are some symbols in the core macros namespace that are not meant for external consumption. Some of these are marked private and some aren't. This ticket asks that the others be marked private as well.

An example of one symbol marked private is defcurried.
An example of one symbol not marked private is caching-hash.



 Comments   
Comment by Mike Fikes [ 27/Apr/16 8:21 AM ]

In CLJS-1636.patch, I checked and it appears nothing in the compiler codebase is explicitly using these symbols outside of the cljs.core namespace. But, it is still worth scanning through these to check if they make sense. For example js-debugger and js-comment are a couple that might actually be meant for public use, but it is difficult to tell.

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 31/Jul/16 1:33 PM ]

Patch no longer applies. Attaching updated patch.

Comment by Mike Fikes [ 12/Feb/17 1:33 PM ]

Previous patch no longer applies; attaching re-baselined patch.

Comment by David Nolen [ 15/Feb/17 5:46 PM ]

fixed https://github.com/clojure/clojurescript/commit/627f7fd513d928531db48392d8f52142fea5eb38





[CLJS-1939] Bad load_file generated for foreign-dep requires on Node Created: 13/Feb/17  Updated: 15/Feb/17  Resolved: 15/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Juho Teperi Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1939-1.patch    

 Description   

After CLJS-1922 ijs :file is preferred as output-path when writing the JS files to output-dir. compiler/load-libs still uses util/relative-name for all cases, which won't work when the JS file is loaded from classpath and written to output-dir using classpath path.

Fix is to use the same logic in load-files to build the path as is used in closure/rel-output-path.



 Comments   
Comment by David Nolen [ 15/Feb/17 5:44 PM ]

fixed https://github.com/clojure/clojurescript/commit/90fc275d939f6c701096378134a930adbd9493f3





[CLJS-1942] Self-host: `cljs.env.macros` and `cljs.analyzer.macros` can't be loaded Created: 14/Feb/17  Updated: 15/Feb/17  Resolved: 15/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1942.patch    
Patch: Code

 Comments   
Comment by António Nuno Monteiro [ 14/Feb/17 3:50 PM ]

Attached patch with fix.

Comment by David Nolen [ 15/Feb/17 5:41 PM ]

fixed https://github.com/clojure/clojurescript/commit/47cd1cef8f7ed5b16cffc7e2ae4d223328091ab7





[CLJS-1941] `cljs.compiler/cljs-files-in` shouldn't return `.cljc` files if a `.cljs` file exists for the namespace Created: 14/Feb/17  Updated: 14/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1941.patch    
Patch: Code

 Description   

We're seeing non-determinism in builds due to this problem. This is the same bug as CLJS-1772, but happens at a different code path.



 Comments   
Comment by António Nuno Monteiro [ 14/Feb/17 12:26 PM ]

Attached patch with fix.





[CLJS-1940] Undeclared var warning when invoking a protocol method on a `js` interop form Created: 14/Feb/17  Updated: 14/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: interop, protocols, regression


 Description   

Compiling the following code under 1.9.743

(ns test.foo)

(defprotocol Proto
  (f [this]))

(defn foo []
  (f js/Math.E))

Produces the following warning:

WARNING: Use of undeclared Var test.foo/js at line 7 src/test/foo.cljs

This is a regression from 1.9.293

https://github.com/Bronsa/CLJS-1.9.473-regression contains a project set up to reproduce this






[CLJS-1938] :elide-asserts compiler option without effect Created: 13/Feb/17  Updated: 13/Feb/17  Resolved: 13/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

Some self-host related things broke the :elide-asserts compiler option.

cljs.closure/build is the only place that binds *assert* when the :elide-asserts option is set. This however binds the clojure.core dynamic var, the cljs.core/assert macro however uses the cljs.core dynamic var. Therefore this is always true and asserts are never removed.

Possible fixes:
[1] change to cljs.core/*assert*.
[2] change to (core/when core/*assert* ...

I would like to revisit http://dev.clojure.org/jira/browse/CLJS-1494 and always emit the asserts but wrapped in a goog-define so Closure is able to remove them without us actually affecting the generated JS.

[1] https://github.com/clojure/clojurescript/blob/af2ca808fcc5efc088b48957b898fed0b59eddbe/src/main/clojure/cljs/closure.clj#L2091
[2] https://github.com/clojure/clojurescript/blob/af2ca808fcc5efc088b48957b898fed0b59eddbe/src/main/clojure/cljs/core.cljc#L2247



 Comments   
Comment by Thomas Heller [ 13/Feb/17 7:05 AM ]

Sorry for the noise, my conclusion was wrong. Looked at the wrong file.





[CLJS-1931] Closure Compiler {{--generate_exports}} flag not supported Created: 08/Feb/17  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Stephens Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

We are including a third-party, Closure-compiler compatible JS library in our ClojureScript project, which is using the @export JSDoc-style tags in the source to export names, and another third party JS library (not Closure Compiler compatible) is expecting those names to be exported.

However, the names are not being exported because, we believe, the --generate_exports flag needs to be passed to the Closure Compiler to get it to parse the JSDoc comments and generate goog.export() calls, and there is no interface to enable that flag from the ClojureScript compiler.



 Comments   
Comment by David Nolen [ 10/Feb/17 12:06 PM ]

fixed https://github.com/clojure/clojurescript/commit/af2ca808fcc5efc088b48957b898fed0b59eddbe





[CLJS-1919] cljs.spec conformer regression Created: 30/Jan/17  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Allan Jiang Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

Upon compiling the following code,

(ns repro.core
  (:require [cljs.spec :as s]))

(defn coll->set [x] 
  (cond
    (set? x) x
    (coll? x) (set x)
    :else ::s/invalid))

(s/def ::set-ints (s/coll-of int? :kind (s/conformer coll->set) :into #{}))
(s/def ::vec-ints (s/coll-of int? :kind vector? :into []))
(try
  (s/valid? ::set-ints [1 2 3]) 
  (catch js/Object e
    (js/console.error e)))
(s/valid? ::vec-ints [1 2 3])

the validation for ::set-ints fails due to using the conformer.

The error message is:

TypeError: cljs.spec.spec_impl.call(...).call is not a function
    at core.cljs:10
    at cljs.spec.every_impl.cljs$core$IFn$_invoke$arity$4.cljs.spec.t_cljs$spec860.cljs$spec$Spec$conform_STAR_$arity$2 (spec.cljs:844)
    at cljs$spec$conform_STAR_ (spec.cljs:40)
    at Function.cljs.spec.valid_QMARK_.cljs$core$IFn$_invoke$arity$2 (spec.cljs:357)
    at cljs$spec$valid_QMARK_ (spec.cljs:353)
    at core.cljs:13


 Comments   
Comment by David Nolen [ 10/Feb/17 11:59 AM ]

As far as I can tell this not a bug, it doesn't work in Clojure 1.9.0-alpha14





[CLJS-1443] ES6 Module Processing at individual :foreign-lib spec Created: 09/Sep/15  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

ES6 module processing could probably benefit from processing at the individual :foreign-lib spec. Brings up questions wrt. source maps and merged source maps when applying other optimization settings.






[CLJS-1912] cljs.build.api/node-modules Created: 27/Jan/17  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 10/Feb/17 10:58 AM ]

fixed in 1.9.456





[CLJS-1074] Externs inference Created: 02/Mar/15  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3211
Fix Version/s: GSoC

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Maria Geller
Resolution: Completed Votes: 4
Labels: None


 Description   

Given all externs generally need to be supplied for js/foo we could probably automatically compute externs based on js/foo usage in user code. For this to work correctly we need to account for property access through js/foo i.e. (def Bar js/foo.Bar). This should infer that Bar is also a foreign object. Things gets more complicated for higher order cases, we probably want to support a ^js type hint.

Finally externs inference needs to account for externs likely already supplied by the user - i.e. don't emit dupes, Google Closure will complain.



 Comments   
Comment by Leon Grapenthin [ 27/Feb/16 3:17 PM ]

Is this still being worked on?

Here is an approach: https://gist.github.com/Chouser/5796967

A very lean first approach would be to generate a `var foo = {}` for every interop expression.

I. e. by experimentation I could observe that no nested statements or var foo = function() statements are required to prevent minification.

js/foo 
js/foo.Bar 
(js/foo.Bar) 
(.-Bar js/foo) 
(.-Bar x) 
;; etc... would all not be minified with 
var foo = {}; 
var Bar = {};

To prevent dupes a cheap way to go would be a CLJS compiler mode in which no extern files are loaded. We can disable Closures externs via the exclude_default_externs compiler flag.

IDK if the minification quality is in any way different if the externs are type annotated or declared nested of with =function() --?

At least it looks like doing this would automate the most common use case of externs in CLJS: Preventing minification.

Comment by David Nolen [ 29/Feb/16 9:05 PM ]

Not actively being worked on at the moment but Maria Geller has a pretty solid proof of concept in a branch that somebody else can pick up. It takes the basic idea from that gist much further.

Comment by Leon Grapenthin [ 01/Mar/16 12:41 AM ]

Branch for reference: https://github.com/mneise/clojurescript/commits/CLJS-1074

Thanks David. Will have a closer look asap.

Comment by David Nolen [ 10/Feb/17 10:58 AM ]

This was shipped in 1.9.456





[CLJS-1934] Self-host: require-macros :reload / :reload-all fails Created: 09/Feb/17  Updated: 10/Feb/17  Resolved: 10/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1934.patch    
Patch: Code

 Description   

In a self-hosted environment set up to work with cljs.js/*load-fn*, evaluating an expression like

(require-macros 'foo.core :reload)

fails to request a reload of the code.



 Comments   
Comment by Mike Fikes [ 09/Feb/17 5:55 PM ]

This was caused by a simple typo. With the typo and the example in the description, reloads would be passed in as an empty map instead of reload which would contain the key :reload-macros and the value :reload. Deleting the extra s makes it consistent with the other nearby calls and does the trick.

Comment by David Nolen [ 10/Feb/17 9:45 AM ]

fixed https://github.com/clojure/clojurescript/commit/bf37131c02a393bbfde3da32d2ffd6f878393cb1





[CLJS-1933] Support CLJS browserless remote REPL from nodejs Created: 09/Feb/17  Updated: 09/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Longworth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, remote, repl
Environment:

Dev machine OSX - build and Cursive editing and REPL
Remote: RaspberryPI - nodejs
compiler out-files shared between devices with OSXFUSE.



 Description   

I would like to develop clojurescript for a remote nodejs target compiling cljs and running a REPL on my development machine.
https://github.com/clojure/clojurescript/wiki/Remote-REPL suggests a way of doing this however:

!) I haven't managed to get this working.
2) I don't like that the solution relies identical absolute file paths for the compiler output, better to have matching relative paths.

I made a post to request help with this but haven't managed to resolve all the issues:
https://groups.google.com/forum/#!topic/clojurescript/Y4ajOcej8Qo

I have made some progress since the post:
1) I dug into the cljs.repl.node source and found I can stop the node hang I reported by specifying repl-env :debug-port, and get:

Clojure 1.8.0
Debugger listening on port 5002
ClojureScript Node.js REPL server listening on 5001
TypeError: Cannot read property 'nameToPath' of undefined
at Object.goog.require (repl:2:49)
at repl:1:-56
at Object.exports.runInThisContext (vm.js:54:17)
at Domain.<anonymous> ([stdin]:50:34)
at Domain.run (domain.js:221:14)
at Socket.<anonymous> ([stdin]:49:25)
at emitOne (events.js:77:13)
at Socket.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:146:16)
at Socket.Readable.push (_stream_readable.js:110:10)
TypeError: goog.provide is not a function
at repl:1:-56
at Object.exports.runInThisContext (vm.js:54:17)
at Domain.<anonymous> ([stdin]:50:34)
at Domain.run (domain.js:221:14)
at Socket.<anonymous> ([stdin]:49:25)
at emitOne (events.js:77:13)
at Socket.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:146:16)
at Socket.Readable.push (_stream_readable.js:110:10)
at TCP.onread (net.js:523:20)
To quit, type: :cljs/quit

This looks like some kind of path problem but I haven't managed to resolve it.

I did some investigations with my original relative-path setup to try and identify the issues:
1) I eliminated absolute paths from the compile output by disabling analysis caching.
2) I ran wireshark on the REPL port and found that absolute paths were being sent by the REPL, this currently makes the relative path option unworkable.

I have many gaps in my knowledge of the REPL operation at the moment and I don't know what the best approach is to getting a good solution for a browserless remote repl setup.



 Comments   
Comment by David Nolen [ 09/Feb/17 12:33 PM ]

It's probably going to be easier to discuss this issue in IRC or Slack first. There's just too many different issues piled into this one. Thanks.





[CLJS-1932] Self-host: Perf regression macroexpand-check Created: 08/Feb/17  Updated: 09/Feb/17  Resolved: 09/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1932.patch    
Patch: Code

 Description   

With https://github.com/clojure/clojurescript/commit/cebfb586355b526253fdd25965de976b49a6973e you can observe a compilation perf regression. This is particularly noticeable if you load cljs.core.async (from Andare) into a self-hosted environment (both Lumo and Planck exhibit 2-3x slowdowns).



 Comments   
Comment by Mike Fikes [ 08/Feb/17 11:30 PM ]

With the attached patch, downstream perf in loading cljs.core.async in Planck is confirmed to return to the previous good perf.

Also, confirmed downstream that the change doesn't break the ability to have specs on macros:

With this macros namespace:

(ns foo.core
  (:require [clojure.spec :as s]))

(defmacro add [a b]
  `(+ ~a ~b))

(s/fdef add
  :args (s/cat :a int? :b int?))

The spec works properly downstream in Planck:

cljs.user=> (require-macros 'foo.core)
nil
cljs.user=> (foo.core/add "a" 3)
            ^
Call to foo.core$macros/add did not conform to spec:
In: [0] val: "a" fails at: [:args :a] predicate: int?
:cljs.spec/args  ("a" 3)
 at line 1
cljs.user=> (foo.core/add 2 3)
5
Comment by David Nolen [ 09/Feb/17 5:35 AM ]

fixed https://github.com/clojure/clojurescript/commits/master





[CLJS-1930] Master broken wrt static field: ES5_STRICT_UNCOMMON Created: 06/Feb/17  Updated: 06/Feb/17  Resolved: 06/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1930.patch    
Patch: Code

 Description   

{{git clone https://github.com/clojure/clojurescript}}

cd clojurescript/

script/build

...

+ java -server -cp /Users/mfikes/.m2/repository/org/clojure/clojure/1.8.0/clojure-1.8.0.jar:/Users/mfikes/.m2/repository/com/google/javascript/closure-compiler-unshaded/v20161201/closure-compiler-unshaded-v20161201.jar:/Users/mfikes/.m2/repository/com/google/javascript/closure-compiler-externs/v20161201/closure-compiler-externs-v20161201.jar:/Users/mfikes/.m2/repository/args4j/args4j/2.33/args4j-2.33.jar:/Users/mfikes/.m2/repository/com/google/guava/guava/20.0/guava-20.0.jar:/Users/mfikes/.m2/repository/com/google/protobuf/protobuf-java/3.0.2/protobuf-java-3.0.2.jar:/Users/mfikes/.m2/repository/com/google/code/gson/gson/2.7/gson-2.7.jar:/Users/mfikes/.m2/repository/com/google/code/findbugs/jsr305/3.0.1/jsr305-3.0.1.jar:/Users/mfikes/.m2/repository/com/google/jsinterop/jsinterop-annotations/1.0.0/jsinterop-annotations-1.0.0.jar:/Users/mfikes/.m2/repository/org/clojure/google-closure-library/0.0-20160609-f42b4a24/google-closure-library-0.0-20160609-f42b4a24.jar:/Users/mfikes/.m2/repository/org/clojure/google-closure-library-third-party/0.0-20160609-f42b4a24/google-closure-library-third-party-0.0-20160609-f42b4a24.jar:/Users/mfikes/.m2/repository/org/clojure/data.json/0.2.6/data.json-0.2.6.jar:/Users/mfikes/.m2/repository/org/mozilla/rhino/1.7R5/rhino-1.7R5.jar:/Users/mfikes/.m2/repository/org/clojure/tools.reader/1.0.0-beta3/tools.reader-1.0.0-beta3.jar:/Users/mfikes/.m2/repository/org/clojure/test.check/0.9.0/test.check-0.9.0.jar:/Users/mfikes/.m2/repository/com/cognitect/transit-clj/0.8.285/transit-clj-0.8.285.jar:/Users/mfikes/.m2/repository/com/cognitect/transit-java/0.8.311/transit-java-0.8.311.jar:/Users/mfikes/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.3.2/jackson-core-2.3.2.jar:/Users/mfikes/.m2/repository/org/msgpack/msgpack/0.6.10/msgpack-0.6.10.jar:/Users/mfikes/.m2/repository/com/googlecode/json-simple/json-simple/1.1.1/json-simple-1.1.1.jar:/Users/mfikes/.m2/repository/org/javassist/javassist/3.18.1-GA/javassist-3.18.1-GA.jar:/Users/mfikes/.m2/repository/commons-codec/commons-codec/1.10/commons-codec-1.10.jar:src/main/clojure:src/main/cljs clojure.main script/aot.clj
Exception in thread "main" java.lang.RuntimeException: Unable to find static field: ES5_STRICT_UNCOMMON in class com.google.javascript.jscomp.DiagnosticGroups, compiling:(cljs/closure.clj:98:1)
	at clojure.lang.Compiler.analyze(Compiler.java:6688)
	at clojure.lang.Compiler.analyze(Compiler.java:6625)
	at clojure.lang.Compiler$MapExpr.parse(Compiler.java:3072)
	at clojure.lang.Compiler.analyze(Compiler.java:6677)
	at clojure.lang.Compiler.access$300(Compiler.java:38)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:589)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6868)
	at clojure.lang.Compiler.analyze(Compiler.java:6669)
	at clojure.lang.Compiler.analyze(Compiler.java:6625)
	at clojure.lang.Compiler.eval(Compiler.java:6931)
	at clojure.lang.Compiler.load(Compiler.java:7379)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5677.invoke(core.clj:5893)
	at clojure.core$load.invokeStatic(core.clj:5892)
	at clojure.core$load.doInvoke(core.clj:5876)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invokeStatic(core.clj:5697)
	at clojure.core$load_one.invoke(core.clj:5692)
	at clojure.core$load_lib$fn__5626.invoke(core.clj:5737)
	at clojure.core$load_lib.invokeStatic(core.clj:5736)
	at clojure.core$load_lib.doInvoke(core.clj:5717)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:648)
	at clojure.core$load_libs.invokeStatic(core.clj:5774)
	at clojure.core$load_libs.doInvoke(core.clj:5758)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:648)
	at clojure.core$require.invokeStatic(core.clj:5796)
	at clojure.core$require.doInvoke(core.clj:5796)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval1.invokeStatic(aot.clj:1)
	at user$eval1.invoke(aot.clj:1)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.load(Compiler.java:7379)
	at clojure.lang.Compiler.loadFile(Compiler.java:7317)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$script_opt.invokeStatic(main.clj:335)
	at clojure.main$script_opt.invoke(main.clj:330)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.RuntimeException: Unable to find static field: ES5_STRICT_UNCOMMON in class com.google.javascript.jscomp.DiagnosticGroups
	at clojure.lang.Util.runtimeException(Util.java:221)
	at clojure.lang.Compiler.analyzeSymbol(Compiler.java:7061)
	at clojure.lang.Compiler.analyze(Compiler.java:6648)
	... 47 more


 Comments   
Comment by António Nuno Monteiro [ 06/Feb/17 6:00 PM ]

Attached patch with fix.

Comment by David Nolen [ 06/Feb/17 6:05 PM ]

fixed https://github.com/clojure/clojurescript

Comment by Mike Fikes [ 06/Feb/17 6:09 PM ]

Confirmed that with the patch I can build locally and tests pass.





[CLJS-1929] When expanding libs don't include Hidden files Created: 04/Feb/17  Updated: 06/Feb/17  Resolved: 06/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1929.patch    

 Description   

When the compiler expands libs that are directories it is including hidden files. The will cause builds to break when editors like Emacs make temp files in the src directories.



 Comments   
Comment by Bruce Hauman [ 04/Feb/17 2:03 PM ]

Providing a simple solution that avoids Hidden files.

Comment by David Nolen [ 06/Feb/17 3:23 PM ]

fixed https://github.com/clojure/clojurescript/commit/182ad1621e2bc4293098dde2f214f5aac4091e75





[CLJS-1905] Self-host: Stacktraces for script/test-self-parity Created: 25/Jan/17  Updated: 06/Feb/17  Resolved: 06/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1905-2.patch     Text File CLJS-1905.patch    
Patch: Code

 Description   

If script/test-self-parity fails, you currently only get a prn of the error, which makes it difficult to locate the cause of the error. Add some infrastructure to print stacktraces.



 Comments   
Comment by Mike Fikes [ 05/Feb/17 12:29 PM ]

Previous patch no longer applies; attaching re-baselined patch.

Comment by David Nolen [ 06/Feb/17 3:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/c4348d4a8187062b3f0023f5256d2f19d9371474





[CLJS-1795] Support more options in the `:closure-warnings` compiler option Created: 28/Sep/16  Updated: 06/Feb/17  Resolved: 06/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1795.patch    
Patch: Code

 Description   

Support more options in the `:closure-warnings` compiler option as per https://github.com/google/closure-compiler/wiki/Warnings#warnings-categories



 Comments   
Comment by António Nuno Monteiro [ 05/Feb/17 10:28 PM ]

Attached patch with fix.

Comment by David Nolen [ 06/Feb/17 3:21 PM ]

fixed https://github.com/clojure/clojurescript/commit/74f342c3f01e448852b80f0d87675710341f5b4a





[CLJS-1902] Add support for compiler option :inline-source-maps Created: 24/Jan/17  Updated: 06/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 1
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



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:09 PM ]

Patches:
https://github.com/darwin/clojurescript/commit/de1da63072c8049b9812d41cce750e5a972c0b0b.patch
https://github.com/darwin/clojurescript/commit/2389fb7839a18299d842d381b135a669a7091869.patch
https://github.com/darwin/clojurescript/commit/3ba594d8c7215ef8c8276737b7e5d8008c6b3a98.patch

Comment by Antonin Hildebrand [ 24/Jan/17 8:27 PM ]

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

Comment by Antonin Hildebrand [ 25/Jan/17 10:25 AM ]

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.

Comment by Dusan Maliarik [ 06/Feb/17 10:00 AM ]

This would be helpful for us as well.





[CLJS-1923] Optimization causes TypeError not a function with a simple three.js dependency Created: 01/Feb/17  Updated: 06/Feb/17  Resolved: 06/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Plumpton Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: cljs, compiler
Environment:

Ubuntu 16.10, Clojurescript 1.9.456


Attachments: GZip Archive problem.tar.gz    

 Description   

I've created a simple test case that creates a three.js renderer and calls one function on loading (.setSize renderer 200 200)

When building with "lein cljsbuild once dev" everything works when the browser loads index.html.

But with "lein cljsbuild once min" an error occurs on loading:
"Uncaught TypeError: (intermediate value).bc is not a function"



 Comments   
Comment by David Plumpton [ 04/Feb/17 6:17 PM ]

Sorry, misunderstanding of externs behaviour, please close.





[CLJS-1927] Restore ability to refer to foreign libs placed within project-local classpath roots Created: 02/Feb/17  Updated: 04/Feb/17  Resolved: 04/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Duplicate Votes: 0
Labels: None

Attachments: Text File CLJS-1927.patch    
Patch: Code

 Description   

https://github.com/clojure/clojurescript/commit/97d2d61e78ce747d02d0e5b2ced706f6fb68ec4e changed how relative paths for foreign lib destinations are determined, but made it so that the compiler (at least in some cases) cannot accommodate foreign libs that are defined and sourced from project-local classpath roots (directories, not jars).

This was originally reported and hashed around starting here: https://github.com/bhauman/lein-figwheel/issues/516#issuecomment-275818898

The most minimal case of this I could construct is here: https://github.com/cemerick/figwheel-516 Oddly, bhauman wasn't able to replicate that failing project; even worse, this failure only affects figwheel, not cljsbuild. We've not been able to figure out why that is, but there is one other person on that figwheel issue thread that hit the same problem.

The attached patch does fix my original project's figwheel usage, the minimal case repo I linked above in my testing, and the other contributor's usage in the figwheel issue thread. The change treats file URLs the same as {{java.io.File}}s with regard to calculating relative paths, which AFAICT is most appropriate for the stated objective of the function in question.



 Comments   
Comment by António Nuno Monteiro [ 02/Feb/17 12:03 PM ]

This is probably related to CLJS-1922. We should come up with a solution that solves both problems.

Comment by Chas Emerick [ 02/Feb/17 12:19 PM ]

Yes, CLJS-1922 looks like a related if not equivalent problem. I wish I had found that before filing this.

Comment by David Nolen [ 03/Feb/17 1:39 PM ]

Please check that CLJS-1922 doesn't solve this problem. It's been applied to master.

Comment by Chas Emerick [ 04/Feb/17 7:40 PM ]

Looks good here with CLJS master, closing as dupe of CLJS-1922. Thanks!





[CLJS-1922] uti/relative-name presumes that local foreign-lib files are under working directory which is not true on Boot Created: 30/Jan/17  Updated: 03/Feb/17  Resolved: 03/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Cljs 1.9.456, Boot-cljs


Attachments: Text File CLJS-1922-1.patch    

 Description   

cljs.util/relative-name presumes that foreign-libs are located under working directory.

This is not true on Boot with local deps.cljs and foreign-libs. In Boot files from source/resource-paths are copied into temp directories (outside of working directory), and classpath contains these temp directories.

I'm investigation two solutions:

Improve relative-name to return paths relative to a classpath directory under which the file is found. Path relative to a classpath directory should be unique enough for output path, as foreign library contents are anyway read from classpath (unless I'm mistaken).

Another solution I'll check, is why the paths need to be relativized, in most cases foreign-lib `:file` value is relative.



 Comments   
Comment by Juho Teperi [ 30/Jan/17 7:07 PM ]

Solution using :file as output path if it is set and is relative. Else falls back to the current behavior.

Comment by David Nolen [ 03/Feb/17 1:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/f7f2142d44a2bddbb74bdabaf45ba032ad452c53





[CLJS-1831] Self-host: Improperly munge ns names Created: 22/Oct/16  Updated: 03/Feb/17  Resolved: 03/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1831-2.patch     Text File CLJS-1831-3.patch     Text File CLJS-1831.patch    

 Description   

If you have a namespace containing a reserved JavaScript keyword, in self-hosted ClojureScript the namespace isn't properly munged for the goog.provide call. A result is that symbols defined in the namespace will be attached to a nonexistent JavaScript object.

To reproduce, add a test namespace, say static.core that does nothing but have an assert that (= 1 1).



 Comments   
Comment by Mike Fikes [ 22/Oct/16 1:04 PM ]

Without the prod code fixes in cljs.js, a new unit test fails, producing the desired warning but then tripping up on attempting to attach to an undefined JavaScript static$ object, with the root cause being a goog.provide('static.core-test') call instead of the needed goog.provide('static$.core-test') as is done in regular ClojureScript:

WARNING: Namespace static.core-test contains a reserved JavaScript keyword, the corresponding Google Closure namespace will be munged to static$.core_test at line 1 src/test/cljs/static/core_test.cljs
#error {:message "ERROR in file src/test/cljs/static/core_test.cljs", :data {:tag :cljs/analysis-error}, :cause #object[ReferenceError ReferenceError: static$ is not defined]}

The production fix is to simply call cljs.compiler/munge instead of cljs.core/munge.

Comment by Mike Fikes [ 16/Dec/16 2:50 PM ]

Previous patch no longer applies. Attaching CLJS-1831-2.patch

Comment by Mike Fikes [ 02/Feb/17 10:04 PM ]

The previous patch no longer applies; attaching re-baselined patch.

Comment by David Nolen [ 03/Feb/17 1:34 PM ]

fixed https://github.com/clojure/clojurescript/commit/c28e41ae6462fdf3931ec9458d8739bc493f54ba





[CLJS-1868] CLJS compilation error with Closure lib dependency on Windows Created: 11/Dec/16  Updated: 03/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Dejan Josifovic Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, cljs, compiler
Environment:

Tested on Windows 10 with java 8 and on windows 7 with java 7 and java 8.
Clojure version is 1.8.0 on both systems.


Attachments: Text File CLJS-1868.patch     Text File CLJS-1868.patch     Text File stacktrace.txt    

 Description   

I encountered this error while developing a cljs project that included openlayers lib in its deps.
Open layers is a Closure library and I used the latest version 3.15.1.

The error that compiler reports is:
java.io.IOException: The filename, directory name, or volume label syntax is incorrect

I have recreated the minimal scenario, the one like on ClojureScript Quick start guide.
Steps to reproduce the problem (basically like the quick start):

  • Download standalone cljs jar from the link in the guide
  • Create the same build.cljs file as defined on the guide
  • Download openlayers-3.15.1.jar and place it in the root
  • Add require statement in core.cljs - (ns hello-world.core (:require ol.Map)) for example
  • Than in cmd run java -cp "cljs.jar;openlayers-3.15.1.jar;src" clojure.main build.clj

I have also tested with cljs master branch (created an uberjar and tested again) and i get the same error.

Attached is the example stacktrace (in that run I added :verbose true to get the 'Copying...' output).



 Comments   
Comment by Dejan Josifovic [ 11/Dec/16 8:06 AM ]

I have also created a git hub repo for this problem: https://github.com/28/openlayers-cljs-compile-error-repo

Comment by Gijs Stuurman [ 12/Dec/16 4:22 AM ]

This issue is present on all OS's, but breaks on Windows. I see it on Ubuntu.

The logic that already exists to shorten filenames for files from jars is not applied for the library in the openlayer.jar.

In verbose mode there is this output:

Copying jar:file:/path/openlayers/openlayers-cljs-compile-error-repo/openlayers-3.15.1.jar!/cljsjs/openlayers/development/ol/events/event.js to out/file:/path/openlayers/openlayers-cljs-compile-error-repo/openlayers-3.15.1.jar!/cljsjs/openlayers/development/ol/events/event.js

The file that gets written into the "out" directory contains a colon (:) and an exclamation point (!) in the path (at the "file:" and "jar!"). On Windows this is a Java IO Exception, because colons are not allowed in file names or paths.

The generated "main.js" file contains:

goog.addDependency("../file:/path/openlayers/openlayers-cljs-compile-error-repo/openlayers-3.15.1.jar!/cljsjs/openlayers/development/ol/events/event.js", ['ol.events.Event'], []);

The "openlayers" jar is a Google Closure compatible JavaScript library in a jar. All its files have a "goog.provides" and "goog.require"'s and the jar contains a "deps.cljs" with the keys :libs and :externs.

The following monkey patch in "build.clj" makes paths without "file:" or "jar!" in them:

(alter-var-root #'cljs.closure/rel-output-path
                (fn [orig-fn]
                  (fn [js & args]
                    (apply orig-fn (dissoc js :closure-lib) args))))

For comparison, the verbose output for the files generated for files from cljs.jar:

Copying jar:file:/path/openlayers/openlayers-cljs-compile-error-repo/cljs.jar!/goog/base.js to out/goog/base.js
Comment by David Nolen [ 12/Dec/16 9:25 AM ]

Gijs thanks for the additional information. Very helpful, will take a look.

Comment by David Nolen [ 16/Dec/16 2:02 PM ]

The suggested fix isn't desirable as far as I can tell since we have a branch specifically for dealing with Closure libs that will be avoided. We need a better explanation.

Comment by Dejan Josifovic [ 03/Feb/17 6:54 AM ]

Hi all,

the problem occurred only for Closure libraries compilation (such as openlayers).
As already commented, it manifested on both Win and Linux, but on Linux it did not
cause problems.

The problem was in the function that produced relative output paths for deps (lib-rel-path).
It tried to remove the first part of the absolute path with clojure.string/replace, but
it tried it with a wrong match (it did not reproduce the actual first part of the path)
so it effectively did nothing.

Example:
(str (io/file (System/getProperty "user.dir") lib-path) File/separator)
produced
file:\C:\Users&#42;***\Documents\Projects\openlayers-cljs-compile-error-repo\cljsjs\openlayers\development\
instead of
file:\C:\Users&#42;***\Documents\Projects\openlayers-cljs-compile-error-repo\openlayers-3.15.1.jar!\cljsjs\openlayers\development\

Attached patch produces better paths that work on both OS.
Can someone check if it is an okay solution?
I have tested it of course on both OS, run CLJS tests etc.

Thanks,
Dejan.

Comment by Dejan Josifovic [ 03/Feb/17 8:23 AM ]

This is the correct patch - first one contained an error.





[CLJS-1928] Self-host: Macro namespace alias in keyword Created: 02/Feb/17  Updated: 02/Feb/17

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1928.patch    
Patch: Code and Test

 Description   

If, in self-hosted ClojureScript you require a macro namespace and establish an alias, as in

(require-macros '[cljs.spec :as s])

You won't be able to use to use the alias in a keyword, as in

::s/foo

The root cause is that when r/*alias-map* is bound to (current-alias-map) this fails to set up mappings for macro namespaces.



 Comments   
Comment by Mike Fikes [ 02/Feb/17 9:27 PM ]

The attached patch revises (current-alias-map) to additionally consult macros namespaces when setting up the alias map. This is tested by adding the (previously-disabled) keyword unit tests, adding a specific test for a macros namespace alias.

(FWIW, a similar technique is also used in Lumo and Planck and, as a consequence, they can properly read forms involving macros namespace aliases, but the resulting forms cannot be compiled in those environments owing to the change needed in cljs.js).





[CLJS-1926] Changes to namespace metadata are not properly transferred to *ns* dynamic var Created: 02/Feb/17  Updated: 02/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A CLJS macro may want to access the metadata of the current ns via (meta *ns*).

Changes to the ns metadata are not reflected back the *ns* var when re-compiling a namespace, the metadata of the first compile will remain. This is due to the analyzer always calling create-ns but never updating the meta. It should probably be updated inside parse 'ns [1]. Clojure always resets the metadata via the ns macro.

One potential conflict is when a .clj and a .cljs file exist for the same namespace and both provide different metadata. Both platforms reseting the meta is probably not ideal, maybe we should vary-meta merge instead?

[1] https://github.com/clojure/clojurescript/blob/94b4e9cdc845c1345d28f8e1a339189bd3de6971/src/main/clojure/cljs/analyzer.cljc#L2312






[CLJS-1925] Use of undeclared Var cljs.user/RegExp when extending protocol for RegExp Created: 01/Feb/17  Updated: 01/Feb/17  Resolved: 01/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-1925.patch     Text File CLJS-1925-test-2.patch    
Patch: Code and Test

 Description   

WARNING: Use of undeclared Var schema.core/RegExp at line 356 /home/juho/.boot/cache/tmp/home/juho/Source/**/**/4i8/-uc8bn7/js/main.out/schema/core.cljs
WARNING: Use of undeclared Var bidi.bidi/RegExp at line 232 /home/juho/.boot/cache/tmp/home/juho/Source/**/**/4i8/-uc8bn7/js/main.out/bidi/bidi.cljc

https://github.com/juxt/bidi/blob/master/src/bidi/bidi.cljc#L230
https://github.com/plumatic/schema/blob/master/src/cljx/schema/core.cljx#L355

First commit with problem: https://github.com/clojure/clojurescript/commit/d961e66edfd3d62f80bed18418c597ee1f569e85

Test case by thheller:

(defprotocol X
  (x [x]))

(defprotocol Y
  (y [y]))

(extend-protocol X
  js/RegExp
  (x [x]
    (y x)))

(extend-protocol Y
  js/RegExp
  (y [y]
    :y))


 Comments   
Comment by António Nuno Monteiro [ 01/Feb/17 3:56 PM ]

Attached patch with fix and test.

As specified in the commit message, this is a regression caused by the improper fix to CLJS-1607, which this patch now properly fixes.

Comment by António Nuno Monteiro [ 01/Feb/17 3:59 PM ]

^ The patch I attached is CLJS-1925.patch. The other should be disregarded as it only includes a test.

Comment by David Nolen [ 01/Feb/17 4:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/8c9cde94cb05b3d5446c2e6182b55d6f29893031





[CLJS-1920] cljs.build.api/node-inputs: package.json files are only added if module entries are top-level Created: 30/Jan/17  Updated: 01/Feb/17  Resolved: 01/Feb/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1920.patch    
Patch: Code

 Comments   
Comment by David Nolen [ 01/Feb/17 10:29 AM ]

fixed https://github.com/clojure/clojurescript/commits/master





[CLJS-1924] The compiler cannot infer the target type of "(. RecordName -prototype)" expressions Created: 01/Feb/17  Updated: 01/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler, extern


 Description   

Repro:

Place

(set! *warn-on-infer* true)

(defrecord Foo [])

anywhere in your source files, compile with :infern-externs true.

Expected:

Multiple warnings like:

  • WARNING: Cannot infer target type in expression (. Foo -prototype)
  • WARNING: Cannot infer target type in expression (. other__8838__auto__ -constructor)
  • WARNING: Cannot infer target type in expression (. user/Foo -getBasis)

There are also warnings for (. cljs.core/List -EMPTY), but this might be unrelated.






[CLJS-1921] Certain files are compiled twice, leading to "Can't redefine a constant ..." errors Created: 30/Jan/17  Updated: 31/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

windows, linux



 Description   

Repro:

Create a folder with the following files:

cljs.jar (the quick start uberjar)
src
  foo.cljc
  core.cljc

The contents shall be:

;; foo.cljc
(ns foo)

(def ^:const a 10)

;; core.cljc
(ns core
  (:require [foo]))

(def ^:const a 10)

In this folder, call java -cp src;cljs.jar clojure.main. In the repl, enter:

(require '[cljs.build.api :refer [build]])
(build "src" {:output-to "out/main.js" :output-dir "out" :verbose true})

If some conditions are met (see [1] and the explanations below), you'll see the compilation fail with "Can't redefine a constant at <...>".

NOTE: the easiest way to reproduce is to sort the value returned by cljs.closure/add-dependency-sources by a function of our choosing, (comp first :provides) for example. This can ensure that the last element will be a dependency which appears twice (see explanations below). However, in this case we have to use a hand-crafted classpath, which includes our patched clojurescript jar (+ its dependencies, + the src folder).

Cause:

NOTE: My understanding of the compiler internals (cljs.js-deps and cljs.closure in particular) is very superficial, but those familiar with it can probably follow along.

The journey starts at cljs.closure/add-dependency-sources, invoked in cljs.closure/build. The function takes some dependencies, converts them to a set, and adds some dependencies to it. Things to note here:

  • the :file entry in each dependency is nil [0]
  • the order of (seq (set inputs)) is "random" (or unspecified rather) [1]
  • the :source-file entry in each dependency is a java.io.File instance
  • the :source-file entry in each additional dependency from cljs.closure/find-cljs-dependencies is a java.net.URL instance
  • there are some dependencies (in the repro case at least) which were in inputs and were added as well with cljs.closure/find-cljs-dependencies (so they appear twice in the list)

The fact that some dependencies are java.io.File, some are java.net.URL instances may be the reason why some items appear multiple times in the set, but I haven't confirmed this.

Next, the computed dependencies are fed into cljs.js-deps/dependency-order, where they are passed to cljs.js-deps/build-index. This function reduces over each dependency (OUTER reduce), and reduces over each provided namespace (:provides entry, INNER reduce).

Here's the crucial part: the INNER reduce associates each namespace in :provides with the dependency, and the OUTER reduce then associates the :file entry with the dependency. But, since the :file entries are nil (see [0]), the value mapped to nil is overridden with each step in the OUTER reduce. Now, let X be a dependency which appeared twice (or at least twice, does not matter) in inputs. If X happens to be the last element of the dependencies (depends on [1]), then in the final map (when the OUTER reduce finishes) X will be mapped to nil. Therefore, in the final index, X will be present twice (once mapped to nil, once mapped to the namespace it provides).

Afterwards, these dependencies are compiled, and when the stars align (= sometimes it happens, sometimes it doesn't, both with parallel and non-parallel build) the compilation will fail with "Can't redefine a constant at <...>". This is most likely due to the fact that certain dependency (X), which has a ^:const var, was compiled twice (and some namespaces are compiled twice, this can be observed when :verbose is set to true).

I could reproduce the error on both windows (locally), and on linux (on a travis-ci server).






[CLJS-1918] case needs a type hint for keywords case when using *warn-on-infer* Created: 30/Jan/17  Updated: 30/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1917] `case` doesn't handle matching against lists Created: 28/Jan/17  Updated: 28/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following works in Clojure but not ClojureScript

(let [foo '(:a :b)]
  (case foo
    '(:a :b) :works))





[CLJS-1916] __dirname and __filename are not defined when compiling for Node.js with optimizations :none Created: 28/Jan/17  Updated: 28/Jan/17  Resolved: 28/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1916.patch    
Patch: Code

 Comments   
Comment by David Nolen [ 28/Jan/17 2:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/ba205f0d091cd541c241a5d0e9419cbf3e4e8808





[CLJS-1864] timestamped source maps broken with Node Created: 29/Nov/16  Updated: 28/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Dmitr Sotnikov Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I'm using Figwheel with Node, and noticed a bug with timestamped source maps. When the :source-map-timestamp compiler flag is set, the sourceMappingURL is set to source.js.map?timestamp.

This works fine in the browser, but breaks in Node where files are loaded from the filesystem. It looks like a simple workaround would be to check if :target is :node and output something like source.js.timestamp.map instead, and use it directly as the value of sourceMappingURL.

Here's a change I made locally in cljs.compiler/emit-source-map that allows source maps to be resolved on Node when using timestamps:

emit-source-map
(defn emit-source-map [src dest sm-data opts]
     (let [timestamp (System/currentTimeMillis)
           filename (str (.getPath ^File dest)
                         (when (and
                                 (true? (:source-map-timestamp opts))
                                 (= (:target opts) :nodejs))
                           (str "." timestamp))
                         ".map")
           sm-file  (io/file filename)]
       (if-let [smap (:source-map-asset-path opts)]
         (emits "\n//# sourceMappingURL=" smap
                (string/replace (util/path sm-file)
                                (str (util/path (io/file (:output-dir opts))))
                                "")
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str
                    (if-not (string/index-of smap "?") "?" "&")
                    "rel=" timestamp)
                  ""))
         (emits "\n//# sourceMappingURL="
                (or (:source-map-url opts) (.getName sm-file))
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str "?rel=" timestamp)
                  "")))
       (spit sm-file
             (sm/encode {(url-path src) (:source-map sm-data)}
                        {:lines                   (+ (:gen-line sm-data) 2)
                         :file                    (url-path dest)
                         :source-map-path         (:source-map-path opts)
                         :source-map-timestamp    (:source-map-timestamp opts)
                         :source-map-pretty-print (:source-map-pretty-print opts)
                         :relpaths                {(util/path src)
                                                   (util/ns->relpath (first (:provides opts)) (:ext opts))}}))))


 Comments   
Comment by David Nolen [ 30/Nov/16 8:32 AM ]

Does Node.js have source map caching issues? The timestamp feature was created for caching issues present in web browsers.

Comment by Dmitr Sotnikov [ 30/Nov/16 8:39 AM ]

I tried it with :source-map-timestamp set to false, and source maps got out of sync when Cljs sources were reloaded.

Comment by David Nolen [ 30/Nov/16 10:01 AM ]

Okay. This issue will require more investigation into Node.js source mapping support before pursuing anything. As the behavior is understood, information should be added here.

Comment by Dmitr Sotnikov [ 30/Nov/16 2:56 PM ]

Sounds like a plan.

Comment by David Nolen [ 30/Nov/16 7:25 PM ]

OK I took a look at the implementation of source-map-support, it does indeed cache the source map. However the proposed idea here isn't comprehensive enough. We need to change all the places where :source-map-timestamp is used in the source code. Patch is welcome.

Comment by Dmitr Sotnikov [ 30/Nov/16 7:28 PM ]

Yeah, I noticed the key is used in a few places. I can definitely take a look at making a patch in the near future if the approach looks good to you.

Comment by Dmitr Sotnikov [ 28/Jan/17 12:39 PM ]

It looks like the approach of adding a timestamp introduces some problems. Generating unique file names would mean that old files have to be cleaned up somehow, since the new files will no longer overwrite them. Having to keep track of that isn't ideal. Perhaps it would be better to see if there's a way to prevent Node from caching the source maps.





[CLJS-1915] cljs.test: Index out of bounds for stack element w/o line/column Created: 28/Jan/17  Updated: 28/Jan/17  Resolved: 28/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1915.patch    
Patch: Code and Test

 Description   

The cljs.test/js-line-and-column function does not properly handle stack elements that have no line and column information encoded in them. In particular, if you pass in something like "eval@[native code]" (which can be part of a stack from JavaScriptCore), you will get an index out of bounds exception.



 Comments   
Comment by David Nolen [ 28/Jan/17 11:11 AM ]

fixed https://github.com/clojure/clojurescript/commit/2f8a1529955acc943ac8082ab5848b2cba54bc4d





[CLJS-1914] Investigate directly requiring CommonJS Node module into a ClojureScript namespace Created: 28/Jan/17  Updated: 28/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 28/Jan/17 10:20 AM ]

We would need to check that it exists. We would also need to parse out CommonJS requires to sort out the foreign libs inputs.





[CLJS-1913] Investigate slow reading / compilation of CLJC files Created: 27/Jan/17  Updated: 27/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1910] Self-host: Need to eliminate use of cljs.nodejs/require Created: 26/Jan/17  Updated: 27/Jan/17  Resolved: 27/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1910.patch    
Patch: Code

 Description   

Need to change this:

https://github.com/clojure/clojurescript/blob/master/src/test/self/self_parity/test.cljs#L56



 Comments   
Comment by António Nuno Monteiro [ 27/Jan/17 12:50 AM ]

The change from `require` to `nodeGlobalRequire` in this commit
https://github.com/clojure/clojurescript/commit/59a7f265fac02c931cc3d5615727da861db62e3b#diff-db201d3634488851510ee2fbce327924R75
means that loading every file will go through `vm.runInThisContext`.

The above causes `require` to not be bound, since `require` is a "free variable" and not a property of the global object (more context here https://github.com/nodejs/node-v0.x-archive/issues/9211#issuecomment-134412415).

A suggested fix is to add `global.require = require;` to `cljs/bootstrap_node.js`. I tested that fix and it worked.

Note that this is not only breaking the self-host tests. It will break every ClojureScript build which targets Node.js under `:none` optimizations, which should therefore be considered critical.

Comment by Mike Fikes [ 27/Jan/17 1:18 AM ]

The attached patch addresses (and only addresses) the primary issue in this ticket: The need to use js/require.

Given the critical and subtle nature of the side issue discovered and detailed in António's comment above, that issue is split off to CLJS-1911 in case some other more appropriate solution is found for it.

Comment by David Nolen [ 27/Jan/17 9:20 AM ]

fixed by removing the breaking change https://github.com/clojure/clojurescript/commit/18d1e5792f6c712b6c8e426ac9123ee4b898a374, CLJS-1911 was the root cause





[CLJS-1911] Need to bind Node.js require Created: 27/Jan/17  Updated: 27/Jan/17  Resolved: 27/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1911.patch    
Patch: Code

 Description   

While making an ostensibly simple change for CLJS-1910, updating from cljs.nodejs/require to js/require, it was discovered that Node was, initially inexplicably, indicating that require wasn't available at runtime.

Credit to António: He discovered the root cause, potential serious ramifications, and a potential patch (attached to this ticket). See more detailed commentary from António here: http://dev.clojure.org/jira/browse/CLJS-1910?focusedCommentId=44906&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-44906

The attached patch is sufficient to allow the patch attached to CLJS-1910 pass script/test-self-parity.

Given the subtleties surrounding this issue, this patch probably warrants review.



 Comments   
Comment by David Nolen [ 27/Jan/17 9:02 AM ]

fixed https://github.com/clojure/clojurescript/commit/35bcfce2565148cd96b31499a2bbc3bfc2257822





[CLJS-1909] Self-host: circular dependency when requiring cljs.reader Created: 26/Jan/17  Updated: 26/Jan/17  Resolved: 26/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1909.patch    
Patch: Code

 Description   

`cljs.analyzer` -> `cljs.reader` -> `cljs.reader$macros` -> `cljs.analyzer`



 Comments   
Comment by David Nolen [ 26/Jan/17 11:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/7de568f7c80ce5f2a41222063251f0ecf688c3e2





[CLJS-1906] Self-host: script/test-self-parity fails Created: 25/Jan/17  Updated: 26/Jan/17  Resolved: 26/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1906.patch    

 Description   

With master as of the writing of this ticket

script/test-self-parity fails with:

#error {:message "Could not parse ns form cljs.clojure-alias-test", :data {:tag :cljs/analysis-error}, :cause #object[Error Error: No protocol method IDeref.-deref defined for type null: ]}


 Comments   
Comment by David Nolen [ 26/Jan/17 10:10 PM ]

fixed https://github.com/clojure/clojurescript/commit/f4e057c7742861d48568b776024d142f68143fac





[CLJS-1908] Improve error messages by using pr-str instead of str when printing objects Created: 26/Jan/17  Updated: 26/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   

Many error messages from ClojureScript include the invalid argument like this:

(throw (js/Error. (str "Doesn't support name: " x)))

If x is nil, then the error message produces is "Doesn't support name: " which is a bit mystifying to debug. If x was wrapped with pr-str then the error message would be the much more understandable: "Doesn't support name: nil".

If there's interest in this, then I can prepare a patch which wraps these kinds of errors with pr-str.






[CLJS-1907] Improve error message from cljs.reader/read-string when reading keyword with number first (e.g. :0seconds) Created: 26/Jan/17  Updated: 26/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File CLJS-1907.patch    

 Description   

In read-keyword, if the keyword doesn't match symbol-pattern then re-matches* returns nil, and then the aget for token throws an error about null not being an object. The provided patch checks if the keyword matches, and if not throws an error with a descriptive error message that the keyword is not valid.

This hit our team when we were trying to deserialise some EDN, and we had keywords with leading numbers in them.

(defn read-keyword
  [reader initch]
  (let [token (read-token reader (read-char reader))
        a (re-matches* symbol-pattern token) ;; returns nil when token is `0s`
        token (aget a 0) ;; throws from aget on nil
        ns (aget a 1)
        name (aget a 2)]
    (if (or (and (not (undefined? ns))
                 (identical? (. ns (substring (- (.-length ns) 2) (.-length ns))) ":/"))
            (identical? (aget name (dec (.-length name))) ":")
            (not (== (.indexOf token "::" 1) -1)))
      (reader-error reader "Invalid token: " token)
      (if (and (not (nil? ns)) (> (.-length ns) 0))
        (keyword (.substring ns 0 (.indexOf ns "/")) name)
        (keyword token)))))


 Comments   
Comment by Daniel Compton [ 26/Jan/17 6:05 PM ]

I don't love using the _ side effect in the let binding, but the alternative makes the code a bit uglier and less readable.





[CLJS-1444] Node.js shim requires `node` invocation to be in the same directory as shim script Created: 10/Sep/15  Updated: 26/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Don't have time to provide a proper repro now but the basic issue can be illustrated by this:

~/c/boot-cljs-example (master=) node target/main.js
module.js:338
    throw err;
          ^
Error: Cannot find module '/Users/martin/code/boot-cljs-example/out/goog/bootstrap/nodejs.js'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/martin/code/boot-cljs-example/target/main.js:6:1)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
~/c/boot-cljs-example (master=) cd target/
~/c/b/target (master=) node main.js
Starting...

This is compiled with boot because that was what I had at hand right now. The compiled shim looks like this:

var path = require("path");
try {
    require("source-map-support").install();
} catch(err) {
}
require(path.join(path.resolve("."),"out","goog","bootstrap","nodejs.js"));
require(path.join(path.resolve("."),"out","cljs_deps.js"));
goog.global.CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"nodejs"};
goog.require("boot.cljs.main");
goog.require("cljs.nodejscli");

The problem here is that path.resolve(".") will return the directory the node command was invoked in and not the directory of the shim. (See the "Cannot find module..." error above)

A solution could be to use __dirname which always resolves to the directory of the current file. This might result in some breakage for existing setups.



 Comments   
Comment by Sebastian Bensusan [ 14/Oct/15 11:31 AM ]

I have a proposed solution but I fear that supporting "run from anywhere" adds essential complexity to the resulting code. My thought process:

1. Relative paths are desirable to produce "context free code." If the user chooses absolute paths, then that behavior is respected and there's nothing to handle (no "path algebra") .

2. When dealing with relative paths, the whole system needs to establish a "frame of reference", a root path. The ClojureScript compiler assumes the path from which it is compiling to be that frame of reference, which usually coincides with the top root of the project. Though arbitrary, it is the only choice that makes sense.

3. The frame of reference is not explicit anywhere in the code, since it is defined as ".". If it were explicit, it would reveal context, as in "/home/some-user/their-folder/this-project/".

4. When we approach the code from another reference point (executing the script from another directory), we first need to find the original compiler path (reference point,) and then resolve all paths from there. The compiler uses `cljs.closure/path-relative-to` for this purpose.

Path algebra:
compiler-path = __dirname - output-to

Node.js

var compiler-path = __dirname.replace(output-to, "")
path.resolve (compiler-path, output-dir, "goog", "base.js")
path.resolve (compiler-path, output-dir, "cljs_deps.js")

which assumes that if output-to was given relatively, then output-dir is also relative. If they are not in sync, more work needs to be done to keep them that way.

It's not up to me to decide if the extra complexity is worth the use-case. I actually hope there is a simpler solution to solve this that I'm not seeing.

Comment by Karol Majta [ 14/Mar/16 10:43 AM ]

I find this behavior really weird and would opt for switching to __dirname. I am also not sure i fully understand consequences of such switch (I have little cljs experience, speaking more from the perspective of nodejs user). My point is: current behavior renders clojurescript hard to use for commandline and desktop applications (namely electron).

For command line and desktop applications assumptions about CWD cannot be made. For now i run my applications through a bootstrap script JS script:

process.chdir(__dirname);
require('./target/out');

I am lucky that my code does not have to use the real CWD, but it's a hack more than a real solution.

Speaking from nodejs perspective:

1. Using absolute paths is considered a bad practice anyway.
2. Nodejs programs that don't use external packages (don't depend on node_modules) can be run from any CWD
3. Nodejs programs that do depend on node_modules will break if run from a different directory than the one containing node_modules, but this is expected behavior.

Comment by J. Pablo Fernández [ 26/Jan/17 5:42 AM ]

I just run into this problem when trying to develop an Electron application. The way it's working right now is essentially unpackageable. I think it would be nice to have this behavior even as an option and I'm happy to work on a patch.

Comment by J. Pablo Fernández [ 26/Jan/17 5:57 AM ]

As far as I can see, this is the relevant code:

https://github.com/clojure/clojurescript/blob/cdaeff298e0f1d410aa5a7b6860232270d287084/src/main/clojure/cljs/closure.clj#L1410-L1411

Comment by J. Pablo Fernández [ 26/Jan/17 6:15 AM ]

A potential workaround seems to use :simple optimization.





[CLJS-1841] Check lib folder before compiling with cljsc, check for compilation success before running tests Created: 06/Nov/16  Updated: 25/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1841.patch     Text File CLJS-1841v2.patch    
Patch: Code

 Description   

If script/bootstrap hasn't been run, then running script/test returns the slightly cryptic error message:

Error: Could not find or load main class clojure.main

This patch checks if the lib folder exists and has files in it. If not it prompts the user to run script/bootstrap and exits.

This patch also adds a check for compilation success before running tests against the various VM engines.



 Comments   
Comment by Daniel Compton [ 25/Jan/17 5:57 PM ]

Add v2 patch.





[CLJS-1903] Remove anonymous vars from dir and apropos output Created: 24/Jan/17  Updated: 25/Jan/17  Resolved: 25/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1903.patch    
Patch: Code

 Description   

Anonymous vars (associated with reify gensyms) appear in dir output. For example, the trailing end of (dir cljs.spec) looks like:

...
spec-impl
spec?
t_cljs$spec3750
t_cljs$spec3780
t_cljs$spec3821
t_cljs$spec3836
t_cljs$spec3854
t_cljs$spec3867
t_cljs$spec3874
t_cljs$spec3970
t_cljs$spec3973
t_cljs$spec3984
t_cljs$spec3987
t_cljs$spec4003
t_cljs$spec4253
t_cljs$spec4256
t_cljs$spec4266
t_cljs$spec4272
tuple-impl
unform
unform*
valid?
with-gen
with-gen*

(For comparison, items like these don't appear in the Clojure REPL for dir output.)

This ticket requests that these be removed from dir as well as apropos as being noise / distraction.



 Comments   
Comment by Mike Fikes [ 24/Jan/17 11:03 PM ]

In addition to the example in the description, with the attached patch:

cljs.user=> (apropos spec)
(cljs.core/special-symbol? cljs.core/specify cljs.core/specify! 
cljs.spec/*fspec-iterations* cljs.spec/->t_cljs$spec3750 
cljs.spec/->t_cljs$spec3780 cljs.spec/->t_cljs$spec3821 
cljs.spec/->t_cljs$spec3836 cljs.spec/->t_cljs$spec3854 
cljs.spec/->t_cljs$spec3867 cljs.spec/->t_cljs$spec3874 
cljs.spec/->t_cljs$spec3970 cljs.spec/->t_cljs$spec3973 
cljs.spec/->t_cljs$spec3984 cljs.spec/->t_cljs$spec3987 
cljs.spec/->t_cljs$spec4003 cljs.spec/->t_cljs$spec4253 
cljs.spec/->t_cljs$spec4256 cljs.spec/->t_cljs$spec4266 
cljs.spec/->t_cljs$spec4272 cljs.spec/and-spec-impl 
cljs.spec/fspec-impl cljs.spec/get-spec cljs.spec/map-spec 
cljs.spec/map-spec-impl cljs.spec/merge-spec-impl 
cljs.spec/multi-spec-impl cljs.spec/or-spec-impl 
cljs.spec/regex-spec-impl cljs.spec/spec-impl cljs.spec/spec? 
cljs.spec/t_cljs$spec3750 cljs.spec/t_cljs$spec3780 
cljs.spec/t_cljs$spec3821 cljs.spec/t_cljs$spec3836 
cljs.spec/t_cljs$spec3854 cljs.spec/t_cljs$spec3867 
cljs.spec/t_cljs$spec3874 cljs.spec/t_cljs$spec3970 
cljs.spec/t_cljs$spec3973 cljs.spec/t_cljs$spec3984 
cljs.spec/t_cljs$spec3987 cljs.spec/t_cljs$spec4003 
cljs.spec/t_cljs$spec4253 cljs.spec/t_cljs$spec4256 
cljs.spec/t_cljs$spec4266 cljs.spec/t_cljs$spec4272)

becomes much more useful:

cljs.user=> (apropos spec)
(cljs.core/special-symbol? cljs.core/specify cljs.core/specify! 
cljs.spec/*fspec-iterations* cljs.spec/and-spec-impl 
cljs.spec/fspec-impl cljs.spec/get-spec cljs.spec/map-spec 
cljs.spec/map-spec-impl cljs.spec/merge-spec-impl 
cljs.spec/multi-spec-impl cljs.spec/or-spec-impl 
cljs.spec/regex-spec-impl cljs.spec/spec-impl cljs.spec/spec? 
cljs.spec/specize*)
Comment by David Nolen [ 25/Jan/17 5:01 PM ]

fixed https://github.com/clojure/clojurescript/commit/a64ab2c975bcc000fd10b4b485b4ba4259e70276





[CLJS-1904] Reload support for JS Modules Created: 25/Jan/17  Updated: 25/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Process modules currently works by changing transforming compiler opts. This means the resulting compiler opts cannot be used to run JS module processing again. This issue is now clear in cljs/repl.cljc. The other issue is that JS module processing is the only time that ClojureScript is involved in the compilation of JS sources - prior to this we never to need to trigger JS compilation ourselves. Thus require + :reload doesn't work on JS namespaces.






[CLJS-1453] cljs.compiler/load-libs does not preserve user expressed require order Created: 17/Sep/15  Updated: 25/Jan/17  Resolved: 25/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-1453-1.patch    

 Description   

Due to putting the requires into a map the original order is lost. This is a problem primarily when order specific side effects are present in the required namespaces.



 Comments   
Comment by Angus Fletcher [ 09/Jan/17 5:08 PM ]

This patch changes deps in parse 'ns and parse 'ns* to be a vector, which gives us deterministic ordering in load-libs.

Comment by Angus Fletcher [ 09/Jan/17 5:36 PM ]

Patch, updated to remove whitespace changes.

Comment by David Nolen [ 25/Jan/17 3:19 PM ]

fixed https://github.com/clojure/clojurescript/commit/94b4e9cdc845c1345d28f8e1a339189bd3de6971





[CLJS-1792] Can't load clojure.spec.test when clojure.test.check is unavailable Created: 23/Sep/16  Updated: 25/Jan/17

Status: Reopened
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Arne Brasseur Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:
Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
[org.clojure/clojure "1.9.0-alpha12"]
[org.clojure/clojurescript "1.9.229" :scope "provided"]


 Description   

Requiring clojure.spec.test results in an error, because it's looking for clojure.test.check.

(ns foo.bar
  (:require [clojure.spec.test]))
Caused by: clojure.lang.ExceptionInfo: No such namespace: clojure.test.check, could not locate clojure/test/check.cljs, clojure/test/check.cljc, or Closure namespace "clojure.test.check" in file file:/home/arne/.m2/repository/org/clojure/clojurescript/1.9.229/clojurescript-1.9.229.jar!/cljs/spec/test.cljs {:tag :cljs/analysis-error}

This problem goes away when adding org.clojure/test.check as a dependency.

This is not an issue in Clojure. An exception is only raised when calling a function that relies on test.check.



 Comments   
Comment by David Nolen [ 30/Sep/16 11:41 AM ]

This is not a bug per se, we can't do what Clojure does here. How to best handle is something to consider. Present a good idea and submit a patch.

Comment by Andrea Richiardi [ 24/Jan/17 4:46 PM ]

I went through cljs.spec.test and I have noticed that basically the [clojure.test.check :as stc] dependencies is there only for using the namespace for stc/ret and others. We could get rid of it and use the ns explicitely.

There is another require, which is [clojure.test.check.properties]. My guess is that it is there because of some side-effect (maybe it registers some spec?). I need some enlightenment here but I could work on a cut-off





[CLJS-1402] Source Mapping Closure Error Logger Created: 08/Aug/15  Updated: 24/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: type-check


 Description   

Current error reports generated by Google Closure point back to the generated JavaScript sources. For JavaScript source that originated from ClojureScript we should generated source mapped reports.



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:06 PM ]

I believe this will be fixed by CLJS-1901 either using `--source_map_input` or inlining source-maps into generated js files (CLJS-1902).





[CLJS-1901] Investigate new Google Closure source mapping support Created: 24/Jan/17  Updated: 24/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Google Closure now contains comprehensive support for (at least from the command line) for source map merging and inline source map generation. We should investigate how reusable this functionality actually is.



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 3:30 PM ]

Investigated it a bit, just sharing what I learned so far:

1. historically there used to be a hidden flag `--source_map_input` which could be used to produce source-map-aware error reporting, not source map composition as name would suggest[1]
2. mid 2016, a patch landed[2], which enhanced this for full source map composition
3. by the end of 2016, the feature seems to be public and enabled in command-line tool by default[3][5]
4. as of today, the official source-maps wiki page[4] has not been updated to reflect this latest development

[1] https://github.com/google/closure-compiler/issues/1360#issuecomment-170716968
[2] https://github.com/google/closure-compiler/pull/1971
[3] https://github.com/google/closure-compiler/pull/2008
[4] https://github.com/google/closure-compiler/wiki/Source-Maps
[5] https://github.com/google/closure-compiler/pull/2129

Comment by Antonin Hildebrand [ 24/Jan/17 3:53 PM ]

Closure compiler also newly understands inlined source maps using data URLs in input Javascript files[1].

1. parsing of inline source maps is enabled by default unless `--parse_inline_source_maps=false` is passed, it is independent on `--source_map_input` flag
2. information from `--source_map_input` and inlined source-maps is merged, inlined maps override `--source_map_input`, the last inlined map wins in case of multiple //# sourceMappingURL=<data URL> present [2]

[1] https://github.com/google/closure-compiler/pull/1982
[2] https://github.com/google/closure-compiler/pull/1982#issuecomment-243249065





[CLJS-1898] JSValue should support metadata to fix source maps contents edge case Created: 23/Jan/17  Updated: 24/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1898.patch     Text File CLJS-1898.patch    
Patch: Code and Test

 Description   

Dirac DevTools REPL relies on proper source-maps[1]. When I type "#js {}" into the REPL prompt, evaluation works, but generated javascript code snippet has wrong source map. Associated source-map's sourcesContent is something like #object[cljs.tagged_literals.JSValue 0x34d692f3 "cljs.tagged_literals.JSValue@34d692f3"]. And it is not what user entered. And this is not a valid cljs source I could parse again for code-completions.

The problem is somewhat subtle. When generating source-map info, the REPL code given a form tries to look at :source metadata[2] and if not present, it simply prints the form using pr. We get Clojure-style printing because JSValue does not implement ClojureScript printing as reported in CLJS-733.

Instead of implementing the rejected idea from CLJS-733. I focused on the first-case code path with :source metadata. It turned out that :source gets added if given form supports metadata protocols. Easiest idea was to change JSValue from deftype to defrecord.

I confirm that this fixed the REPL issue in Dirac. Now I'm getting back the original source code as entered by user.

Note that similar issue is present with #uuid and #inst tags as well. Those mostly work because their printer is compatible with both Clojure and ClojureScript. But fundamentally user might get back different source-map source than entered into the REPL. For example whitespace will be missing. This might cause confusion because line/column information reported by reader which might not match this artificial source-map content generated by printing.

I believe a fix would be to wrap UUID and Date forms created by tagged literals into something similar to JSValue and work with this wrapped value instead. Same with #queue. This would make cljs.tagged-literal code consistent, because every form originated from data readers would be inside a wrapper. It would enable arbitrary metadata on wrappers.

As a side note:
I noticed that reader's metadata get lost. They are not transplanted from form produced by the reader to generated tagged literal forms.
My second goal was to copy reader's metadata. Unfortunately this wasn't possible with current implementation of tools.reader. I can get reader metadata from passed form, but that is not the full picture, it only describes content after the tag was already consumed. I would need metadata from tag itself, passed into data-reader call somehow[3].

I don't know about any real-world problem caused by missing reader metadata on tagged literals, but I think ideally it should be fixed. For example when implementing error-reporting in cljs-oops, I had to handle this edge case[4], because line/column metadata was missing in some edge cases.

[1] https://github.com/binaryage/dirac/releases/tag/v0.8.0
[2] https://github.com/clojure/clojurescript/blob/960bb9b778190aa7359acb2f74cc61d452cef2ae/src/main/clojure/cljs/repl.cljc#L476
[3] https://github.com/clojure/tools.reader/blob/3f36a18a6c5d53a4fc718822131ee75625fd44dc/src/main/cljs/cljs/tools/reader.cljs#L834
[4] https://github.com/binaryage/cljs-oops/blob/d530a3cdf8cbab39bd2699c36caded4414224c50/src/lib/oops/reporting.clj#L29



 Comments   
Comment by Antonin Hildebrand [ 23/Jan/17 5:58 PM ]

I just noticed that some tests broke because of the defrecord change. My original implementation was using deftype with clojure.lang.IObj implementation. But it had a shortcoming of reusing val as the target for holding metadata. Which might not be IObj in general case. And also a separate implementation would have to be done for bootstrapped. Also I didn't want to add another field into deftype. I will leave it open for your suggestion how to properly implement this.

Comment by Antonin Hildebrand [ 24/Jan/17 2:28 PM ]

Another stab at it - this time I rely on the fact that val field must be a vector or a map. So it can carry metadata for JSValue without changing adding new fields to deftype.

Tests passing.





[CLJS-1900] Source maps for processed JavaScript modules Created: 24/Jan/17  Updated: 24/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently we don't emit source maps for JS modules converted into Google Closure namespaces.






[CLJS-1899] Local bindings conflict with global JS namespace Created: 24/Jan/17  Updated: 24/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Not sure if it's a bug or expected behaviour, but this:

(defn test-fn []
  (let [href     js/location.href
        location "123"]
    href))

gets compiled to this (not in advanced mode):

cognician.chat.ui.pages.insights.test_fn = (function cognician$chat$ui$pages$insights$test_fn(){
var href = location.href;
var location = "123";
return href;
});

and local location var shadows global I'm trying to access in location.href.

That sort of thing is expected and one should pay attention and work around stuff like this in JS, but in CLJS it's very confusing because nothing hints what am I doing wrong and why that code fails. I remember one of ClojureScript goals was to fix JS semantics, so maybe there's a way this might be addressed? At least throw a warning, maybe?



 Comments   
Comment by Thomas Heller [ 24/Jan/17 5:04 AM ]

This came up recently on the #cljs-dev slack channel. There is definitely a bug somewhere.

(let [href     js/location.href
      location "123"]
  href)

produces

var href_51444 = location.href;
var location_51445 = "123"; // << correct

So it works at the top level, but when inside a defn (and others) we get

(ns test)
(defn test-fn []
  (let [href     js/location.href
        location "123"]
    href))
test.test_fn = (function test$test_fn(){
var href = location.href;
var location = "123"; // << incorrect
return href;
});
Comment by David Nolen [ 24/Jan/17 7:27 AM ]

Taking a quick look it seems that maybe we aren't checking `:js-globals` consistently and often only looking at locals? Also now that externs inference is a thing we should probably compute `:js-globals` from all known externs instead of the obviously incomplete list we currently have in place.





[CLJS-1897] Too many externs generated Created: 23/Jan/17  Updated: 23/Jan/17  Resolved: 23/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

If a user supplies an extern file and then writes code which uses something not defined in the extern we will generated duplicated information in the inferred extern. For example we will generate:

var Foo;
Foo.Bar = {};
Foo.Bar.prototype.baz;

But Foo and Foo.Bar are already present in the existing extern.



 Comments   
Comment by David Nolen [ 23/Jan/17 3:59 PM ]

fixed https://github.com/clojure/clojurescript/commit/960bb9b778190aa7359acb2f74cc61d452cef2ae





[CLJS-1896] Externs file validation Created: 23/Jan/17  Updated: 23/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Invalid externs file will fail silently and not provide the expected externs inference warnings. We should try to catch such issues when we parse the file and throw an exception.






[CLJS-1886] RangedIterator should only be created from cljs.core.PersistentVector instances Created: 10/Jan/17  Updated: 22/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1886.patch     Text File CLJS-1886.patch    
Patch: Code

 Description   

RangedIterator uses cljs.core.PersistentVector internals and thus should not be created from other types of vectors. subvec does not follow this rule.



 Comments   
Comment by ewen grosjean [ 10/Jan/17 5:21 AM ]

This fixes a regression introduced by http://dev.clojure.org/jira/browse/CLJS-1855.

Comment by David Nolen [ 11/Jan/17 7:08 AM ]

Does this patch match the implementation approach in Clojure? Thanks.

Comment by ewen grosjean [ 11/Jan/17 11:08 AM ]

Yes, the Clojure approach is to check the type of the wrapped vector and to use a ranged iterator on instances of APersistentVector. It falls back to a more general iterator when the wrapped vector is not an instance of APersistentVector.
https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/jvm/clojure/lang/APersistentVector.java#L565

Comment by David Nolen [ 16/Jan/17 4:51 PM ]

Hrm that's not really the same since Clojure is checking for an interface. We probably want to replicate this behavior by using a marker protocol and testing for it with `implements?`.

Comment by ewen grosjean [ 17/Jan/17 5:39 AM ]

Clojurescript's ranged iterator goes quite deep into the persistent vector implementation details. Shouldn't marker protocol be used to mark abstract things ?

Comment by David Nolen [ 17/Jan/17 5:56 AM ]

That's not how we use marker protocols in ClojureScript so I'm not really concerned about that. This is something for expert implementers anyhow.

Comment by ewen grosjean [ 22/Jan/17 1:48 PM ]

Clojure checks for APersistentVector because the Clojure ranged iterator implementation only uses methods from APersistentVector.

The Clojurescript ranged iterator goes deeper into the persistent vector implementation details.

I can make a marker protocol named "APersistentVector", but if we want to follow the Clojure approach, then we probably want both PersistentVector and SubVec to implement it, just like in Clojure.

But this won't work because the Clojurescript ranged iterator implementation does not work on SubVec instances.

Two other possibilities are to not make SubVec to implement the marker protocol or to change the ranged iterator implementation.

Comment by ewen grosjean [ 22/Jan/17 2:29 PM ]

New patch attached. It introduces a new marker protocol named APersistentVector and makes PersistentVector to implement it





[CLJS-1895] Externs inference needs to support user supplied externs Created: 21/Jan/17  Updated: 21/Jan/17  Resolved: 21/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 21/Jan/17 4:37 PM ]

fixed https://github.com/clojure/clojurescript/commit/342923bd4428ec8cce3f487eaf37a8ee27bab14f





[CLJS-1873] Self-host: Unit tests fail owing to test.check dep Created: 16/Dec/16  Updated: 21/Jan/17  Resolved: 21/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1873.patch    
Patch: Code and Test

 Description   

Currently the self-host unit tests fail to run with:

$ script/test-self-parity
Testing with Node
WARNING: baz is a single segment namespace at line 1 src/test/cljs/baz.cljs
#error {:message "No such namespace: cljs.test.check, could not locate cljs/test/check.cljs, cljs/test/check.cljc, or Closure namespace \"cljs.test.check\"", :data {:tag :cljs/analysis-error}}

Reason:

With https://github.com/clojure/clojurescript/commit/d1b8b31f7247688098d9e61bb33302a6edc57c2c
the cljs.spec-test namespace was revised to require the cljs.spec.test namespace, which, in turn requires the clojure.test.check namespace. (The auto-aliasing to cljs.test.check is tried an this fails, and this is the diagnostic you ultimately see.)

The port of test.check to self-hosted ClojureScript (TCHECK-105) hasn't yet been released, and, so far, the self-host tests have been avoiding the need for test.check.



 Comments   
Comment by Mike Fikes [ 16/Dec/16 7:07 PM ]

Attached patch fixes the self-test unit tests by moving the test.check-dependent portion into a test namespace for cljs.spec.test (namely cljs.spec.test-test) which is skipped in self-host tests. (When TCHECK-105 is released we can re-visit and revise the self-host test runner to load the needed source from the test check JAR that is placed in lib by script/bootstrap.

Comment by David Nolen [ 21/Jan/17 6:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/1f9fad5ee4450e579f1659f62dcdbcafc20298f4





[CLJS-1874] Self-host: :fn-var true for macros Created: 16/Dec/16  Updated: 21/Jan/17  Resolved: 21/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1874.patch    
Patch: Code and Test

 Description   

In self-host ClojureScript, :fn-var is unconditionally set to true for macros.

One issue this creates is for cljs.test/function?, here

https://github.com/clojure/clojurescript/blob/d1b8b31f7247688098d9e61bb33302a6edc57c2c/src/main/cljs/cljs/test.cljc#L23

in that it causes macros in is expressions, like (is (or true)) to be mis-handled
as if they were functions.

Here is a minimal repro illustrating :fn-var being set:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52055
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
true
cljs.user=> (let [st (cljs.js/empty-state)]
  (cljs.js/eval-str st
    "(ns cljs.user (:require-macros foo.core))"
    nil
    {:eval    cljs.js/js-eval
     :load    (fn [_ cb]
                (cb {:lang   :clj
                     :source "(ns foo.core) (defmacro add [a b] `(+ ~a ~b))"}))
     :context :expr}
    (fn [_]
      (-> @st
        (get-in [:cljs.analyzer/namespaces 'foo.core$macros :defs 'add])
        (select-keys [:macro :fn-var])))))
{:macro true, :fn-var true}
cljs.user=>

Note in the repro above both :macro and :fn-var are set to true.



 Comments   
Comment by David Nolen [ 21/Jan/17 6:55 AM ]

fixed https://github.com/clojure/clojurescript/commit/51ca92b7260bfa22f309766e6ae914d044e5cc5e





[CLJS-1877] :foreign-libs entries should be allowed to specify directories along with individual files Created: 21/Dec/16  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Comments   
Comment by David Nolen [ 20/Jan/17 3:33 PM ]

Work in progress here https://github.com/clojure/clojurescript/tree/cljs-1877

Comment by David Nolen [ 20/Jan/17 5:12 PM ]

One issue right now is that we blindly copy :libs files already in :output-dir (like Closure processed files) back into :output-dir via write-cljs.closure/javascript.

Comment by David Nolen [ 20/Jan/17 5:56 PM ]

basic work in place https://github.com/clojure/clojurescript/commit/30acd88d02857dc903b4da7c028c522d21b37b25. Likely needs further refinement but that can be handled with smaller tickets.





[CLJS-1890] s/form for s/nilable in cljs.spec does not match clojure.spec Created: 17/Jan/17  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Miikka Koskinen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojure 1.9-alpha14



 Description   
Clojure
user=> (s/form (s/nilable int?))
(clojure.spec/nilable clojure.core/int?)
ClojureScript
cljs.user=> (s/form (s/nilable int?))
(cljs.spec/and (cljs.spec/or :cljs.spec/nil cljs.core/nil? :cljs.spec/pred cljs.core/int?) (cljs.spec/conformer cljs.core/second))

What I expected from ClojureScript was (cljs.spec/nilable cljs.core/int?).



 Comments   
Comment by David Nolen [ 20/Jan/17 3:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/2ef5b0d0b0f95f779cc99334300f5fe39144fd23





[CLJS-1811] Can't compose cljs.spec.test.instrument (or cljs.spec.test.check) with cljs.spec.test.enumerate-namespace Created: 05/Oct/16  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: JR Heard Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojurescript 1.9.229



 Description   

When I call

(stest/instrument
  (stest/enumerate-namespace 'my.ns))

I get a stack trace that includes a message like `Caused by: java.lang.RuntimeException: No such namespace: stest`.

The same thing happens when I call

(stest/check
  (stest/enumerate-namespace 'my.ns))


 Comments   
Comment by JR Heard [ 05/Oct/16 5:29 PM ]

(I'm still a newbie here, please let me know if there's any more information I can provide!)

Comment by David Nolen [ 16/Dec/16 2:36 PM ]

Thanks for the report will take a look at this.

Comment by David Nolen [ 30/Dec/16 2:23 PM ]

After some consideration I now think that the Clojure enumerate-namespace helper isn't a good fit and will be unnecessarily challenging to implement in the same way. syntax-quote does not try to qualify symbols with periods in them, perhaps we should just rely on this behavior.

Comment by David Nolen [ 20/Jan/17 3:13 PM ]

fixed https://github.com/clojure/clojurescript/commit/ec220f51efb8a456479c4d0b5c27e611d3abb551





[CLJS-1894] Unnecessary analysis of core.cljs on first compile Created: 20/Jan/17  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

The first compile does not use the cached analysis provided in the ClojureScript JAR.



 Comments   
Comment by David Nolen [ 20/Jan/17 2:35 PM ]

fixed https://github.com/clojure/clojurescript/commit/a3f552cb032d7de0a564d6d5667258b8004500a9





[CLJS-1893] Unnecessary analysis of core.cljs Created: 20/Jan/17  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 20/Jan/17 2:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/462e2295d6e6c1a01b279c8aa091f832c9d09824





[CLJS-1892] Dependencies in JARs are analyzed everytime even if an analysis cache file exists Created: 20/Jan/17  Updated: 20/Jan/17  Resolved: 20/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 20/Jan/17 12:39 PM ]

This appears to be a regression introduced by the fixes to Transit analysis cache support.

Comment by David Nolen [ 20/Jan/17 1:05 PM ]

Upon further investigation it may be that this never worked correctly when following dependencies of an ns form.

Comment by David Nolen [ 20/Jan/17 1:16 PM ]

fixed https://github.com/clojure/clojurescript/commit/ba0983bc1e7ffb45907cc3762fef11a3c3e0ee03





[CLJS-1891] UUID.toString can return non-string Created: 18/Jan/17  Updated: 20/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Summary

An incorrectly-constructed UUID can cause TypeError from str.

Detail

The cljs.core/uuid constructor stores its argument in the uuid field of the UUID deftype. See core.cljs line 10400.

It is assumed that this field is the string representation of the UUID, so it is also the return value of returned from UUID.toString. See core.cljs line 10377.

In correct code, the argument to cljs.core/uuid will never be anything but a string, so this is safe. But incorrect code can call cljs.core/uuid on any type, such as an Object. This leads to the following error when attempting to convert the UUID back into a string:

cljs.user=> (str (uuid #js{:foo "bar"}))
#object[TypeError TypeError: Cannot convert object to primitive value]
   Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (jar:file:~/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2782:22)
   cljs$core$str (jar:file:~/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2775:1)
nil

The error is thrown from arity-1 of cljs.core/str, which coerces its argument to a string by wrapping it in a JavaScript array and calling Array.join; see core.cljs line 2819

Presumably the implementation of Array.join is calling toString on the object. toString returns something which is not a string and cannot be coerced into a string, leading to the TypeError. This can be demonstrated more directly with:

cljs.user=> (str #js{:toString (fn [] #js{})})
#object[TypeError TypeError: Cannot convert object to primitive value]
   Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (jar:file:/Users/stuart/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2782:22)
   cljs$core$str (jar:file:/Users/stuart/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2775:1)
nil

Although the ECMAscript specifications are vague on the subject, it seems safe to say that it is incorrect for toString to return anything which is not a string.

Related

CLJS-1599, "UUIDs are not equal for upper/lower case strings," also relates to construction of UUIDs.



 Comments   
Comment by David Nolen [ 20/Jan/17 11:47 AM ]

Is just throwing a non-string value satisfactory?





[CLJS-1887] add :watch-error-fn option Created: 12/Jan/17  Updated: 16/Jan/17  Resolved: 16/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Shaun LeBron Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1887.patch    

 Description   

Like `:watch-fn` notifies on a successful build, the proposed `:watch-error-fn` can notify on a failed one.

See: https://github.com/clojure/clojurescript/wiki/Compiler-Options#watch-fn



 Comments   
Comment by Shaun LeBron [ 12/Jan/17 11:11 PM ]

Had some fun verifying this with figwheel-sidecar's `print-exception` function:
https://github.com/cljs/tool/blob/watch-fig-errors/target/script/watch.clj

Comment by David Nolen [ 16/Jan/17 4:49 PM ]

fixed https://github.com/clojure/clojurescript/commit/5603b313de751f0d7344b7ae790ce0591fab5f77





[CLJS-1889] A lone ampersand `&` can be used to name a var, but throws when invoked. `(&)` Created: 15/Jan/17  Updated: 16/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Aleksander Madland Stapnes Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I can use the symbol & to name something, but if I try to invoke it, the following exception is thrown:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:src/amp/core.cljs {:file #object[java.io.File 0x5e63cad "src/amp/core.cljs"]}, compiling/home/madstap/code/ampersand/build.clj:3:1)
at clojure.lang.Compiler.load(Compiler.java:7391)
at clojure.lang.Compiler.loadFile(Compiler.java:7317)
at clojure.main$load_script.invokeStatic(main.clj:275)
at clojure.main$script_opt.invokeStatic(main.clj:335)
at clojure.main$script_opt.invoke(main.clj:330)
at clojure.main$main.invokeStatic(main.clj:421)
at clojure.main$main.doInvoke(main.clj:384)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.lang.Var.invoke(Var.java:379)
at clojure.lang.AFn.applyToHelper(AFn.java:154)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:src/amp/core.cljs {:file #object[java.io.File 0x5e63cad "src/amp/core.cljs"]}
at clojure.core$ex_info.invokeStatic(core.clj:4617)
at cljs.compiler$compile_file$fn__3147.invoke(compiler.cljc:1406)
at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1376)
at cljs.closure$compile_file.invokeStatic(closure.clj:430)
at cljs.closure$fn__4204.invokeStatic(closure.clj:497)
at cljs.closure$fn__4204.invoke(closure.clj:493)
at cljs.closure$fn_4146$G4139_4153.invoke(closure.clj:389)
at cljs.closure$compile_sources$iter_43154319$fn_4320.invoke(closure.clj:829)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:688)
at clojure.core$next__4341.invokeStatic(core.clj:64)
at clojure.core$dorun.invokeStatic(core.clj:3033)
at clojure.core$doall.invokeStatic(core.clj:3039)
at cljs.closure$compile_sources.invokeStatic(closure.clj:826)
at cljs.closure$build.invokeStatic(closure.clj:1942)
at cljs.build.api$build.invokeStatic(api.clj:198)
at cljs.build.api$build.invoke(api.clj:187)
at cljs.build.api$build.invokeStatic(api.clj:190)
at cljs.build.api$build.invoke(api.clj:187)
at user$eval24.invokeStatic(build.clj:3)
at user$eval24.invoke(build.clj:3)
at clojure.lang.Compiler.eval(Compiler.java:6927)
at clojure.lang.Compiler.load(Compiler.java:7379)
... 11 more
Caused by: clojure.lang.ExceptionInfo: No method in multimethod 'parse' for dispatch value: & at line 5 src/amp/core.cljs {:file "src/amp/core.cljs", :line 5, :column 1, :tag :cljs/analysis-error}
at clojure.core$ex_info.invokeStatic(core.clj:4617)
at cljs.analyzer$error.invokeStatic(analyzer.cljc:628)
at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2871)
at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2892)
at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3011)
at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3056)
at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3073)
at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1255)
at cljs.compiler$compile_file_STAR_$fn__3124.invoke(compiler.cljc:1325)
at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1159)
at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1316)
at cljs.compiler$compile_file$fn__3147.invoke(compiler.cljc:1396)
... 34 more
Caused by: java.lang.IllegalArgumentException: No method in multimethod 'parse' for dispatch value: &
at clojure.lang.MultiFn.getFn(MultiFn.java:156)
at clojure.lang.MultiFn.invoke(MultiFn.java:251)
at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2867)
at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2870)
... 43 more



 Comments   
Comment by Aleksander Madland Stapnes [ 16/Jan/17 1:59 AM ]

Better explanation as I can't seem to edit the description: https://gist.github.com/madstap/c77581185afa7fea8bbf2556f2d9fafe





[CLJS-1888] Seqs of PHMs and PAMs do not handle metadata correctly Created: 13/Jan/17  Updated: 13/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata

Attachments: Text File CLJS-1888.patch    

 Description   

Metadata on parent seq ends up being passed to the next seq. Calling `empty` on a seq also ends up carrying metadata.

Examples:

(def s (with-meta (seq {:a 1 :b 2}) {:some :meta}))

(meta s) => {:some :meta} ;; Good
(meta (rest s))  => {:some :meta} ;; Bad, expected nil
(meta (next s))  => {:some :meta} ;; Bad, expected nil
(meta (empty s)) => {:some :meta} ;; Bad, expected nil





[CLJS-1885] assoc should return an array-map when passed a nil collection Created: 09/Jan/17  Updated: 09/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1885.patch    
Patch: Code




[CLJS-1884] Give a chance to MetaFn to be removed by closure under :advanced optimization Created: 09/Jan/17  Updated: 09/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1884.patch    
Patch: Code




[CLJS-1883] Foreign libs can't be found on Node.js Created: 04/Jan/17  Updated: 09/Jan/17  Resolved: 09/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Roman Scherer Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1883.patch    

 Description   

Since CLJS-1718 foreign libs are placed under their relative path, but still loaded from the root when using Node.js.



 Comments   
Comment by Roman Scherer [ 04/Jan/17 10:20 AM ]

David, this patch uses util/relative-name instead of util/get-name when emitting the cljs.core.load_file instruction. I believe it's the same function used to place the foreign libs into the output directory. I also changed some of my previously written tests to use io/file instead of the hardcoded file separator.

Comment by David Nolen [ 09/Jan/17 11:47 AM ]

fixed https://github.com/clojure/clojurescript/commit/8e183ba8ad3eefea75c127a5fdb76c2925b809f0





[CLJS-1882] Constant table dependency order issue with Google Closure modules Created: 30/Dec/16  Updated: 04/Jan/17  Resolved: 04/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Roman Scherer Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1882.patch    

 Description   

When compiling ClojureScript code with :optimize-constants keywords and symbols will be instantiated in the constants_table.js file and referenced from other namespaces. This is an optimization to avoid instantiating the same keyword/symbol twice. However, care must be taken that a keyword or symbol is actually instantiated before being used as a reference. This means that constants_table.js must be before any namespace using a keyword or symbol when sorting sources in dependency order. At the moment there are 2 code paths for this, one that doesn't use :modules, and one that does.

The code path without :modules sorts the dependencies in Clojure and makes sure constants_table.js comes directly after cljs.core. It uses information from parse-ns and adds a "virtual" :requires dependency.
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L727
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L3303

The code path with :modules uses Google Closure to sort the dependencies. Google Closure doesn't use the same information as the code path above. It looks only at the goog.require and goog.provide statements of the provided files and sorts them accordingly. At which place constants_table.js lands in the sorted list is not deterministic, because information about the dependencies is missing.

The missing information is the knowledge about which namespace uses a keyword or symbol from constants_table.js. If a namespace uses a keyword or symbol from the constants table it needs to require it, otherwise not.

The attached is a first draft adding a goog.provide to the constants table file and a goog.require to all other namespaces except cljs.core.



 Comments   
Comment by Roman Scherer [ 30/Dec/16 4:53 PM ]

If we go the direction emitting the information about dependencies regarding the constant table in the source files we should move the constants_table.js file name namespace under the cljs.core namespace to avoid conflicts. I would suggest clojure.core.constants or clojure.core.constants-table.

I think we could also remove code in the first code path that does the special handling at the moment. If all files correctly declare their dependencies parse-ns and Google Closure work the same way. They both parse the ns form an find out the dependency order from there. No special injections like this:
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L3303

The attached patch adds a goog.require for the constants table to every ClojureScript file at the moment, except cljs.core. This could be avoided if information about which namespace uses a keyword or symbol is available when code is emitted.

Thoughts?

Comment by Roman Scherer [ 31/Dec/16 6:33 AM ]

David, I updated the patch to use the cljs.core.constants namespace and added a test.

Comment by Roman Scherer [ 31/Dec/16 6:35 AM ]

I also realized that parse-ns and Google Closure can't share logic. The parse-ns fn parses ClojureScript files, Google Closure JavaScript files. So the middle section of my first comment is not relevant.

Comment by David Nolen [ 02/Jan/17 7:33 PM ]

The patch mostly looks good. However a small nitpick, we shouldn't be requiring the constants namespace unless we're compiling with that option enabled.

Comment by Roman Scherer [ 03/Jan/17 3:01 AM ]

David, there's a check for (-> @env/compiler :options :emit-constants) around the form that emits the goog.require for the constants table. Did you miss that, or do you want something else?

Comment by David Nolen [ 03/Jan/17 9:58 AM ]

Roman, I did miss that. Thanks!

Comment by David Nolen [ 04/Jan/17 6:47 AM ]

fixed https://github.com/clojure/clojurescript/commit/12c805f7417f39135d2b36985cf2cda98a08fe40





[CLJS-1574] CLJS string equivalence is very slow in Chrome Created: 16/Feb/16  Updated: 03/Jan/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

Google Chrome 48.0.2564.109 on Mac OS X 10.11.3
Safari 9.0.3 (11601.4.4) on Mac OS X 10.11.3



 Description   

Clojurescript's equivalence for strings in Google Chrome is ~1000 times slower than equivalent javascript functionality, and ~1000 times slower than the same function in Safari.

Google Chrome
js equiv:  0.005 seconds
cljs equiv:  1.898 seconds
Safari
js equiv:  0.005 seconds
cljs equiv:  0.006 seconds
(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(deftest test-js-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (js* "~{} == ~{}" str1 str2)
                  "js equivalence")
        end   (now)
        ]
    (println "js equiv: " (delta end start))))

(deftest test-cljs-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (= str1 str2)
                  "cljs equivalence")
        end   (now)
        ]
    (println "cljs equiv: " (delta end start))))


 Comments   
Comment by Stephen Nelson [ 16/Feb/16 6:06 PM ]

This bug only occurs when cljs.pprint has been required.

Comment by Stephen Nelson [ 16/Feb/16 6:38 PM ]

After a whole lot of binary search, here's a minimal reproduction. When cljs.pprint is loaded it constructs write-option-table. It seems that constructing a hash map with the keys :added and :ns causes a call to (= :added :ns), which is sufficient to cause string equality to become extremely slow.

(ns hello-world.core)

(enable-console-print!)

(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(defn test [] 
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (= str1 str2)
        end   (now)
        ]
      (println "cljs equiv: " (delta end start))))

(test)

(= :added :ns)

(test)
Comment by Peter Schuck [ 17/Feb/16 4:50 PM ]

Is the ClojureScript compiled with options :optimizations :advanced or :static-fns true? Compiling ClojureScript without those options results in call indirection for all function calls which might explain the slowdown. See http://swannodette.github.io/2015/03/16/optimizing-clojurescript-function-invocation/ for more information.

Comment by Stephen Nelson [ 17/Feb/16 9:06 PM ]

This happens with :advanced, :simple, and without optimisations. Stepping through the generated javascript seems to indicated that the slow down comes from the VM's internal function dispatch. Regardless, I don't think that the extra function calls related to dynamic dispatch in clojurescript could add minutes of overhead per call. Note that the test case above only uses 128k of string data, the case where I encountered this issue first used ~512K and took about 5 minutes to complete a single function call.

Comment by Francis Avila [ 17/Feb/16 9:14 PM ]

I have reproduced this in Chrome for Linux, on :none and :advanced optimization levels using different test code. I verified the result of the compare so the JIT won't optimize it away and I used performance.mark() and performance.measure() for timing, although none of this should have mattered.

Every subsequent string compare after the first -equiv-invoking use of equal is significantly slower for no reason I can see. There are no intermediate GCs or anything to suggest that it should be slower--it just takes longer! The only thing I can think of is maybe the keyword-equals triggers a deopt because it makes the equal-function megamorphic, but the code is run so few times that there should not be jit optimizations kicking it at all. Also, the keyword-compare itself remains fast.

I suspect a Chrome/v8 bug. Possibly a different internal string representation kicks in for some reason which has a slower compare? This is only an issue for largish, non-constant strings, and the slowdown is proportional to string size. I'm going to try and reproduce this with pure JS.

Comment by Francis Avila [ 18/Feb/16 12:33 AM ]

All you need to reproduce this is to use the strict equality operator in a function body non-monomorphically. Subsequent executions of the function with strings (at least) which have not been compared before the polymorphic call will be very slow.

If you replace strict equality (triple-equal) with normal equality (double-equal), this issue goes away.

This is clearly a Chrome/v8 bug, but I'm not sure where to report it.

Minimal pure-javascript reproduction:

function str(size) {
  var s = "";
  for (var i = 0; i < size; i++) s += "a";
  return s;
}

function eq(x, y) {
  performance.mark("start");
  x === y; // No slowdown if use == instead
  performance.mark("end");
}

function print_measures() {
  performance.getEntriesByType("measure")
  .forEach(entry => console.log(entry.name, entry.duration));
}

var s1 = str(64 * 1024);
var s2 = str(64 * 1024);
var s3 = str(64 * 1024);

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(0, 0);
performance.measure("eq(0, 0)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

print_measures();

Results with Chrome 48.0.2564.109 (64-bit) on a slow iMac with OS X 10.11.3

eq(s1, s2)   4.465000000000003     // fast string compare
eq(0, 0)     0.009999999999990905  // break monomorphism of eq()
eq(s1, s3) 259.665                 // Now string compare is slow
eq(s1, s2)   0.019999999999924967  // Repeated call still fast
eq(s1, s3) 232.52499999999998      // ... but not from after the polymorphic invoke
Comment by Francis Avila [ 22/Feb/16 3:14 AM ]

Issue added to v8: https://bugs.chromium.org/p/v8/issues/detail?id=4773

Comment by Francis Avila [ 03/Jan/17 9:23 AM ]

Update: This bug was fixed upstream today, so it should start showing up in releases eventually.





[CLJS-1853] Docstrings are included in compiled output Created: 15/Nov/16  Updated: 02/Jan/17  Resolved: 02/Jan/17

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Richard Newman Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1853.patch    
Patch: Code

 Description   

Looking at code compiled with 'advanced':

$cljs$core$cst$0sym$0_STAR_print_DASH_base_STAR_$$, "target/release-browser/cljs/pprint.cljs", 13, 1, !0, 672, 675, $cljs$core$List$EMPTY$$, "The base to use for printing integers and rationals.", $cljs$core$truth_$$($cljs$pprint$STAR_print_base_STAR$$) ? $cljs$pprint$STAR_print_base_STAR$$.$cljs$lang$test$ : null]))]);

...

$cljs$core$cst$0sym$0cljs$0pprint$$, $cljs$core$cst$0sym$0_STAR_print_DASH_right_DASH_margin_STAR_$$, "target/release-browser/cljs/pprint.cljs", 22, 1, !0, 625, 630, $cljs$core$List$EMPTY$$, "Pretty printing will try to avoid anything going beyond this column.\nSet it to nil to have pprint let the line be arbitrarily long. This will ignore all\nnon-mandatory newlines.", $cljs$core$truth_$$($cljs$pprint$STAR_print_right_margin_STAR$$) ? $cljs$pprint$STAR_print_right_margin_STAR$$.$cljs$lang$test$ :

It looks like docstrings aren't stripped from dynamic vars, only from functions.

This is a bit of a waste of space...



 Comments   
Comment by António Nuno Monteiro [ 21/Dec/16 7:42 AM ]

I'm not seeing this behavior. Can you provide a minimal repro which exhibits the bug you're seeing?

Comment by Richard Newman [ 01/Jan/17 9:39 AM ]

Here's a repro case.

https://github.com/rnewman/cljs-1853

Comment by António Nuno Monteiro [ 01/Jan/17 11:15 AM ]

After some investigation, the issue seems unrelated to the description of this ticket.

Specifically, the problem occurs because there's a map which refers to Vars directly. See:
https://github.com/clojure/clojurescript/blob/95fd110f55c57b890422763ed8f2644cfbf159de/src/main/cljs/cljs/pprint.cljs#L692

Comment by Richard Newman [ 01/Jan/17 2:55 PM ]

Feel free to rename the ticket to "Required library code that includes a map that refers to vars that have associated metadata results in that metadata appearing in the compiled output, regardless of whether the metadata is ever accessed".

From an optimization perspective, I regard this behavior (however you'd like to characterize it!) as a bug — code that simply requires and uses 'cljs.pprint' ends up, when compiled at any optimization level, containing a pile of unused verbose English strings.

Comment by Richard Newman [ 01/Jan/17 2:57 PM ]

It seems like an easy 'fix' for this specific issue is to just delete `write-option-table` entirely — it's private and unused. But I don't see why `write-option-table` wouldn't be discarded by the compiler or the optimizer, and even if it exists I don't see why unused metadata wouldn't be stripped. So perhaps that's three bugs of varying specificity, each of which would make ClojureScript a little bit more production-ready.

Comment by António Nuno Monteiro [ 02/Jan/17 11:11 AM ]

Attached a patch commenting out the `cljs.pprint/write-option-table` map, as per the Slack discussion.

Comment by David Nolen [ 02/Jan/17 7:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/08b5d185b616d309a5dd8cb25e5c08d59d2f5005





[CLJS-1838] cljs.compiler.api/compile-root doesn't use .cljc/.cljs precedence rules correctly. Created: 03/Nov/16  Updated: 30/Dec/16  Resolved: 30/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Kaplinskiy Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

When you have both a .cljc and a .cljs for the same namespace, compile-root seems to arbitrarily choose which gets compiled. Case-in-point: cljs.test.



 Comments   
Comment by António Nuno Monteiro [ 05/Nov/16 9:29 AM ]

What ClojureScript version was this tested against? It would seem to me that CLJS-1772 fixed this issue (which was shipped in 1.9.229).

Comment by Mike Kaplinskiy [ 29/Dec/16 3:48 PM ]

Indeed the latest cljs fixes the issue.





[CLJS-1879] optimize cljs.core/str macro Created: 22/Dec/16  Updated: 29/Dec/16  Resolved: 29/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File str-perf.patch    
Patch: Code

 Description   

Currently the cljs.core/str macro generates direct calls to the cljs.core/str function via a js* inlined call. This means that it never directly dispatches to the correct arity although we know know that it is 1.

This has performance cost and prevents dead code elimination through closure.

The added patch changes the macro to directly dispatch to the correct function instead, yielding improved performance and making it dead code removable.

Also added a benchmark which is a bit useless when run with :advanced as it is removed but still useful for the future I guess.



 Comments   
Comment by Thomas Heller [ 22/Dec/16 12:13 AM ]

Benchmark on node (bstr was a temp namespace that just had the new macro)

;;; str
[], (str 1), 1000000 runs, 55 msecs
[], (str nil), 1000000 runs, 39 msecs
[], (str "1"), 1000000 runs, 44 msecs
[], (str "1" "2"), 1000000 runs, 257 msecs
[], (str "1" "2" "3"), 1000000 runs, 334 msecs

;;; bstr
[], (bstr/str 1), 1000000 runs, 13 msecs
[], (bstr/str nil), 1000000 runs, 8 msecs
[], (bstr/str "1"), 1000000 runs, 11 msecs
[], (bstr/str "1" "2"), 1000000 runs, 173 msecs
[], (bstr/str "1" "2" "3"), 1000000 runs, 210 msecs
Comment by Thomas Heller [ 22/Dec/16 12:18 AM ]

http://dev.clojure.org/jira/browse/CLJS-890

Has a lot of history on the topic and there are still some more issues that could be optimized, but this is easily the biggest gain with no real downside.

Comment by David Nolen [ 22/Dec/16 8:28 AM ]

Nice!

Comment by David Nolen [ 29/Dec/16 3:20 PM ]

fixed https://github.com/clojure/clojurescript/commit/cb66c277fc869ab9a5892cfb9ce6d1d0121d3dc7





[CLJS-1878] Use some? over (not (nil %)) in analyzer Created: 21/Dec/16  Updated: 29/Dec/16  Resolved: 29/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Ambrose Bonnaire-Sergeant Assignee: Ambrose Bonnaire-Sergeant
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File some.patch    

 Description   

I did a pass over analyzer.cljc and updated some of the branching idioms to propagate ^boolean tags using `some?` instead of `(not (nil? %))`. I also carefully added some extra `some?` or `true?` tests to propagate more ^boolean tags.



 Comments   
Comment by David Nolen [ 29/Dec/16 3:19 PM ]

fixed https://github.com/clojure/clojurescript/commit/96493c74d9d49cfd00e042a13f9d287ca238187f





[CLJS-1880] Few missing boolean annotations on some hasNext method calls Created: 23/Dec/16  Updated: 29/Dec/16  Resolved: 29/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1880.patch    

 Description   

There are a few places where boolean annotations are missing on calls to `hasNext`.

Also a case in the `HashMapIter` where `seen` should be annotated.



 Comments   
Comment by David Nolen [ 29/Dec/16 3:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/47fa30dd95c850aa0643aae9c274c09bc202065c





[CLJS-1881] Improve cljs.core/distinct perf by using transient map Created: 25/Dec/16  Updated: 29/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File cljs-1881-transient-in-distinct.patch    
Patch: Code

 Description   

Current implementation of cljs.core/distinct uses persistent set. This patch improves performance by ~10%-33% by using transient map instead. Mirror Clojure task CLJ-2090

10 elements
(reduce + 0 (distinct coll))     12.360220502805724 µs => 9.504153281757874 µs (-23%)
(transduce (distinct) + 0 coll)  7.689213711227641 µs => 5.3549045227207 µs (-30%)
100 elements
(reduce + 0 (distinct coll))     136.43424283765356 µs => 106.66990187713321 µs (-21%)
(transduce (distinct) + 0 coll)  73.05427319211107 µs => 48.737280701754386 µs (-33%)
1000 elements
(reduce + 0 (distinct coll))     1.1207102908277415 ms => 919.8952205882359 µs (-17%)
(transduce (distinct) + 0 coll)  677.2834912043312 µs => 482.79681467181547 µs (-28%)
10000 elements
(reduce + 0 (distinct coll))     4.777295238095228 ms => 4.3203448275862115 ms (-9%)
(transduce (distinct) + 0 coll)  2.889020114942531 ms => 2.44890487804879 ms (-15%)

Benchmarking code: https://gist.github.com/tonsky/258c3d715e6a485522f7ba5e663624fd






[CLJS-324] cljs.core/format Created: 24/Jun/12  Updated: 27/Dec/16

Status: Reopened
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File 0001-CLJS-324-implement-cljs.core-format-as-wrapper-for-g.patch     File goog.string.format-0.0-1913.tgz    

 Description   

Implement cljs.core/format. Make sure there is nothing in gclosure for this. I suppose we should try to emulate the jvm version as much as that makes sense?



 Comments   
Comment by Michał Marczyk [ 28/Jun/12 11:29 AM ]

GClosure does actually include a string formatter – goog.string.format. Note that that's both a non-ctor function name and the GClosure "namespace" name, so in order to use it one must require goog.string.format on top of gstring and say gstring/format (or perhaps leave out the gstring require spec and use goog.string/format – not tested, but should work).

The patch to introduce format and printf as wrappers for goog.string.format is naturally extremely simple, so here it is. Note that this particular patch must be applied on top of CLJS-328.

I have no idea how goog.string.format compares to the JVM's String/format (basic number formatting seems to work as it should in sprintf-like functions, but other than that I haven't tested it much).

Comment by David Nolen [ 29/Jun/12 10:44 AM ]

The tests fail for me with this patch applied.

Comment by David Nolen [ 29/Jun/12 11:07 AM ]

fixed, http://github.com/clojure/clojurescript/commit/8f518760a3df8b351208e97bb70270856623bb0a

Comment by David Nolen [ 11/Sep/13 5:05 PM ]

Backing this one out, goog.string.format defies advanced optimization and it provides few of the capabilities of Clojure's format - which does a lot because of java.util.Formatter. Apologies for the churn, but this is a simple thing for people to stub in themselves for the little bit of functionality it actually delivers.

Comment by Lars Bohl [ 12/Oct/13 6:33 AM ]

Uploading a slighly modified version lein-cljsbuild's "advanced" demo, to demonstrate that using goog.string.format produces a runtime error with clojurescript 0.0.1913. Run "lein ring server" and navigate to http://localhost:3000/

The code in hello.cljs shows that goog.string.toTitleCase works, but not goog.string.format.

Comment by Julien Eluard [ 12/Oct/13 6:43 AM ]

It looks like you are not requiring correctly format. See a working example here.

Comment by Lars Bohl [ 12/Oct/13 6:58 AM ]

Julent, thanks! It needs another [goog.string.format] after [goog.string :as gstring] before you can use gstring/format. hello.cljs now looks like this, and throws no exceptions: https://www.refheap.com/19693

Comment by Ikuru Kanuma [ 27/Dec/16 6:52 PM ]

Would much appreciate a DCE compliant implementation of this, as I happen to be maintaining a library that broke because of this removal.
Has anyone worked on this since?

Comment by Gary Fredericks [ 27/Dec/16 7:31 PM ]

I thought about trying to port java.util.Formatter just for fun.

I can't figure out what you mean by "DCE compliant".

Comment by Ikuru Kanuma [ 27/Dec/16 8:33 PM ]

Oh, DCE = dead code elimination.
I thought it was a common abbrev. from this mail:
https://groups.google.com/forum/#!topic/clojure-dev/URrnaU6KWQk

I would definitely send my cheers if you do decide to put in the time!





[CLJS-1497] `find` on an associative collection does not return collection key Created: 30/Nov/15  Updated: 21/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1497-2.patch     Text File CLJS-1497.patch    
Patch: Code and Test

 Description   

Instead find returns the passed in key. This means metadata on the key will appear to be lost. Related to CLJS-1496.



 Comments   
Comment by Rohit Aggarwal [ 15/Jun/16 3:39 PM ]

Proposed Plan

IAssociative protocol has a function called -entry-at which has been commented out. This function needs to be implemented which will return the necessary data structure, similar to the way it has been done in Clojure.

An example of its implementation for PersistentArrayMap is:

(-entry-at
 [coll k]
 (let [idx (array-map-index-of coll k)]
   (when-not (neg? idx)
     [(aget arr idx) (aget arr (inc idx))])))

We will need to implement this for all the collections which implement that protocol.

A failing test case:

(deftest test-find-meta-cljs-1497
  (let [k        [1 2 3]
        m        {:my "meta"}
        v        1
        xs       {(with-meta k m) v}
        [k' v']  (find xs k)]
    (is (= k k'))
    (is (= v v'))
    (is (= m (meta k')))))
Comment by António Nuno Monteiro [ 13/Nov/16 10:23 AM ]

Attached patch with proposed fix and tests.

Comment by Alex Miller [ 18/Nov/16 7:50 PM ]

dnolen's comment was lost here in a system migration, but he said: "The proposed patch is a breaking change for people who implement custom collections. We need a new protocol `IEntryAt` or something like this."

Comment by António Nuno Monteiro [ 21/Dec/16 9:03 AM ]

Attached an updated patch as per review comments.





[CLJS-1876] Faster PersistentVector, Subvec and ChunkedSeq reduce. Created: 19/Dec/16  Updated: 19/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: File benchmarks.cljs     Text File CLJS-1876.patch    
Patch: Code

 Description   

This patch improved the performance of the 2-arity `-reduce` of PersistentVectors as well as the 2 and 3 arity versions of `-reduce` for `Subvecs` and `ChunkedSeqs`.

For large (> 32) `Subvecs` and `ChunkedSeqs` saw a 7x speed up in V8 and up to 11x in JavascriptCore. Smaller versions saw no change.

The 2-arity version of PersistentVector `-reduce` was also improved ~4x and ~7x in V8 and JavascriptCore respectively.

In the benchmarks below all runs where (N <= 32) were run 1,000,000 times. For the larger collections 100,000 iterations were made.

PersistentVector 3-arity reduce (no code was changed)

N master (v8) patched (v8) master (jsc) patched (jsc)
0 44 44 17 19
1 121 102 29 32
2 151 133 35 37
4 219 199 50 50
8 360 336 79 79
16 606 588 130 131
32 1124 1109 243 250
100 329 338 75 76
1000 3235 3317 704 725
10000 32960 33575 7365 7316

Persistent Vector 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 65 41 29 12
1 96 58 49 42
2 147 66 87 45
4 246 85 113 44
8 446 142 202 69
16 829 276 362 98
32 1627 506 693 132
100 534 154 236 41
1000 5442 1568 2321 329
10000 58396 15386 26162 3406

ChunkedSeq 3-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 57 69 93 88
1 54 60 31 26
2 68 63 27 28
4 94 91 37 39
8 146 152 59 58
16 272 266 121 107
32 479 526 170 174
100 1186 165 459 39
1000 11760 1539 4547 334
10000 121986 16080 48639 3384

ChunkedSeq 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 62 63 96 97
1 23 31 16 19
2 35 40 20 17
4 68 70 26 29
8 116 120 49 47
16 232 223 83 89
32 467 444 156 158
100 1123 164 417 39
1000 12328 1659 4516 327
10000 126570 15940 47435 3330

Subvec 3-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 67 67 51 34
1 185 71 100 35
2 296 84 160 44
4 514 100 259 52
8 958 156 405 77
16 1878 295 733 138
32 3668 565 1433 139
100 1164 165 462 36
1000 12596 1600 4798 337
10000 122919 16108 48093 3511

Subvec 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 73 49 35 22
1 169 58 75 48
2 289 70 117 54
4 544 103 197 56
8 961 159 378 74
16 1852 288 697 103
32 3644 514 1425 145
100 1245 147 441 39
1000 12065 1537 4556 333
10000 122519 15600 46324 3370

The source code for the benchmarks is attached.






[CLJS-1866] RangedIterator performance tweaks Created: 08/Dec/16  Updated: 19/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Thomas Mulvaney
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-1866.patch     Text File CLJS-1866-updated.patch     Text File CLJS-1866-updated.patch    
Patch: Code

 Description   

The attached patch simplifies and speeds up the RangedIterator.

The benchmarks were run using the following function to test vector iteration:

(defn consume-iterator
  [v]
  (let [iter (-iterator v)]
    (loop []
      (when (.hasNext iter)
        (.next iter)
        (recur)))))

A series of "simple-benchmarks" were setup as follows:

(simple-benchmark [v (into [] (range N))] (consume-iterator v) I)

Where 'N' and 'I' were values from the 'Vector Size' and 'Iterations' columns of the table below .

Vector Size Iterations V8 Speed [msec] (master) V8 Speed [msec] (patch) JSC Speed [msec] (master) JSC Speed [msec] (patch)
1 100,000 15 11 13 7
2 100,000 14 10 7 4
4 100,000 18 10 9 5
8 100,000 27 12 14 6
16 100,000 43 17 19 9
32 100,000 74 24 37 15
100 100,000 217 59 105 45
1000 100,000 2008 524 1032 392
10,000 100,000 20390 5856 10249 4178
100,000 10,000 20334 5324 10324 4387

Javascript engine versions used:

  • V8 version 5.1.281.47
  • JSC version Unknown

The RangedIterator constructor function `ranged-iterator` was also made private.



 Comments   
Comment by David Nolen [ 16/Dec/16 2:04 PM ]

Let's get a patch with the performance change without altering the constructor first. Thanks.

Comment by Thomas Mulvaney [ 17/Dec/16 7:15 AM ]

Rebased and constructor no longer private.

Comment by David Nolen [ 17/Dec/16 9:59 AM ]

Sorry for not being clear. Leave the fields of the deftype alone even if we aren't using them for now. We want the performance enhancements without changing the API at all.

Comment by Thomas Mulvaney [ 19/Dec/16 5:58 AM ]

Thanks that makes sense. Fixed in this patch.





[CLJS-1859] Comparing a map and a record with = produces different results based on argument order Created: 23/Nov/16  Updated: 17/Dec/16  Resolved: 16/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: 1.9.293

Type: Defect Priority: Minor
Reporter: Juan Facorro Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Mac OS X
V8
java version "1.8.0_73"
Java(TM) SE Runtime Environment (build 1.8.0_73-b02)
Java HotSpot(TM) 64-Bit Server VM (build 25.73-b02, mixed mode)



 Description   

The result of comparing a map and a record is different based on the order of the arguments to =:

To quit, type: :cljs/quit 
cljs.user=> (defrecord R []) 
cljs.user/R 
cljs.user=> (= {} (R.)) 
true 
cljs.user=> (= (R.) {}) 
false 

The result is the same for the code above with tags r1.7.228, r1.8.34 and 1.9.293.

This seems to be rooted in the fact that when a map is the first argument, the function used to make the comparison is the implementation of equiv from the map. But when a record is the first argument the implementation used is the one from the record, which checks if the types of both arguments are equal.

In Clojure JVM the implementation of equiv in clojure.lang.APersistentMap checks for the marker interface clojure.lang.MapEquivalence, which avoids this situation.



 Comments   
Comment by David Nolen [ 16/Dec/16 2:18 PM ]

This is not a bug an in fact the behavior of ClojureScript is simply wrong. Records and maps are never equal in Clojure.

Comment by Juan Facorro [ 17/Dec/16 12:40 PM ]

I don't understand. If the behavior in ClojureScript is wrong, shouldn't it be fixed so that (= {} (R.)) returns to false.

Comment by David Nolen [ 17/Dec/16 6:05 PM ]

Yes but that's a different issue from what this ticket described.





[CLJS-1875] Difference in seqable? between Clojure and ClojureScript Created: 17/Dec/16  Updated: 17/Dec/16  Resolved: 17/Dec/16

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Arne Brasseur Assignee: Arne Brasseur
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1875.patch     Text File CLJS-1875.patch    
Patch: Code and Test

 Description   

The seqable? predicate introduced in Clojure 1.9 will return true for anything that can successfully be passed to seq. This includes instances of (I)Seqable, native arrays, Java collections, and strings.

Clojure docstring: "Return true if the seq function is supported for s"

In ClojureScript seqable? only checks for (satisfies? ISeqable s)

Clojurescript docstring: "Return true if s satisfies ISeqable"

Clojure:

(seqable? (array 1 2 3)) ;;=> true
(seqable? "foo") ;;=> true

ClojureScript

(seqable? (array 1 2 3)) ;;=> false
(seqable? "foo") ;;=> false



 Comments   
Comment by Arne Brasseur [ 17/Dec/16 2:47 AM ]

This adds the same checks to seqable? that are being used in seq.

This one I wasn't sure how to test:

(native-satisfies? ISeqable s)

Comment by David Nolen [ 17/Dec/16 6:25 AM ]

You don't need `native-satisfies?`. Otherwise looks good.

Comment by Arne Brasseur [ 17/Dec/16 8:18 AM ]

Version without native-satisfies?

Comment by Arne Brasseur [ 17/Dec/16 8:19 AM ]

Thanks, I updated the patch.

Comment by David Nolen [ 17/Dec/16 9:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/101d7d9e03e90518e6769781dd33fbe6387d2d44





[CLJS-1829] 3-arity get does not return not-found on negative key values for arrays and strings Created: 21/Oct/16  Updated: 17/Dec/16  Resolved: 17/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Thomas Mulvaney
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-1829.patch     Text File CLJS-1829-updated.patch    
Patch: Code and Test

 Description   

Examples of failing cases:

Calling `(get "asd" -1 :not-found)` returns nil rather than :not-found.
Calling `(get #js [1 2 3] -1 :not-found)` returns nil rather than :not-found.



 Comments   
Comment by António Nuno Monteiro [ 21/Oct/16 5:10 AM ]

Good catch. Probably related to https://github.com/clojure/clojurescript/commit/9cb8da1d82078cfe21c7f732d94115867f455a0a

Mind if I work on this?

Comment by Thomas Mulvaney [ 21/Oct/16 5:49 AM ]

The attached patch also catches the case where a key is nil. Also using charAt rather than aget for strings.

Comment by Thomas Mulvaney [ 21/Oct/16 5:50 AM ]

Sorry António, I just saw your comment.

Comment by David Nolen [ 16/Dec/16 1:51 PM ]

The patch needs to be rebased on master. Thanks.

Comment by Thomas Mulvaney [ 17/Dec/16 7:13 AM ]

Rebased.

Comment by David Nolen [ 17/Dec/16 9:55 AM ]

fixed https://github.com/clojure/clojurescript/commit/4f08dbdedb4d5645a977eb43d21a4ebf28597374





[CLJS-1823] read-string on map with multiple keys should throw error Created: 15/Oct/16  Updated: 17/Dec/16  Resolved: 17/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Thomas Mulvaney
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-1823-alt.patch     Text File CLJS-1823.patch    

 Description   

On an array-map sized string `(read-string "{:a 1 :b 2 :a 1}")` returns a map with 2 :a keys.
On a hash-map sized string with duplicates no error is thrown.

Both of these cases deviate from Clojure's behaviour which is to throw a duplicate key exception.



 Comments   
Comment by Rohit Aggarwal [ 16/Oct/16 5:22 AM ]

I am attaching a patch which makes the behaviour of reading a PersistentArrayMap same as PersistentHashMap. Now the duplicates are removed from PersistentArrayMap.

Both however don't throw an error.

Comment by Thomas Mulvaney [ 17/Oct/16 5:03 AM ]

I have attached an alternative patch which throws on duplicate when reading and handles other duplicate cases as per clojure. The extra cases which were not handled correctly previously have been added as tests.

Comment by David Nolen [ 16/Dec/16 1:56 PM ]

Thomas can we get this patch without the changes to to the API? i.e. leave no-clone stuff alone.

Comment by Mike Fikes [ 16/Dec/16 7:38 PM ]

For reference, here is an example use of the no-clone stuff that would break: https://github.com/cognitect/transit-cljs/blob/master/src/cognitect/transit.cljs#L94

Comment by David Nolen [ 17/Dec/16 6:28 AM ]

I accidentally applied this one. I went ahead and removed all API level deletions and changes. Thomas, for future reference never change existing APIs. Only additions and new names. Thanks.

Comment by David Nolen [ 17/Dec/16 6:29 AM ]

fixed https://github.com/clojure/clojurescript/commit/24f4189445d802fcd3155855cf5f51e4c1902785





[CLJS-1812] cljs.spec.test.check has the side effect of instrumenting vars it's called on Created: 05/Oct/16  Updated: 16/Dec/16  Resolved: 16/Dec/16

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: JR Heard Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File patch-0001.patch    

 Description   

When using (stest/check) to test a function, I noticed that calls to the function later on in my code behaved as if the function were instrumented, even though I never explicitly called (stest/instrument) on it.

I think that there's something wrong with this line: https://github.com/clojure/clojurescript/blob/7e90ad5/src/main/cljs/cljs/spec/test.cljc#L157 .

As far as I can tell from poking around at this code, re-inst# is always true, because in cljs.spec.test, calling (unstrument `foo/bar) will return [foo/bar] even if foo/bar is not currently instrumented. From what I can tell from poking around in the REPL, this is a departure from Clojure's behavior, which is to return [] in this situation.

As always, it's possible I'm insane or misinterpreting how this system is intended to behave; but for what it's worth, the print statements I've added to my local copy of Clojurescript indicate that if you call (stest/check `foo/bar) when @instrumented-vars is {}, re-inst# will be true, and `foo/bar will therefore be instrumented at the end of check's execution. This seems like it's probably a bug.



 Comments   
Comment by JR Heard [ 06/Oct/16 10:16 AM ]

I think I'd actually like to take a stab at this one, if that's all right.

Comment by JR Heard [ 06/Oct/16 3:03 PM ]

Added a patch. Let me know if I goofed in its formatting, commit message style, etc. Includes two tests that fail before this change and pass after it.

I had trouble writing the second test assertion in the way I wanted - (is (not (thrown? js/Error (h-cljs-1812 "foo")))) didn't seem to do the trick - so I ended up just calling the helper function with invalid args directly; please let me know if there's a better way

Comment by David Nolen [ 16/Dec/16 2:35 PM ]

fixed https://github.com/clojure/clojurescript/commit/d1b8b31f7247688098d9e61bb33302a6edc57c2c





[CLJS-1786] Add knob for controlling printing of namespaced maps Created: 21/Sep/16  Updated: 16/Dec/16  Resolved: 16/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Task Priority: Major
Reporter: David Nolen Assignee: Lauri Oherd
Resolution: Completed Votes: 0
Labels: newbie

Attachments: Text File CLJS-1786.patch    

 Description   

See these Clojure commits:

https://github.com/clojure/clojure/commit/d57b5559829be8e8b3dcb28a20876b32615af0cb
https://github.com/clojure/clojure/commit/b49c1984a1527d17951fbb23ddf9406805a1343f
https://github.com/clojure/clojure/commit/05a8e8b323042fa043355b716facaed6003af324



 Comments   
Comment by Lauri Oherd [ 28/Sep/16 4:30 PM ]

Added print-namespace-maps flag for controlling printing of namespaced maps.
Unlike in Clojure repl the default value is false in Clojurescript repl.
Please comment if this needs to be fixed.

Comment by David Nolen [ 28/Sep/16 4:44 PM ]

Why should the default be different from Clojure?

Comment by Lauri Oherd [ 06/Oct/16 10:59 AM ]

I couldn't figure out initially how to set the print-namespace-maps value to true only in repl environment.
This is resolved in an updated patch. Please check if the implementation is acceptable - in file src/main/clojure/cljs/repl.cljc line 558 the following statement is executed: (set! cljs.core.print-namespace-maps true)

Comment by António Nuno Monteiro [ 06/Oct/16 11:05 AM ]

I don't think that's the desired approach for `print-namespace-maps` to work in every REPL. The wrap-fn is not used for every REPL, as custom REPLs may specify their own.

Why don't you bind the dynamic variable where all others are being bound?

Comment by Lauri Oherd [ 06/Oct/16 1:20 PM ]

Thank you for reviewing the previous patch and suggesting a better approach, António! Updated the patch with suggested solution.

Comment by David Nolen [ 16/Dec/16 1:59 PM ]

fixed https://github.com/clojure/clojurescript/commit/95fd110f55c57b890422763ed8f2644cfbf159de





[CLJS-1836] nth doesn't throw for IndexedSeqs Created: 25/Oct/16  Updated: 16/Dec/16  Resolved: 16/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-1836.patch    
Patch: Code and Test

 Description   

Examples:

(nth (seq (array 0 1 2 3)) 4) => nil (expected js/Error)
(nth (seq (array 0 1 2 3)) -1) => nil (expected js/Error)
(nth (seq [0 1 2 3]) 4 :not-found) => nil (expected :not-found)
(nth (seq [0 1 2 3]) -1 :not-found) => nil (expected :not-found)

This only affects sequences of javascript arrays, strings and small (<= 32 elements) PersistentVectors.



 Comments   
Comment by David Nolen [ 16/Dec/16 1:49 PM ]

fixed https://github.com/clojure/clojurescript/commit/3b3db232711266f9f41c94c777084664d6d0b71b





[CLJS-1872] Remove warnings from (:require [foo :refer-macros [bar]) when (:require-macros [foo :refer [bar]) works Created: 14/Dec/16  Updated: 15/Dec/16  Resolved: 15/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Greg Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: warning
Environment:

macOSX Sierra, and Ubuntu 16.04, but I suspect its platform independent.



 Description   

Both ns forms

(:require [foo :refer-macros [read-file])
and
(:require-macros [foo :refer [read-file])

work, but the :require form raises a WARNING: Use of undeclared Var cljs.core/slurp at line 5 src/pairwise/cljsmacros.cljc

where cljsmacros contains

(defmacro read-file [file]
(clojure.core/slurp file))



 Comments   
Comment by António Nuno Monteiro [ 14/Dec/16 10:11 AM ]

Not sure this is a bug. `slurp` is not in `cljs.core`, which is why you get the warning.

Comment by David Nolen [ 15/Dec/16 9:32 AM ]

not an issue





[CLJS-1871] A declare with :arglists should generate static function calls Created: 14/Dec/16  Updated: 14/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance


 Description   

This is performance enhancement.

  1. Problem
    After a declare the compiler doesn't know which arities the function will be defined with and hence generates code that checks if that arity is defined and then either calls it or uses a slower `xy-fn.call(null, ...)` construct. This is not optimal since it can be slower and also generates slightly more code.

Especially functions which only have one arity are problematic since they will end up being called with `xy-fn.call`.

  1. Affects
    Code that uses a function that was only declared and not def'ed yet. Such as `cons` in `IndexedSeq` or `conj!` in `TransientHashMap`.
    1. Performance
      A preliminary benchmark showed neglible improvements in Chrome v54 but a significant (factor of 2.2) performance benefit in Firefox.
  1. Solution
    Most of the declares should use `(def ^{:declare true, :arglists '([x xs])} cons)` and the compiler should take the `:arglists` into consideration and emit direct function calls instead.





[CLJS-1870] Quoted specs check in require macro symbols Created: 11/Dec/16  Updated: 13/Dec/16  Resolved: 13/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1870.patch    
Patch: Code and Test

 Description   

If you use the new require macro and fail to quote the lib specs, it will indicate such. But the check will derail for a symbol lib spec.

For example: (require [clojure.set]) causes a diagnostic: "Arguments to require must be quoted. Offending spec: [clojure.set]".
On the other hand: (require clojure.set) doesn't produce the expected diagnostic, but derails with "Don't know how to create ISeq from: clojure.lang.Symbol".



 Comments   
Comment by David Nolen [ 13/Dec/16 1:05 PM ]

fixed https://github.com/clojure/clojurescript/commit/cfbaba0f7f2f9f918896528ca8e814831218458b





[CLJS-1869] Regression importing goog.Uri Created: 11/Dec/16  Updated: 13/Dec/16  Resolved: 13/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: António Nuno Monteiro
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1869-2.patch     Text File CLJS-1869.patch    
Patch: Code and Test

 Description   

With the latest release, a regression was introduced which breaks importing goog.Uri.

To repro, download the released cljs.jar and then

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56132
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.293"
cljs.user=> (import 'goog.Uri)
nil
cljs.user=> (Uri. "http://example.com")
WARNING: Use of undeclared Var cljs.user/Uri at line 1 <cljs repl>
WARNING: Use of undeclared Var cljs.user/Uri at line 1 <cljs repl>
TypeError: cljs.user.Uri is not a constructor
    at repl:1:90
    at repl:9:3
    at repl:14:4
    at ContextifyScript.Script.runInThisContext (vm.js:26:33)
    at Object.exports.runInThisContext (vm.js:79:17)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:221:14)
    at Socket.<anonymous> ([stdin]:49:25)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)

For reference, here is it working with the previous release:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 55926
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.230"
cljs.user=> (import 'goog.Uri)
nil
cljs.user=> (Uri. "http://example.com")
#object[Object http://example.com]
cljs.user=> (.getScheme *1)
"http"


 Comments   
Comment by Mike Fikes [ 11/Dec/16 4:52 PM ]

Hey António, assigning over to you for a patch review to see if you have any views on the attached.

Comment by Mike Fikes [ 11/Dec/16 5:48 PM ]

António pointed out that canonicalize-specs was likely not defective as it was preserved in the revisions. The root cause appears to be that the original REPL code did not call canonicalize-specs for its import REPL special, but instead had applied a somewhat simpler canonicalization function literal for that case.

Attaching a second revision patch that introduces a new canonicalize-import-specs that is faithful to the original REPL code and also as a benefit re-adds support for doing things like (import '(goog.Uri)).

Comment by David Nolen [ 13/Dec/16 1:01 PM ]

fixed https://github.com/clojure/clojurescript/commit/3ca51dc947de98b9b83949356e30ecd307bc5088





[CLJS-1653] cljs.spec: keys* causes exception Created: 29/May/16  Updated: 13/Dec/16  Resolved: 29/Jul/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1653-2.patch     Text File CLJS-1653.patch    
Patch: Code

 Description   

From https://clojure.org/guides/spec there is a keys* usage in the Entity Maps section. If tried with cljs.spec an exception is thrown:

cljs.user=> (require '[cljs.spec :as s])
nil
cljs.user=> (s/def ::server (s/keys* :req [::id ::host] :opt [::port]))
clojure.lang.ExceptionInfo: No method in multimethod 'parse' for dispatch value: & at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 17, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4617)
	at clojure.core$ex_info.invoke(core.clj:4617)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:580)
	at cljs.analyzer$error.invoke(analyzer.cljc:576)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2420)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:2419)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2442)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2443)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2613)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2612)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$parse_invoke_STAR_$ana_expr__2106.invoke(analyzer.cljc:2257)
	at clojure.core$map$fn__4785.invoke(core.clj:2646)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.PersistentVector.create(PersistentVector.java:73)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:44)
	at clojure.core$vec.invokeStatic(core.clj:377)
	at clojure.core$vec.invoke(core.clj:367)
	at cljs.analyzer$parse_invoke_STAR_.invokeStatic(analyzer.cljc:2264)
	at cljs.analyzer$parse_invoke_STAR_.invoke(analyzer.cljc:2235)
	at cljs.analyzer$parse_invoke.invokeStatic(analyzer.cljc:2273)
	at cljs.analyzer$parse_invoke.invoke(analyzer.cljc:2271)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2417)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:2414)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2421)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:2419)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2442)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2443)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:2423)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:2555)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:2551)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:2602)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:2593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:2618)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:2605)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:450)
	at cljs.repl$evaluate_form.invoke(repl.cljc:440)
	at cljs.repl$eval_cljs.invokeStatic(repl.cljc:570)
	at cljs.repl$eval_cljs.invoke(repl.cljc:563)
	at cljs.repl$repl_STAR_$read_eval_print__6148.invoke(repl.cljc:875)
	at cljs.repl$repl_STAR_$fn__6154$fn__6163.invoke(repl.cljc:914)
	at cljs.repl$repl_STAR_$fn__6154.invoke(repl.cljc:913)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1154)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1145)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:877)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:761)
	at cljs.repl$repl.invokeStatic(repl.cljc:995)
	at cljs.repl$repl.doInvoke(repl.cljc:925)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at user$eval6335.invokeStatic(NO_SOURCE_FILE:3)
	at user$eval6335.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.eval(Compiler.java:6890)
	at clojure.core$eval.invokeStatic(core.clj:3105)
	at clojure.main$eval_opt.invokeStatic(main.clj:288)
	at clojure.main$eval_opt.invoke(main.clj:282)
	at clojure.main$initialize.invokeStatic(main.clj:308)
	at clojure.main$null_opt.invokeStatic(main.clj:342)
	at clojure.main$null_opt.invoke(main.clj:339)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: No method in multimethod 'parse' for dispatch value: &
	at clojure.lang.MultiFn.getFn(MultiFn.java:156)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2416)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:2414)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2421)
	... 85 more


 Comments   
Comment by Mike Fikes [ 30/May/16 10:55 AM ]

The attached patch works around the issue by qualifying the reference to cljs.spec/&.

With it, the example in the docstring works:

(s/conform (s/cat :i1 integer? :m (s/keys* :req-un [::a ::c]) :i2 integer?) [42 :a 1 :c 2 :d 4 99])
{:i1 42, :m {:a 1, :c 2, :d 4}, :i2 99}

(There is probably a more correct solution that probably involves changing the analyzer, which would lead to users being able to refer & and use it as (& ...), but this patch would get us by for now if desired.)

Comment by Mike Fikes [ 27/Jul/16 10:05 AM ]

Original patch no longer applies; attaching CLJS-1653-2.patch.

Comment by David Nolen [ 29/Jul/16 3:13 PM ]

fixed https://github.com/clojure/clojurescript/commit/5a78c9fb881f08b1f9578f9cd9dd265bbdd5c720

Comment by Stanislav Gurenkov [ 12/Dec/16 6:02 PM ]

Still getting this error, looks like patch was not merged properly.

Comment by David Nolen [ 13/Dec/16 12:59 PM ]

Thanks fixed in the master.





[CLJS-1461] Convert analyzer to conform to tools.analyzer's spec Created: 28/Sep/15  Updated: 12/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Ambrose Bonnaire-Sergeant
Resolution: Unresolved Votes: 6
Labels: None


 Description   

tools.analyzer has a handy common AST format for map-based analysis results. We should use this format in the ClojureScript analyzer so tooling that already accepts this format can work seamlessly with ClojureScript.

Work in progress: https://github.com/frenchy64/clojurescript/tree/reflect






[CLJS-1867] Analyzer error when running a project with compiler build from master Created: 10/Dec/16  Updated: 11/Dec/16  Resolved: 11/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Roman Liutikov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

I was about to test a new externs inference on a sample project and got the following analyzer error:

----  Could not Analyze  resources/public/js/compiled/out/sablono/interpreter.cljc   line:84  column:12  ----

   at line 84 resources/public/js/compiled/out/sablono/interpreter.cljc

  82  (when (some? props)
  83    (when (nil? (.-value props))
  84      (set! (.-value props) js/undefined))
          ^---  at line 84 resources/public/js/compiled/out/sablono/interpreter.cljc
  85    (when (nil? (.-checked props))
  86      (set! (.-checked props) js/undefined)))
  87  (if (empty? children)

ClojureScript build is done from commit 97aa4303f46.

Project dependencies are:

[[org.clojure/clojure "1.8.0"]
 [org.clojure/clojurescript "1.9.363"]
 [rum "0.10.7"]]

The code:

(ns app.core
  (:require [rum.core :as rum]))

Note that this compiles without errors with the latest ClojureScript release 1.9.293



 Comments   
Comment by David Nolen [ 11/Dec/16 9:08 AM ]

There's currently very little useful information in this issue. Do not report error information not directly produced by the ClojureScript compiler. This issue will be closed shortly if it's not corrected. Thanks.

Comment by David Nolen [ 11/Dec/16 9:24 AM ]

I've probably fixed the issue in master. But in the future, issues must be more informative and more minimal.





[CLJS-1806] build api fails to generate inline code for :target :nodejs Created: 01/Oct/16  Updated: 09/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.76
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

when inline code is provided as vector to the composition of `cljs.build.api/build` and `cljs.build.api/inputs` methods under `:target :nodejs` the provided inline code is not output.

;; this outputs code
(cljs.build.api/build (cljs.build.api/inputs '[(ns hello.core) (+ 1 2)]) {}) 

;; this does not output inline code
(cljs.build.api/build (cljs.build.api/inputs '[(ns hello.core) (+ 1 2)]) {:target :nodejs}) 


;; When you don't use cljs.build.api/inputs everything works correctly
(cljs.build.api/build '[(ns hello.core) (+ 1 2)] {}) ;; this outputs code

(cljs.build.api/build '[(ns hello.core) (+ 1 2)] {:target :nodejs}) ;; this outputs code


 Comments   
Comment by Bruce Hauman [ 30/Oct/16 11:31 AM ]

From @ykomatsu on Github:

add-preloads seems to remove cljs/nodejs.cljs.

https://github.com/clojure/clojurescript/blob/ab7a4911f1fd3a81210b1a9f2d84857748f8268b/src/main/clojure/cljs/closure.clj#L897

This patch will fix this problem but I am not sure if this is correct solution.

https://github.com/ykomatsu/clojurescript/commit/fc986467e66e6a628dc8f0e8a2ef2b30f715fd23

Comment by Dusan Maliarik [ 09/Dec/16 2:15 PM ]

Would anyone from the team please look at the patch? Thank you

Comment by David Nolen [ 09/Dec/16 6:22 PM ]

Please attach a patch to the ticket for review. Linking out of JIRA is not desirable. Thanks.





[CLJS-1865] Google Closure Compiler in JavaScript Created: 06/Dec/16  Updated: 06/Dec/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.36
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Google released its JavaScript version of the Closure Compiler – "this allows Closure Compiler to run entirely in JS. Java is not required":

Switching to the JS compiler means JS devs coming to ClojureScript will be able to use the tools they're familiar with a simplify the onboarding docs on the website.

NB: I discovered this while experimenting with using ClojureScript with Polymer:






[CLJS-1718] Foreign lib files should be placed in a location that matches their namespace Created: 29/Jul/16  Updated: 02/Dec/16  Resolved: 02/Dec/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Affects master



 Description   

Using several foreign libs with the same file name ends up just including one of them, as the files are placed at the root of the `:output-dir`.

A solution for this would be placing those files in a location that matches their `:provides` namespace.



 Comments   
Comment by David Nolen [ 02/Dec/16 5:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/97d2d61e78ce747d02d0e5b2ced706f6fb68ec4e





[CLJS-1863] :reload/:reload-all issue with .cljc runtime/macro nses Created: 29/Nov/16  Updated: 29/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Brandon Bloom discovered an issue where a ns that is used both from runtime and for macros (where the macros are self-required) won't respect `(require ... :reload/:reload-all)`.



 Comments   
Comment by David Nolen [ 29/Nov/16 3:59 PM ]

Whatever we do needs to be copied over into a bootstrapped, but that's a separate issue.





[CLJS-1862] allow NodeJS's NODE_MODULES to be set as a REPL option Created: 28/Nov/16  Updated: 29/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Marc Daya Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: nodejs, repl

Attachments: Text File 65.patch    
Patch: Code

 Description   

The NodeJS REPL that ships with ClojureScript seems to assume that all NodeJS modules are installed globally, or that node's NODE_PATH environment variable is set for the process that starts the REPL (e.g. CIDER). Allowing this to be set as a REPL option make it possible for modules to be installed and made available to the REPL by build tooling, eliminating manual steps by the user and improving repeatability.



 Comments   
Comment by David Nolen [ 28/Nov/16 4:26 PM ]

Thanks. Have you submitted your Clojure CA yet?

Comment by Marc Daya [ 29/Nov/16 2:02 PM ]

It has just been filed.





[CLJS-1860] Resolve JS modules referred by their fully-qualified namespace Created: 24/Nov/16  Updated: 28/Nov/16  Resolved: 28/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1860.patch    
Patch: Code

 Description   

This is part 2 of CLJS-1848. When a JS module is used in a macro, the analyzer should find the correct `full-ns` if the module is fully qualified.

Without this patch, downstream namespaces that use the macro would need to explicitly require the JS module. This shouldn't be necessary if the module has already been required in the namespace that declares the macro.

This patch doesn't introduce any new behavior. Instead, it mimics the current CLJS namespaces behavior for JS modules.



 Comments   
Comment by David Nolen [ 28/Nov/16 8:46 AM ]

fixed https://github.com/clojure/clojurescript/commit/79a20afe360249ab6cb652f4465b7ccd01a923f2





[CLJS-1861] Use usr/bin/env in build scripts for portability Created: 25/Nov/16  Updated: 28/Nov/16  Resolved: 28/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1861.patch    

 Description   

There are a couple of build scripts where

#!/bin/bash
could be converted to
#!/usr/bin/env bash
for additional portability.



 Comments   
Comment by David Nolen [ 28/Nov/16 8:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/170fd767752a4839b25038c86b2d6a6aa3b25ab7





[CLJS-1858] Should allow `:cache-analysis true` and `cache-analysis-format nil` Created: 21/Nov/16  Updated: 22/Nov/16  Resolved: 22/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1858.patch    
Patch: Code

 Description   

This was intended but doesn't actually work.



 Comments   
Comment by David Nolen [ 22/Nov/16 1:00 PM ]

fixed https://github.com/clojure/clojurescript/commits/master





[CLJS-1857] Fix self-host tests Created: 20/Nov/16  Updated: 21/Nov/16  Resolved: 21/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1857.patch    
Patch: Code

 Description   

self-hosted tests are broken in master given the latest externs inference commits.



 Comments   
Comment by Mike Fikes [ 20/Nov/16 1:23 PM ]

+1 LGTM and script/test-self-parity, script/test-self-host, and script/test all pass for me (apart from the expected Nashorn error we have been seeing recently)

Comment by David Nolen [ 21/Nov/16 3:00 PM ]

fixed https://github.com/clojure/clojurescript/commit/e156e74fcd1dfa70fb0f35d187825dd9b131431c





[CLJS-1856] Self-host: load-deps doesn't delegate to itself Created: 17/Nov/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1856.patch    
Patch: Code

 Description   

The first arity of cljs.js/load-deps should be calling the second arity of itself, but it is currently inadvertently making a call to cljs.js/analyze-deps.

Note: This arity is not being used. The arguments being passed along were properly updated with CLJS-1826, but it is now actually inadvertently calling analyze-deps with an arity error. (The real fix is to make it call load-deps).



 Comments   
Comment by Alex Miller [ 18/Nov/16 7:54 PM ]

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/e01b8a0366f5c0519a6fa49025592bb345718235





[CLJS-1855] Subvec should implement IIterable Created: 17/Nov/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1855.patch    
Patch: Code

 Description   

Currently calling `iter` on a `Subvec` falls back to `seq-iter`. We can obtain performance equal to `PersistentVector` iteration by using `ranged-iterator`. This also brings Subvecs more inline with the Clojure implementation.

Benchmark:

(simple-benchmark [sv (subvec (into [] (range 1000000)) 0 1000000)]
                  (let [i (iter sv)]
                    (loop []
                      (if (.hasNext i)
                        (do (.next i)
                            (recur)))))
                  100)
Engine Master (ms) Patch (ms) Gain (master/patch)
V8 41979 3550 11x
jsc 29348 2405 12x

`ChunkedSeqs` could gain the same performance boost by implementing IIterable and utilizing `ranged-iterator` however this would deviate from Clojure implementation wise.



 Comments   
Comment by Alex Miller [ 18/Nov/16 7:54 PM ]

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/d2cb3eeca6f37ef36e28be9975db3816d04355a1





[CLJS-1643] Emit more informative error when emitting a type which has no emit multimethod case Created: 21/May/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1643.patch    
Patch: Code and Test

 Description   

Users may accidentally splice in Clojure functions and other things in macros that work in Clojure which cannot work in ClojureScript. We may want to include a default warning for the most common/likely error cases.



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 8:27 AM ]

Attached patch with fix and test.

Comment by Alex Miller [ 18/Nov/16 7:52 PM ]

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/417350ddabea283ef8f576b8e361a249d9bfb9e7





[CLJS-1816] Basic timing info in verbose output Created: 11/Oct/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: compiler, newbie

Attachments: Text File CLJS-1816.patch    
Patch: Code

 Description   

When logging verbose output during compilation of individual namespaces we could include basic timing information to make it easier to spot slow-to-compile namespaces.



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 6:42 AM ]

Attached patch with fix.

Comment by Alex Miller [ 18/Nov/16 7:53 PM ]

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/6602f769ed4d52fd67577aacaf9cfe6db05b8ef3





[CLJS-1616] Self-host: improve documentation for compile-str Created: 06/Apr/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: Yehonathan Sharvit Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-1616.patch    
Patch: Code

 Description   

It's not clear at all how to use the `opts` arguments for compile-str.

In the code - https://github.com/clojure/clojurescript/blob/c3899acf797eb6779c53b313f5606c5018360b83/src/main/cljs/cljs/js.cljs#L660 - we
only have
:load - library resolution function, see load-fn
:source-map - set to true to generate inline source map information

In fact, there is also :verbose and ::def-emits-var

They are not documented.

Are there more options?



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 8:53 AM ]

Attached patch with fix.

Comment by Alex Miller [ 18/Nov/16 7:52 PM ]

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/33a7e5bcac763d40ca684404cf772e9745d264a





[CLJS-901] Protocolize compiler access to the file system Created: 03/Dec/14  Updated: 18/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None


 Description   

Currently builds are based on files on disk. It is desirable to be able to instead get in memory builds, WebDAV based builds, S3 based builds, etc. Many of these alternative strategies are not in scope for the ClojureScript compiler but this does not mean we should not supply the needed hooks for users to control the behavior.



 Comments   
Comment by Thomas Heller [ 03/Dec/14 9:31 AM ]

This and some other issues opened recently (CLJS-900, CLJS-851, CLJS-899, ...) have some overlap with what I have done in shadow-build [1]. Memory based builds are actually already possible cause it will only touch the disk when asked to, although the API could use some cleanup.

Anyways, might be worthwhile to coordinate these efforts to make CLJS more accessible for everyone.

[1] https://github.com/thheller/shadow-build

Comment by Alan Dipert [ 04/Feb/15 11:36 AM ]

I too think it would be totally awesome to have builds based on sources from disparate places.

One alternative in this spirit I have been thinking about is a "SourceSet" approach. The idea is, instead of teaching CLJS how to consume various place-types directly via protocols, provide an API for building a "SourceSet" value and also a build function that takes the SourceSet as input. I imagine the SourceSet in its simplest form as a map of namespaces to string sources.

With a value to represent sources that is place-agnostic and immutable, 3rd party tools can consume/emit/transform these values before invoking a compile without knowledge or interest in CLJS internals. Conversely CLJS need not be concerned with how SourceSets are constructed.

This whole idea is inspired by boot's FileSets, which work basically the same but can't have the "it fits in memory" assumption.





[CLJS-1854] Self-host: Reload ns with const Created: 16/Nov/16  Updated: 16/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap


 Description   

Bootstrapped ClojureScript fails to allow you to reload a namespace containing a constant.

To reproduce, evaluate the following forms in a REPL:

(require 'cljs.js)

(def st (cljs.js/empty-state))

(cljs.js/eval st
  '(require (quote foo.core))
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 1)"}))}
  prn)

(cljs.js/eval st
  '(require (quote foo.core) :reload)
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 2)"}))}
  prn)

The expectation is that the :reload directive in the last require will allow the namespace to be loaded with the const def being re-defined.

Instead, you get the following in the eval callback:

{:error #error {:message "Could not eval foo.core", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't redefine a constant at line 1 ", :data {:file nil, :line 1, :column 15, :tag :cljs/analysis-error}}}}

Note: This has probably been a defect in bootstrapped ClojureScript for quite a while (maybe forever). In particular, it is not a regression introduced with the new require capability (CLJS-1346).

FWIW, Planck has been working around this (and violating public API), manipulating cljs.js/*loaded* via its require REPL special, essentially purging portions of the analysis cache when reloading: https://github.com/mfikes/planck/blob/1.17/planck-cljs/src/planck/repl.cljs#L329-L348






[CLJS-1758] QuickStart guide fails at browser REPL step with "TypeError: parentElm is null" Created: 18/Aug/16  Updated: 16/Nov/16  Resolved: 18/Aug/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

When I follow http://clojurescript.org/guides/quick-start up to the browser REPL step, I get an error in the browser console and the REPL never connects. The browser error is TypeError: parentElm is null on line 482 of crosspagechannel.js: parentElm.appendChild(iframeElm);.

I have put up a self-contained example to demonstrate the bug: https://github.com/timmc/sscce-CLJS-1758

  • I am using this command to run the REPL: rlwrap java -cp cljs-1.9.216.jar:src clojure.main repl.clj
  • Refreshing the browser does not help.
  • Using Chromium 52.0.2743.116 instead of Firefox ESR 45.3.0 does not help.
$ java -version
java version "1.8.0_31"
Java(TM) SE Runtime Environment (build 1.8.0_31-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.31-b07, mixed mode)

$ uname -a
Linux bc-timmc 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2+deb8u3 (2016-07-02) x86_64 GNU/Linux
(Debian GNU/Linux 8.5)


 Comments   
Comment by Tim McCormack [ 18/Aug/16 11:27 AM ]

CLJS 1.9.89 fails the same way.

Comment by António Nuno Monteiro [ 18/Aug/16 11:51 AM ]

Not a bug. Your index.html file needs to at least have a `<body>` tag.
All html examples in the quickstart provide valid HTML, you just chose not to use it.

Comment by Tim McCormack [ 18/Aug/16 12:41 PM ]

Ah, of course! I compared the other files with the demo, but not that one!

I could have sworn the body element was always created automatically in the DOM whether or not it was declared. TIL.

(And confirmed, it works now.)

ETA: Not sure which resolution to pick, so leaving open.

Comment by HU Ze [ 16/Nov/16 4:17 AM ]

I am facing same problem before, I think you put your <script> in <head> and actually it MUST be put in <body>. To make it simply, just copy and paste from the Quick Start.





[CLJS-1852] explain-data problems don't provide resolved symbols under :pred Created: 15/Nov/16  Updated: 15/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Same as http://dev.clojure.org/jira/browse/CLJ-2059 which has a patch.






[CLJS-1846] Range issues Created: 10/Nov/16  Updated: 15/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1846.2.patch     Text File CLJS-1846.patch    
Patch: Code and Test

 Description   

Problem
There are a number of bugs with Range which occur when the step size is 0 or where negative.

Examples

cljs.user=> (count (range 10 0 0))
-Infinity  ;Expect Infinity

cljs.user=> (nth (range 10 0 -1) -1)
11 ; Expected IndexOutOfBounds

cljs.user=> (take 5 (sequence identity (range 0 10 0)))
() ; Expected (0 0 0 0 0)

cljs.user=> (into [] (take 5) (range 0 10 0))
[] ; Expected [0 0 0 0 0]


 Comments   
Comment by David Nolen [ 10/Nov/16 4:37 PM ]

This patch is headed in the right direction but it needs to be more vigilant about performance. I'm more than happy to talk it over via IRC or Slack. Thanks!

Comment by Thomas Mulvaney [ 15/Nov/16 8:24 AM ]

Updated patch with performance tweaks.

  • Added the ^boolean annotation to the `some-range?` helper.
  • Removed calls to methods of Range where possible.
  • Improved 2-arity reduces performance over master significantly by replacing the call to ci-reduce.




[CLJS-1651] Self-host: Cannot replace core macro-function Created: 28/May/16  Updated: 14/Nov/16  Resolved: 14/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1651.patch    
Patch: Code and Test

 Description   

Replace double to multiply by two in self host and you will see that operator-position resolution chooses the core macro:

ClojureScript Node.js REPL server listening on 54425
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval-str st "(defn double [x] (* 2 x))" nil {:eval cljs.js/js-eval :context :expr} identity)
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1
{:ns cljs.user, :value #object[cljs$user$double "function cljs$user$double(x){
return ((2) * x);
}"]}
cljs.user=> (cljs.js/eval-str st "[(double 3) (apply double [3])]" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value [3 6]}

The correct result above would be [6 6].



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 11:27 AM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 13/Nov/16 12:07 PM ]

Confirmed that António's patch fixes things downstream in Planck.

Comment by David Nolen [ 14/Nov/16 1:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/4abcec8b7af601cb21342a559f5ee731fb19f7ff





[CLJS-1764] Double warning for undeclared Var Created: 26/Aug/16  Updated: 14/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1764.patch    
Patch: Code

 Description   

A regression occurred where an undeclared Var in a {{require}}d file causes two diagnostics:

$ more src/foo/core.cljs
(ns foo.core)

(def x 2)

abc
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.227.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52749
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.227"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.211.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56704
To quit, type: :cljs/quit
cljs.user=>  *clojurescript-version*
"1.9.211"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit


 Comments   
Comment by David Nolen [ 16/Sep/16 2:04 PM ]

If somebody wants to do a git bisect to sort this one out, that would be awesome

Comment by António Nuno Monteiro [ 07/Nov/16 9:34 AM ]

Only seems to happen at the REPL

Comment by António Nuno Monteiro [ 13/Nov/16 3:47 PM ]

Patch with fix.

This only happened when `require`ing at the REPL. Required namespaces ended up being analyzed twice, once in `cljs.repl` and once in `cljs.closure`. The patch adds wraps compiling these NSes in `cljs.closure` in a `cljs.analyzer/no-warn`.

Comment by David Nolen [ 14/Nov/16 9:24 AM ]

How will this not effect non REPL cases?

Comment by António Nuno Monteiro [ 14/Nov/16 9:29 AM ]

I just now realized that it will probably affect those cases as well, although the `add-dependencies` function seems to (currently) only be used in `cljs.repl`. What other approach should I try? Restrict the cases where we

*analyze-deps*
at the REPL?

Comment by Thomas Heller [ 14/Nov/16 9:51 AM ]

FWIW I don't think this is related to the REPL at all.

I have been seeing doubled warnings for a while now in shadow-build but never bothered to find you why.

abc

(defn x [y] xyz)

Will always warn twice about "xzy" but only once for "abc", doesn't matter if a REPL is involved or not.





[CLJS-1848] Analyzer can't find JS modules during macro-expansion Created: 12/Nov/16  Updated: 13/Nov/16  Resolved: 13/Nov/16

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: