<< Back to previous view

[CLJS-910] JavaScriptCore 0xbbadbeef EXC_BAD_ACCESS when evaluating (list 0 1 ... 18) Created: 16/Dec/14  Updated: 20/Dec/14

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: None
Environment:

Embedded JavaScriptCore on iOS simulator
Connected via Weasel / simple-brepl
:whitespace optimization


Attachments: PNG File memory.png     Text File stacktrace.txt    

 Description   

If I evaluate

(list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)

JavaScriptCore exhibits a 0xbbadbeef EXC_BAD_ACCESS, with a fairly deep stacktrace:

(lldb) bt

  • thread #1: tid = 0x3f7e, 0x0111e583 JavaScriptCore`WTFCrash + 67, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x0111e583 JavaScriptCore`WTFCrash + 67
    frame #1: 0x011395a9 JavaScriptCore`WTF::fastMalloc(unsigned long) + 1929
    frame #2: 0x00c9cb56 JavaScriptCore`WTF::Vector<JSC::UnlinkedInstruction, 0ul, WTF::UnsafeVectorOverflow>::expandCapacity(unsigned long) + 86
    frame #3: 0x00c90f27 JavaScriptCore`JSC::BytecodeGenerator::emitGetById(JSC::RegisterID*, JSC::RegisterID*, JSC::Identifier const&) + 311
    frame #4: 0x00fd4617 JavaScriptCore`JSC::DotAccessorNode::emitBytecode(JSC::BytecodeGenerator&, JSC::RegisterID*) + 551
    ...

(Full stack trace attached as stacktrace.txt)

This only occurs with :whitespace optimization and does not under :advanced.

If you evaluate (list 0), it works, and so does (list 0 1), all the way up to 17. Interestingly, it gets progressively slower as you evaluate longer lists.



 Comments   
Comment by Mike Fikes [ 16/Dec/14 12:35 PM ]

While the EXC_BAD_ACCESS is arguably a bug in JavaScriptCore, it is likely provoked by excessive memory usage of the (list 0 1 ...) form. The attached memory.png shows what appears to be 2^n memory usage for evaluating a list of size n. This graph was produced while REPLd into an iOS device, monitoring memory use from Xcode.

Comment by Mike Fikes [ 18/Dec/14 11:28 AM ]

The construct

(list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)

works in r2014 (released Nov 6, 2013), but fails in r2024 (released Nov 8, 2013).

In r2014, the emitted JavaScript is:

cljs.core.list.call(null, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18)

while in r2024 the emitted JavaScript is:

cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null,
cljs.core.List.EMPTY, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1), 0)

This particular commit, between r2014 and r2014 on Nov 7 2013, is likely when the emitted JavaScript changed (I haven't pulled down that specific revision):

https://github.com/clojure/clojurescript/commit/5bcbc4745f599e352c51e01b210755a88aa4bc5f#diff-b64165608bed8fb21a132890b4e2fca2R1279

Comment by Mike Fikes [ 18/Dec/14 12:07 PM ]

Knowing this, it is trivial to reproduce this in desktop Safari (and also see that it works fine in Chrome and Firefox).

If you go to clojurescript.net, or himera.herokuapp.com, and define a function that returns a list constructed with (list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18), you can see that those websites are built using ClojureScript r2014 or earlier, as cljs.core.list.call(null, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18) appears in the reader literal for the returned function.

But, with either of those sites, if you evaluate the following, you will cause a severe performance problem in Safari:

(conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj '() 21) 20) 19) 18) 17) 16) 15) 14) 13) 12) 11) 10) 9) 8) 7) 6) 5) 4) 3) 2) 1) 0)

My hope is that knowing this will make it easier to profile (using desktop tools) what is giving Safari grief executing the resulting JavaScript.

Comment by Mike Fikes [ 18/Dec/14 2:38 PM ]

I don't understand JavaScriptCore's evaluation strategy. I found that if you manually revise the deeply nested composition of cljs.core._conj.call(...) invocations to extract a temporary var or two, as below, then the "doubling" effect is cut short, and the code executes quickly.

This revised code essentially builds a list of length 13 first, and then conses on 3 more elements, and 3 more.

var list0 = cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null,cljs.core.List.EMPTY, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6);

var list1 = cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, list0, 5), 4), 3);

cljs.core._conj.call(null, cljs.core._conj.call(null, cljs.core._conj.call(null, list1, 2), 1), 0)

I found that I can cause something similar to occur from the ClojureScript side by adding a do form, as in:

(conj (conj (conj (conj (conj (do nil
(conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj (conj '() 21) 20) 19) 18) 17) 16) 15) 14) 13) 12) 11) 10) 9) 8) 7) 6) 5)) 4) 3) 2) 1) 0)

Comment by Mike Fikes [ 18/Dec/14 7:00 PM ]

This JavaScriptCore perf problem is easily reproduced purely with JavaScript:

inc=function( x ) { return x + 1; }

alert(
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, inc.call(null, inc.call(null, inc.call(null,
inc.call(null, 1))))))))))))))))))))))

Try this, at, say jsfiddle.net and you will see it max a core and use several GB in Safari, but not FireFox or Chrome.

Comment by Mike Fikes [ 19/Dec/14 9:00 AM ]

As indicated in this ticket's description, this problem doesn't occur with :advanced mode optimizations. Just to confirm, I produced the JavaScript with :pseudo-names set to true for (list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18), and you can see that it doesn't use the problematic "call" construct:

$cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$_conj$$($cljs$core$List$EMPTY$$, 18), 17), 16), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1), 0);

Correction 20-DEC-2014: Even with :advanced mode optimization, this occurs. The setting that is needed to avoid the "call" construct is {:static-fns true}, as was set for the above output. With {:static-fnc false}, the emitted coded under :advanced mode is:

$cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$_conj$$.call(null, $cljs$core$List$EMPTY$$, 18), 17), 17), 15), 14), 13), 12), 11), 10), 9), 8), 7), 6), 5), 4), 3), 2), 1);
Comment by Mike Fikes [ 19/Dec/14 9:09 AM ]

Since the fundamental problem is easily reproducible with pure JavaScript, I've opened a StackOverflow regarding it: http://stackoverflow.com/questions/27568249/javascriptcore-deeply-composed-call-performance

Comment by David Nolen [ 19/Dec/14 12:08 PM ]

That was the first thing I was going to suggest trying, repo in plain JavaScript. Thanks for doing this - I believe this may explain some oddities we've encountered elsewhere.

Comment by Mike Fikes [ 19/Dec/14 1:41 PM ]

I’ve filed rdar://19310764 with Apple, and copied it here for reference: http://openradar.appspot.com/radar?id=5864025753649152

Comment by Francis Avila [ 19/Dec/14 5:38 PM ]

Excellent debugging work. Shouldn't this be filed against webkit instead? http://www.webkit.org/quality/reporting.html. (I already searched for this issue in their bug tracker and found nothing.)

This still leaves open the question of whether CLJS should do anything to mitigate. Many of the cljs macros that shadow functions expand recursively--maybe they should expand to loops or reduces instead?

For example:

(defmacro list
  ([] '(.-EMPTY cljs.core/List))
  ([& xs]
    `(let [a# (array ~@(reverse xs))]
       (areduce a# i# l# (list)
         (. l# cljs$core$ICollection$_conj$arity$2 (aget a# i#))))))

Or maybe cheat and emit a ChunkedCons instead?

(defmacro cclist
  ([] '(.-EMPTY cljs.core/LIST))
  ([& xs]
    `(cljs.core/ChunkedCons.
       (cljs.core/ArrayChunk.
         (array ~@(reverse xs)) 0 ~(count xs))
       nil nil nil)))
Comment by Mike Fikes [ 19/Dec/14 6:54 PM ]

Thanks Francis. I've confirmed that it occurs in the WebKit Nightly r177573, and I've moved the ticket to https://bugs.webkit.org/show_bug.cgi?id=139847

Comment by Thomas Heller [ 20/Dec/14 4:19 AM ]

FWIW to toggle between fn.call(null, ...) and fn(...) you can use the compiler option {:static-fns true}. This works with all optimization levels (also :none) but I'm told it causes issues with the REPL. But if you don't use a REPL it is safe to use that option always, I have been for years. Maybe the best short term option.

Comment by Mike Fikes [ 20/Dec/14 8:43 AM ]

Given Thomas's comment, I now realize that my comments above regarding :whitespace and :advanced are incorrect. (My project.clj has a dev and release build, and in addition to a change in the optimization directive, I had a change in the :static-fns directive.)

The correction: This problem occurs for both :whitespace and :advanced, iff :static-fns is set to false.

Comment by David Nolen [ 20/Dec/14 3:22 PM ]

One thing I'm curious about is whether the issue manifests if you're not nesting calls to the same function? This should be easy to confirm with plain JavaScript.

Comment by Mike Fikes [ 20/Dec/14 4:26 PM ]

Hey David, it appears to still cause a problem even with distinct functions. (This fiddle exhibits the problem with distinct functions, so don't follow the link on mobile Safari: http://jsfiddle.net/mfikes/Lwr78cmk/1/ )





[CLJS-912] Minor enhancements to bootstrap script Created: 18/Dec/14  Updated: 20/Dec/14  Resolved: 20/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Alex Dowad Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Check-dependencies-in-bootstrap-script.patch     Text File 0001-Enhancements-to-bootstrap-script.patch     Text File 0002-Display-status-message-if-download-fails-while-boots.patch     Text File 0003-Retry-failed-downloads-in-bootstrap-script-before-gi.patch    
Patch: Code

 Description   

Just some small tweaks to make things a bit friendlier for first-time users. See the attached *.patch files.

I would like to remove the -s flag from invocations of curl as well, at least for the larger downloads. I like feedback about what a script is doing, while it's doing it. What do you think? Silence is golden?



 Comments   
Comment by Alex Dowad [ 18/Dec/14 1:29 PM ]

Just filled in and signed the Clojure CA.

Comment by David Nolen [ 18/Dec/14 1:39 PM ]

Great! Can we please get a single squashed patch? Thanks!

Comment by Alex Dowad [ 18/Dec/14 2:21 PM ]

OK, here is one squashed commit. I would have thought that you would like "semantic commits" – one commit for each conceptual change. I guess this stuff is too trivial to litter the history with a lot of fine-grained commits.

Comment by David Nolen [ 20/Dec/14 3:57 PM ]

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





[CLJS-914] thrown-with-msg? is unable to get message of exception Created: 19/Dec/14  Updated: 20/Dec/14  Resolved: 20/Dec/14

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

Type: Defect Priority: Major
Reporter: Matthew Boston Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 914.patch    

 Description   

In JavaScript, the message of an Error is retrieved with the Error.prototype.message property but the implementation of cljs.test/thrown-with-msg? uses the method .getMessage. This is causing the following error when writing tests with thrown-with-msg?.

Test console: expected: (thrown-with-msg? ArithmeticException #"Divide by zero" (\ 1 0))
Test console: actual:
Test console: #<TypeError: 'undefined' is not a function (evaluating 'e_5184auto_.getMessage()')>



 Comments   
Comment by Matthew Boston [ 19/Dec/14 5:31 PM ]

patch file for CLJS-914

Comment by David Nolen [ 20/Dec/14 3:35 PM ]

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





[CLJS-915] On empty call, List and PersistentQueue do not retain meta, sorted-set/sorted map do not retain comparator Created: 19/Dec/14  Updated: 20/Dec/14  Resolved: 20/Dec/14

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

Type: Defect Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: collections

Attachments: Text File cljs-915-fix-empty-on-collections.patch    

 Description   

-empty method is implemented incorrectly at List, PersistentQueue, PersistentSortedSet, PersistentSortedMap

(-> (sorted-set-by (comp - compare))
    empty
    (into [2 3 1])) => #{1 2 3} (should be #{3 2 1})
(-> (sorted-map-by (comp - compare))
    empty
    (into [[2 :b] [3 :c] [1 :a]])) => {1 :a, 2 :b, 3 :c} (should be backwards)
(meta (empty '^{:a 1} (1 2 3))) => nil (should be {:a 1})
(meta (empty (with-meta (.-EMPTY PersistentQueue) {:b :c}))) => nil (should be {:b :c})

Patch with tests attached: cljs-915-fix-empty-on-collections.patch



 Comments   
Comment by David Nolen [ 20/Dec/14 3:27 PM ]

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





[CLJS-913] Error when trying to convert javascript object to clojure (js->clj obj) under :advanced compilation Created: 18/Dec/14  Updated: 18/Dec/14

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

Type: Defect Priority: Major
Reporter: Erik Wickstrom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2496

Tested on OSX 10.10 with Chrome 39



 Description   

I'm trying to convert a javascript object (parsed JSON from a web service) into a clojure data structure. It works fine if I use :simple optimization with the closure compiler, however when I switch to :advanced optimization, I get the following error:

(let my-data #js {} ; see below for JSON
(.info js/console "converted to clojure" (str (js->clj my-data))))

Uncaught Error: No protocol method IEmptyableCollection.-empty defined for type object: [object Object]

Note that this also only seems to happen with this chunk of JSON and a few other samples (though according to all the JSON linters I've tried, it is valid). I've passed other input through without issue. So it is somewhat intermittent but deepish object hierarchies seem to be a commonality.

Here is the JSON (also posted here http://pastebin.com/PLffFrFf )

[{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"16 de Septiembre","short_name":"16 de Septiembre","types":["neighborhood","political"]},{"long_name":"Miguel Hidalgo","short_name":"Miguel Hidalgo","types":["sublocality_level_1","sublocality","political"]},{"long_name":"Ciudad de México","short_name":"México D.F.","types":["locality","political"]},{"long_name":"Distrito Federal","short_name":"D.F.","types":["administrative_area_level_1","political"]},{"long_name":"Mexico","short_name":"MX","types":["country","political"]}],"formatted_address":"16 de Septiembre, Miguel Hidalgo, 11810 Ciudad de México, D.F., Mexico","geometry":{"bounds":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}},"location":{"D":-99.20755880000002,"k":19.402037},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"West Jakarta","short_name":"West Jakarta","types":["locality","political"]},{"long_name":"Kamal","short_name":"Kamal","types":["administrative_area_level_4","political"]},{"long_name":"Kalideres","short_name":"Kalideres","types":["administrative_area_level_3","political"]},{"long_name":"West Jakarta City","short_name":"West Jakarta City","types":["administrative_area_level_2","political"]},{"long_name":"Jakarta","short_name":"Jakarta","types":["administrative_area_level_1","political"]},{"long_name":"Indonesia","short_name":"ID","types":["country","political"]}],"formatted_address":"Kamal, Kalideres, West Jakarta City, Jakarta 11810, Indonesia","geometry":{"bounds":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}},"location":{"D":106.70282500000008,"k":-6.101219},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["route"]},{"long_name":"Příbram District","short_name":"Příbram District","types":["administrative_area_level_2","political"]},{"long_name":"Central Bohemian Region","short_name":"Central Bohemian Region","types":["administrative_area_level_1","political"]},{"long_name":"Czech Republic","short_name":"CZ","types":["country","political"]},{"long_name":"261 01","short_name":"261 01","types":["postal_code"]}],"formatted_address":"11810, 261 01, Czech Republic","geometry":{"bounds":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}},"location":{"D":13.982032200000049,"k":49.7225575},"location_type":"GEOMETRIC_CENTER","viewport":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}}},"types":["route"]}]

Here is the original post to the mailing list: https://groups.google.com/forum/#!topic/clojurescript/MZ9esXbn2tg






[CLJS-889] 2 characters not supported in re-pattern: \u2028 \u2029 Created: 18/Nov/14  Updated: 18/Dec/14

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

Type: Defect Priority: Trivial
Reporter: Dave Sann Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2371"]
ubuntu


Attachments: Text File 0001-re-pattern-works-on-strings-containing-u2028-or-u202.patch    

 Description   

The following 2 patterns

(re-pattern "[\u2028]")
(re-pattern "[\u2029]")

Cause:

SyntaxError: Invalid regular expression: missing terminating ] for character class

They are probably somewhat rare characters in practice.

http://www.fileformat.info/info/unicode/char/2028/index.htm
http://www.fileformat.info/info/unicode/char/2029/index.htm



 Comments   
Comment by Alex Dowad [ 18/Dec/14 1:51 PM ]

The problem is that re-pattern uses a (.*) regexp to pick out the "pattern" portion of a regexp string (separate from the "flags" portion), and "." doesn't match \u2028 and \u2029.

Comment by Alex Dowad [ 18/Dec/14 2:13 PM ]

Here's a patch which fixes the problem (it would be better if there was a test too):

From 442867b5627c04b59902d2d133c6b17355639157 Mon Sep 17 00:00:00 2001
From: Alex Dowad <alexinbeijing@gmail.com>
Date: Thu, 18 Dec 2014 22:09:23 +0200
Subject: [PATCH] re-pattern works on strings containing \u2028 or \u2029

JavaScript RegExps don't match \u2028 or \u2029 with ., which was causing
the previous implementation of re-pattern to fail on those characters.
(It was as if the regexp was chopped short at the point where the offending
character appeared.)

src/cljs/cljs/core.cljs | 3 ++-
1 file changed, 2 insertions, 1 deletion

diff --git a/src/cljs/cljs/core.cljs b/src/cljs/cljs/core.cljs
index 525cde6..7c4de10 100644
— a/src/cljs/cljs/core.cljs
+++ b/src/cljs/cljs/core.cljs
@@ -7994,7 +7994,8 @@ reduces them without incurring seq initialization"
[s]
(if (instance? js/RegExp s)
s

  • (let [[_ flags pattern] (re-find #"^(?:(?([idmsux])))?(.)" s)]
    + (let [flags (or (second (re-find #"^(?([idmsux]*))" s)) "")
    + pattern (subs s (count flags))]
    (js/RegExp. pattern flags))))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Printing ;;;;;;;;;;;;;;;;

2.0.0.GIT

Comment by Alex Dowad [ 18/Dec/14 2:15 PM ]

Sorry, JIRA's helpful formatted mangled that patch. Here it is again.





[CLJS-806] support ^:const Created: 09/May/14  Updated: 17/Dec/14

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

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

Attachments: Text File cljs_806.patch    
Patch: Code and Test

 Description   

Currently def's do not support ^:const annotations, this is useful in conjunction with case.



 Comments   
Comment by Peter Schuck [ 17/Dec/14 4:53 PM ]

def's marked with ^:const are now treated as constants, they can't be redefined or set to a different value. Currently only case takes advantage of this. Additionally the emitted var is annotated as a constant for the closure compiler.





[CLJS-911] Cljs's clojure.string.replace replacement fn takes different args to Clj's clojure.string.replace Created: 17/Dec/14  Updated: 17/Dec/14

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

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2411



 Description   

Clojure's `clojure.string.replace` takes a replacement fn which receives args `[[group1 group2 ...]]`.
ClojureScript's `clojure.string.replace` takes a replacement fn which receives args `[group1 group2 ...]` (i.e. & args).

It's my understanding that something like the `clojure.string` ns is intended partly to help pave over superficial API differences like this.

Modding ClojureScript's `string.replace` to match Clojure's behaviour would be trivial, but this would be a breaking change for anyone that's come to rely on the faulty[?] behaviour.

Would you like a patch for this? Can submit for http://dev.clojure.org/jira/browse/CLJS-794 while I'm at it (both involve a change to `clojure.string/replace`).

Thanks!






[CLJS-651] optimize true branch of satisfies? usage Created: 01/Nov/13  Updated: 16/Dec/14

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

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

Attachments: Text File cljs_651.patch    
Patch: Code

 Description   

The true branch of a satisfies? test should be hinted so that the type doesn't need type hints



 Comments   
Comment by Peter Schuck [ 16/Dec/14 2:51 PM ]

All paths taken on satisfies are now hinted as boolean





[CLJS-908] Duplicate goog.require emit when using :refer Created: 14/Dec/14  Updated: 16/Dec/14  Resolved: 16/Dec/14

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

Type: Defect Priority: Trivial
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File duplicate-require-emit.diff    
Patch: Code

 Description   

In a CLJS (ns ...) a (:require [some-ns :refer (something)]) will end up with two goog.require("some_ns") statements in the generated .js. This also used to happen with (:require [clojure.string :as str]) but was fixed some time ago. The attached page should fix it for all cases.



 Comments   
Comment by David Nolen [ 16/Dec/14 12:22 PM ]

Thanks!

Fixed
https://github.com/clojure/clojurescript/commit/ec0023bd45a7f956b1e58aab84cb207a94e35965





[CLJS-696] remove arguments usage from defrecord constructor Created: 23/Nov/13  Updated: 16/Dec/14  Resolved: 16/Dec/14

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

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

Attachments: Text File cljs_696_v1.patch    
Patch: Code

 Description   

There is no need for the arguments usage in the defrecord constructor and it's a perf hit for construction. We should always construct defrecords by passing in the extra three arguments: __extmap, __meta, and hash automatically.



 Comments   
Comment by Peter Schuck [ 16/Dec/14 11:59 AM ]

The constructor now has __extmap, __meta, and __hash in all the places it's constructor is called, the positional factory, map factory, and direct constructor invocation. This is the first time going deep into the ClojureScript compiler so there may be some clean up to do or other places a records constructor is called that I didn't take care of.

Comment by David Nolen [ 16/Dec/14 12:20 PM ]

Excellent work! Thanks! https://github.com/clojure/clojurescript/commit/491dd1bb6ba446407298d6fb93dd6cbd578d3b76





[CLJS-909] Add stable api for consumers of compiler data. Created: 15/Dec/14  Updated: 16/Dec/14  Resolved: 16/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS_909.patch    

 Description   

The ClojureScript compiler is under very active development. It would be nice for consumers of internal compiler data to have a stable api.



 Comments   
Comment by Bruce Hauman [ 15/Dec/14 2:50 PM ]

Here's the patch

Comment by David Nolen [ 16/Dec/14 12:10 PM ]

fixed https://github.com/clojure/clojurescript/commit/163f079407d1310755d326884b6965d9d74ed3c5





[CLJS-905] Dependency tree fail Created: 11/Dec/14  Updated: 14/Dec/14  Resolved: 12/Dec/14

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

Type: Defect Priority: Major
Reporter: Dusan Maliarik Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, compiler


 Description   

In the following repo

https://github.com/skrat/deporder-bug-cljs

Looking at the compiled source, I would expect the `bug.b` module to be initialized before `bug.a`. It is not, and it leads to a runtime error about `bug.b/registry` being undefined. Now, when I switch things around (`around` branch in the repo), rename `bug.a` to `bug.b` and vice versa, I will get a compiler warning about `bug.a/registry` being undeclared, but at runtime everything works as expected.

This example was extracted from a large project that uses similar macro that expands to `swap!`, where with `none` optimizations everything works, but with `simple` and `advanced` it throws during runtime.



 Comments   
Comment by Thomas Heller [ 11/Dec/14 6:38 AM ]

Technically not a "bug" but a weakness due to the nature ClojureScript handles macros.

'bug.macros requires 'bug.b cause it emits code calling it. 'bug.a requires 'bug.macro and therefore technically requires 'bug.b but since it cannot figure this out you technically have to tell it by also doing a (:require [bug.b]) in 'bug.a.

I had this problem a while back [1] and implemented a workarround by letting the macro declare what namespaces it requires, which in your case would look like

(def macro
  ^{:js-requires '[bug.b]}
  zwap [v]
  `(swap! bug.b/registry conj ~v))

But I was turned down so the feature never made it.

The recommended way is to have macros in namespaces that have a matching CLJS namespace and using :include-macros/:refer-macros when requiring them. Has its own issues cause a) everyone needs to know that bug.b uses macros and b) everyone needs to know what actually is a macro.

Anyways, in your case move bug/macros.clj to bug/b.clj and rewrite bug/a.cljs to

(ns bug.a
  (:require [bug.b :refer-macros (zwap)]))

[1] https://groups.google.com/forum/?fromgroups#!searchin/clojurescript/macro$20require/clojurescript/sLRGCIa8E1c/N9sFcTP_i9wJ

Comment by Thomas Heller [ 11/Dec/14 6:49 AM ]

Shameless plug: shadow-build [1] supports the macro meta feature. Still sort of hack-ish since it only works when macros are referred directly but my use of macros is very limited, basically only whats in [2].

[1] https://github.com/thheller/shadow-build
[2] https://github.com/thheller/shadow/blob/master/src/clj/shadow/macros.clj

Comment by Dusan Maliarik [ 11/Dec/14 8:33 AM ]

I wonder how this works in Clojure land, but I would still say it's a bug in ClojureScript compiler, because, afaik, it first expands macros, which results in a symbol from another namespace ('bug.b), and at that point, 'bug.b should be added as a dependency, resulting in goog.require('bug.b') in the compiled 'bug.a, isn't that right?

Comment by Francis Avila [ 11/Dec/14 9:18 AM ]

In java Clojure you still need to ensure that the symbols you expand in your macros are resolvable by the time the expanded form is executed. For example, if you create a macro that includes a clojure.data/diff call, you need to require clojure.data somewhere in your program, ideally in the namespace that defines the macro. Symbols which refer to other namespaces do not automatically require those namespaces. Most likely you will get a {{ClassNotFoundException [namespace-part]}} when you try to execute the form.

However Clojurescript introduces the extra problem that the macro expansion and execution environments are different. (In Clojure they are the same.) So there are additional constraints not present in Clojure:

1. A separate, completely different namespace dependency tree exists at compile time vs runtime.
2. Because of the Google Closure compiler, the entire runtime dependency tree must be known statically at compile time. (Otherwise some code you need at runtime might not be compiled into the final js.)
3. Because of javascript's dynamism, some symbols may not be known even to the Google Closure compiler. For example, if you have a macro which expands to calls to jQuery code, neither clojure, clojurescript, nor google closure, nor even your browser have a mechanism to "require" jQuery. The code must simply trust that you have done whatever is necessary to ensure jQuery/whatever is resolvable when it finally executes.

Additionally, automatically requiring namespaces from macro expansion is impossible (not a bug in the ClojureScript compiler):
1. The namespaces might not exist (i.e. the symbol is being used as a symbol, rather than as a reference or var).
2. The namespace/symbol might not be resolveble right now. This is clearer in the clojurescript case: is that symbol a reference to a clojure namespace that I should require right now, or is it a reference to a clojurescript namespace which I need to expand code to require, but I can't actually expect it to exist until runtime?
3. The macro might create symbols dynamically, in which case there is no way to know what namespaces the macro requires without executing the macro.

Comment by Dusan Maliarik [ 11/Dec/14 9:36 AM ]

Yes that's clear to me. Let's factor the non-ClojureScript deps like jQuery out, that's of course impossible to get right for everyone. What I propose is:

  1. run the macros
  2. walk through the expanded code
  3. when a symbol from another namespace is found, if this namespace isn't already required in that package, do either
    • add the require for that namespace
    • print out a warning

Currently it doesn't do anything, it just breaks during the runtime.

Comment by Thomas Heller [ 11/Dec/14 9:46 AM ]

I get a Warning?

WARNING: Use of undeclared Var bug.b/registry at line 8 src/bug/a.cljs

Comes down to a tooling problem again (hint: use shadow-build) that the warning is only printed once with lein-cljsbuild.

Comment by Dusan Maliarik [ 12/Dec/14 4:58 AM ]

Still, why does the different naming produce different results?

Comment by David Nolen [ 12/Dec/14 6:22 AM ]

This is not a bug.

Comment by Dusan Maliarik [ 12/Dec/14 7:14 AM ]

The behavior we talked about earlier might not be a bug indeed, but the fact that merely changing namespace names breaks things, sure is a bug. David, I advise to checkout the repo, try for yourself.

Comment by David Nolen [ 12/Dec/14 10:23 AM ]

I looked at the repo there is no compiler bug that I can discern. You just have two different invalid ClojureScript programs. All the salient points have been covered by other commenters.

Comment by Dusan Maliarik [ 12/Dec/14 11:31 AM ]

I won't insist anymore, nuff was said. Still the point about naming choice affecting the compiler output, wasn't addressed (compare 'master' and 'around' branches). "invalid ClojureScript programs" made me laugh though thanks

Comment by Thomas Heller [ 14/Dec/14 5:32 AM ]

Naming has very little to do with it. If you declare no dependency when using the macro (either bug.b or bug.a depending on branch) the analyzer cannot establish its "correct" position in the dependency graph. Therefore it is basically "luck" whether it will end up in the correct position or not. Declare the correct position and it will always be in the correct position.

Pretty sure the around branch will behave exaclty like master when you switch the order of the :require, but again: DO NOT RELY on such undefined behavior. Declare the dependencies!

(ns bug.core
  (:require [bug.b :refer [woot]]
            [bug.a :refer [registry]]
            ))




[CLJS-807] Emitter cannot emit BigInt or BigDecimal Created: 13/May/14  Updated: 13/Dec/14  Resolved: 03/Dec/14

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: numerics
Environment:

r2202


Attachments: Text File cljs-807.patch    
Patch: Code and Test

 Description   

The reader understands BigInt and BigDecimal literals, but the emitter will throw an exception when it finds them.

I know CLJS does not have a proper number tower, but it should at least be able to accept literals like "1N" or "1.5M".

Attached is a patch which will cause the emitter to coerce BigInt and BigDecimal to double-approximations before emitting.



 Comments   
Comment by David Nolen [ 02/Dec/14 5:40 AM ]

Please rebase this patch to master. Thanks!

Comment by Francis Avila [ 03/Dec/14 12:06 AM ]

Rebased to master

Comment by David Nolen [ 03/Dec/14 7:35 AM ]

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





[CLJS-836] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 13/Dec/14

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

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

Attachments: Text File cljs_836_v1.patch    
Patch: Code and Test

 Description   

See http://dev.clojure.org/jira/browse/CLJ-1499



 Comments   
Comment by Peter Schuck [ 12/Dec/14 4:56 PM ]

This is a port of the patches / diffs at http://dev.clojure.org/jira/browse/CLJ-1499. I'm getting around a 2X perf improvement when running the benchmark quite.

Comment by David Nolen [ 13/Dec/14 8:43 AM ]

Thanks, looks great! Will check with Alex Miller about the status of CLJ-1499 and see if we can apply this one.





[CLJS-892] Improve performance of compare-symbols/compare-keywords Created: 29/Nov/14  Updated: 12/Dec/14  Resolved: 12/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs_892.patch    
Patch: Code

 Description   

It appears to me that compare-symbols function is in a good position. It knows a and b are not nil (nils are checked in compare), it knows a and b are both symbols (again, checked in compare) and it knows that .-ns and .-name fields are either nil or Strings.

This is how this function is implemented in core.cljs now:

(defn- compare-symbols [a b]
  (cond
   (= a b) 0
   (and (not (.-ns a)) (.-ns b)) -1
   (.-ns a) (if-not (.-ns b)
              1
              (let [nsc (compare (.-ns a) (.-ns b))]
                (if (zero? nsc)
                  (compare (.-name a) (.-name b))
                  nsc)))
   :default (compare (.-name a) (.-name b))))

Given prerequisites above, we can replace quite expensive (= a b) call (one nil check, identical? check, -equiv call, protocol dispatch, (instance? Symbol) check) with (identical? (.-str a) (.-str b)).

Also all calls to compare here happen in a controlled environment. Their arguments are not nil, and they are Strings. Call to compare is quite expensive too (identical? check, two nil checks, two type calls, comparison of types). All three calls to compare can be replaced with direct garray/defaultCompare call.

From my measurments, each change gives ×2 performance boost in comparison speed, total speedup ~4 times.

I also created compare-keywords function. They used to share implementation with compare-symbols before, but they differ in property .-fqn/.-str which is now used.

Patch attached: cljs_892.patch



 Comments   
Comment by David Nolen [ 02/Dec/14 4:52 AM ]

Looks useful. Mind adding a case to the benchmarks to the repo so I can verify behavior under the various engines on my machine? jsperf link would also be acceptable. Thanks!

Comment by Nikita Prokopov [ 11/Dec/14 4:22 PM ]

Updated version with benchmark code

Comment by Nikita Prokopov [ 11/Dec/14 4:24 PM ]

David, I uploaded new patch version with updated benchmark_runner.cljs, adding simple test for keywords comparison, will it do? Please check it out.

Comment by David Nolen [ 12/Dec/14 10:17 AM ]

Excellent performance enhancement, thanks! https://github.com/clojure/clojurescript/commit/f40a9cf49e8cd3ad016514df5d9aae9df995c801





[CLJS-903] Add volatile! (vswap! and vreset!) Created: 09/Dec/14  Updated: 12/Dec/14  Resolved: 12/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File ds--001--volatile.diff     File ds--002--volatile-transducers.diff     Text File transducer_atom_to_volatile.patch    
Patch: Code

 Description   

From JS perspective implementing volatile! in CLJS does not make much sense. It duplicates same functionality present in atom (and in fact CLJS statefull transducers use atom in place of volatile!).

However if you want to share statefull transducer function between CLJ and CLJS, you run into troubles and have to fall back to cljx workarounds.



 Comments   
Comment by David Nolen [ 09/Dec/14 12:53 PM ]

A patch is most welcome for this.

Comment by Peter Schuck [ 10/Dec/14 11:40 AM ]

The new volatile functions (volatile!, vswap!, and vreset!) are just aliases of their atom counterparts

Comment by Daniel Skarda [ 12/Dec/14 7:25 AM ]

Implementation of Volatile without alias to Atom.

Should be a little bit faster than Atoms (no watches, no validations, no meta handling etc).

Second patch updates transducer function to use volatile!

Comment by David Nolen [ 12/Dec/14 10:09 AM ]

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





[CLJS-907] False positives from arithmetic checks Created: 12/Dec/14  Updated: 12/Dec/14

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

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


 Description   
WARNING: cljs.core/>=, all arguments must be numbers, got [#{nil clj-nil} number] instead. at line 59 file:/Users/kimmoko/.m2/repository/prismatic/dommy/1.0.0/dommy-1.0.0.jar!/dommy/core.cljs

Line 59 from core.cljs is the body of when-let:

        (when-let [i (utils/class-index class-name c)]
          (>= i 0))))))
(defn class-index
  "Finds the index of class in a space-delimited class-name
    only will be used when Element::classList doesn't exist"
  [class-name class]
  (loop [start-from 0]
    (let [i (.indexOf class-name class start-from)]
      (when (>= i 0)
        (if (class-match? class-name class i)
          i
          (recur (+ i (.-length class))))))))


 Comments   
Comment by David Nolen [ 12/Dec/14 10:06 AM ]

The problem is that the inferred type of class-index doesn't include number.





[CLJS-906] Naming conventions: use clojure.pprint instead of cljs.pprint Created: 12/Dec/14  Updated: 12/Dec/14  Resolved: 12/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

I noticed a recent addition of cljs.pprint namespace to CLJS repo (which is great, thanks!).

I would like to point out minor naming issue. Could we avoid using cljs prefix when there is no significant difference between Clojure and ClojureScript version?

I am very sorry for my nitpicking. But these minor differences make unnecessary obstacles for maintaining code which targets both CLJ and CLJS.

Thank you, Dan



 Comments   
Comment by David Nolen [ 12/Dec/14 10:02 AM ]

Not possible due to the presence of an existing Clojure source file with the same namespace that provide macros.





[CLJS-887] cljs.repl.browser does not serve css by default Created: 17/Nov/14  Updated: 10/Dec/14

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

Type: Defect Priority: Trivial
Reporter: Christopher Shea Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl
Environment:

*


Attachments: File cljs-browser-repl-serve-css.diff    
Patch: Code

 Description   

A browser repl's server serves js, cljs, map, and html files by default, but not css.



 Comments   
Comment by John Chijioke [ 10/Dec/14 2:05 AM ]

What is css needed for from the repl?

Comment by Christopher Shea [ 10/Dec/14 9:13 AM ]

cljs.repl.browser/send-static can already send css with the appropriate MIME type, so this just seems like a minor oversight.

If you follow the Using the browser as an Evaluation Environment section of the ClojureScript wiki and have a css file linked from your html page, it will not be served, which can make it more difficult to work on your project (you won't see the effects of changing an element's class, e.g.).

As a workaround, I've been doing this when setting up my repl:

(server/dispatch-on :get
  (fn [{:keys [path]} _ _] (.endsWith path ".css"))
  browser/send-static)

It's not that my interactions with the repl require the css, it's the browser that's connected to the repl that does.





[CLJS-904] timeout in start-evaluator is too short Created: 10/Dec/14  Updated: 10/Dec/14

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

Type: Defect Priority: Major
Reporter: John Chijioke Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The timeout specified in cljs.browser.repl/start-evaluator is too short.
https://github.com/clojure/clojurescript/blob/master/src/cljs/clojure/browser/repl.cljs#L87

Usually I have to update it from 50 to 500 to get my repl working any time I update to a new version. Would be nice to have this factored in as I don't suspect it has any noticeable effect on any other functionality.






[CLJS-890] Incorrect behaviour of (str obj) when obj has valueOf method Created: 24/Nov/14  Updated: 08/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Nikita Beloglazov Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None


 Description   

Example

(str #js {"toString" (fn [] "hello") "valueOf" (fn [] 42)}) ; => "42"

The problem in the fact that ClojureScript uses concatenation to convert values to strings and that doesn't work well with objects which have valueOf() method overriden.

Example in js:

var obj = {
    toString: function() { return 'hello'; },
    valueOf: function() { return 42; }
};
console.log(String(obj)); => 'hello'
console.log(obj.toString()); => 'hello'
console.log('' + obj); => '42'

Potential solution might be to use String() function. Using toString() won't work as described in this issue: http://dev.clojure.org/jira/browse/CLJS-847



 Comments   
Comment by Kevin Neaton [ 24/Nov/14 10:34 AM ]

Is there a valid use case where toString and valueOf are not in sync? E.g.

(not= (.toString x) (js/String (.valueOf x))

If not, is it "incorrect" for the two methods to be out of sync?

Comment by Nikita Beloglazov [ 24/Nov/14 10:40 AM ]

Here is an example of such use case: https://github.com/processing-js/processing-js/blob/master/src/Objects/Char.js
That's how I found this bug.

Comment by Kevin Neaton [ 24/Nov/14 10:49 AM ]

Thanks for the link. I see what you mean.

Comment by David Nolen [ 02/Dec/14 5:08 AM ]

The problem with going with String appears to be a massive performance hit to printing http://jsperf.com/string-vs-tostring2/6.

Unless a brilliant idea is proposed this seems best solved / worked around in user code.

Comment by Nikita Beloglazov [ 02/Dec/14 6:41 AM ]

Append performs better on strings and numbers, but it performs worse on objects so it is not a clear performance hit. If I heavily work with objects and use (str) to convert them into strings then I actually lose on performance with current implementation.
Anyway current implementation of str is incorrect as it doesn't honor toString method. And this is what str function supposed to do. I believe a compiler should be correct first and then worry about performance.

Comment by David Nolen [ 02/Dec/14 7:38 AM ]

Sorry going back over this I believe the issue is really that we simply need to backout CLJS-801.

Comment by David Nolen [ 02/Dec/14 7:41 AM ]

reverted CLJS-801 in master

Comment by Francis Avila [ 02/Dec/14 10:32 AM ]

CLJS-801 only deals with the str macro. Aren't we still going to have str function problem because of CLJS-847? https://github.com/clojure/clojurescript/commit/08b4b1585cf0ef739e903985ee4c6b7fc6c47642 (also Yet if we use toString there, Safari 6.0.5 blows up. Maybe we need {{[o].join('')}}? Depending on where the bug is this may be wrong in Safari 6.0.5 too.

What we need to do very specifically is somehow get the return value of the (in ECMASCRIPT-ese) ToString abstract operation on the object (or the underlying ToPrimitive abstract operation with the String hint). String concat with the add operator

Options as I see it are:

  • x.toString() : Bad because of CLJS-847
  • {{[x].join('')}} : Should work (and does right thing for null/undefined), but I think we should test in Safari 6.0.5. Also very slow.
  • String
  • String.prototype.concat
  • String.prototype.slice(x,0) String.prototype.substring(x,0) String.prototype.substr(x, 0)
  • x.toString() normally, but String if we detect that we'll trigger CLJS-847. (Can specialize on startup.)
Comment by David Nolen [ 02/Dec/14 10:35 AM ]

Is there any evidence that higher usage of str is actually problematic?

Comment by Francis Avila [ 02/Dec/14 10:44 AM ]

String concat using the addition operator uses an un-hinted ToPrimitive abstract call (which will try x.valueOf() first then x.toString(), usually) and then {{ToString}}s the result of that, so it's not an option unless we are concating primitive values.

Details:

Comment by David Nolen [ 02/Dec/14 10:50 AM ]

I'm not really all that concerned about the specification, only if it matters in the wild. If this doesn't affect Safari 6.05 we don't care.

Comment by Francis Avila [ 02/Dec/14 11:01 AM ]

Is there any evidence that higher usage of str is actually problematic?

Kevin Neaton, who opened CLJS-847, was using a patch in production which only addressed the higher order case and he said the patch fixed the issue for them. He was unaffected by the str macro case because it either used ''+x already (with CLJS-801 applied) or it used {{[x].join('')}} (which hasn't been tested with Safari 6.0.5 yet, but probably works).

So if we had a problem using ''+x with the str macro, we will certainly have a problem with ''+x with a string function as long as CLJS-847 is applied.

I haven't pulled down master yet, but here is a test case which I bet will fail with the CLJS-847 patch:

(def tricky-obj #js {"toString" (fn [] "hello") "valueOf" (fn [] 42)})
(assert (= (apply str tricky-obj) "hello")) ;; will get "42"
Comment by Francis Avila [ 02/Dec/14 11:09 AM ]

I'm not really all that concerned about the specification, only if it matters in the wild. If this doesn't affect Safari 6.05 we don't care.

To be clear, there are two issues here:

CLJS-847: x.toString() fails on Safari 6.0.5. Workaround is ''+x (only done in str macro case).
CLJS-890: ''+x doesn't give expected results for objects which define valueOf. Expectation is that x.toString() is called, instead x.valueOf().toString(). Fix is to use array join instead of string concat in str macro, but it doesn't address the ''+x workaround from CLJS-847.

To make matters worse, it looks like the toString() error on Safari may only be triggered at certain JIT levels!

Comment by Francis Avila [ 02/Dec/14 11:10 AM ]

Workaround is ''+x (only done in str macro case).

I mean "Workaround is ''+x (only done in str function case)."

Comment by Nikita Beloglazov [ 08/Dec/14 6:14 PM ]

Can this bug be reopened meanwhile? If I understand correctly the fix should affect https://github.com/clojure/clojurescript/commit/08b4b1585cf0ef739e903985ee4c6b7fc6c47642 but this code still present in HEAD.

Comment by David Nolen [ 08/Dec/14 6:37 PM ]

We've switched to goog.string.buildString in master https://github.com/clojure/clojurescript/commit/94eb8a960fef6aaca4ba44b251cefbfa04d0f6ac

Comment by Nikita Beloglazov [ 08/Dec/14 8:32 PM ]

Yes, that works. Cool, thanks!





[CLJS-897] AOT core.cljs Created: 02/Dec/14  Updated: 08/Dec/14

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

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


 Comments   
Comment by David Nolen [ 08/Dec/14 4:22 PM ]

As pointed out by Colin Fleming it will be important to cache both optimized and unoptimized ClojureScript source - specifically the static-fns build option toggled.





[CLJS-901] In memory based builds Created: 03/Dec/14  Updated: 08/Dec/14

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

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


 Description   

Currently builds are based one files on disk. It is desirable to be able to pass arbitrary ClojureScript programs as string or seq of forms and get the fully compiled source including dependencies as a string result.



 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





[CLJS-885] read-string result raising a warning Created: 11/Nov/14  Updated: 05/Dec/14  Resolved: 05/Dec/14

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

Type: Defect Priority: Major
Reporter: Stefano Pugnetti Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: Compiler, bug
Environment:

Ubuntu 12.04 (64 bit)
Oracle JDK 1.7.0_72
leiningen 2.5.0
ClojureScript 0.0-2371
lein-cljsbuild 1.0.4-SNAPSHOT



 Description   

The result of read-string is not correctly recognized as a number in the example discussed here:

https://groups.google.com/forum/#!topic/clojurescript/kwJNH2MBbao

Wrapping it in a call to the identity function makes the symptom disappear.



 Comments   
Comment by David Nolen [ 05/Dec/14 1:37 PM ]

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





[CLJS-815] Range is overwritten rather than shadowed Created: 13/Jun/14  Updated: 05/Dec/14  Resolved: 05/Dec/14

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

Type: Defect Priority: Major
Reporter: Jamie Brandon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

cljs-0-0-2173
https://github.com/jamii/range-bug/blob/master/project.clj



 Description   

https://github.com/jamii/range-bug

https://github.com/jamii/range-bug/blob/master/src/range/core.cljs defines a (deftype Range ...)

https://github.com/jamii/range-bug/blob/master/out/range/core.js should contain

range.core.Range = ...

but instead contains

cljs.core.Range = ...

This breaks cljs.core/range.

The only warning given is 'WARNING: >Range already refers to: />Range being replaced by: range.core/->Range at line 3 src/range/core.cljs' which is correct even in the expected case of shadowing Range rather than overwriting.

Adding (:refer-clojure :exclude [Range]) produces the correct result.



 Comments   
Comment by David Nolen [ 05/Dec/14 1:20 PM ]

fixed in master





[CLJS-865] js-dependency-index won't work when running from an uberjar Created: 26/Sep/14  Updated: 03/Dec/14

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

Type: Defect Priority: Major
Reporter: Dan Johansson Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

Window7
[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2322"]
java 1.7



 Description   

I'm having problems running an uberjar with [lein-light-nrepl "0.0.18"].
I've tracked the problem down to the filter in cljs.js-deps/goog-resource.
It expects to find goog/deps.js in the google jar file but it is in the uberjar file.



 Comments   
Comment by Dan Johansson [ 29/Sep/14 2:27 AM ]

I use this workaround for now:

(defn load-nrepl []
  (require 'cljs.js-deps)
  
  (alter-var-root (find-var 'cljs.js-deps/goog-resource)
                  (fn [of]
                    (fn [path]
                      (first
                       (enumeration-seq (.getResources (.getContextClassLoader (Thread/currentThread)) path))))))
  
  (require 'lighttable.nrepl.handler))

It just removes the filter.
I guess the filter is there to make sure you get the correct goog/deps.js file? I don't know what assumptions to make about build environments to suggest a generic solution.

Comment by Dan Johansson [ 29/Sep/14 2:28 AM ]

This is the error I get:

Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
	at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:544)
	at clojure.java.io$fn__8628$G__8610__8635.invoke(io.clj:69)
	at clojure.java.io$reader.doInvoke(io.clj:102)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.js_deps$goog_dependencies_STAR_.invoke(js_deps.clj:241)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invoke(core.clj:624)
	at clojure.core$memoize$fn__5097.doInvoke(core.clj:5846)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at cljs.js_deps$js_dependency_index.invoke(js_deps.clj:265)
	at cljs.env$default_compiler_env.invoke(env.clj:45)
	at cljs.env$default_compiler_env.invoke(env.clj:42)
	at lighttable.nrepl.cljs__init.load(Unknown Source)
	at lighttable.nrepl.cljs__init.<clinit>(Unknown Source)
	... 52 more
Comment by James Cash [ 29/Sep/14 3:58 PM ]

I was able to work around this same issue by explicitly including the google-closure-library jar in the classpath (i.e. instead of `java -jar my-uber.jar`, `java -cp google-closure-library-$VERSION.jar:my-uber.jar my-uber.core`)

Comment by Taha Siddiqi [ 08/Oct/14 10:19 PM ]

This is what worked for me.

(defn goog-resource [original-fn path]
  (if-let [resource (io/resource path)]
    resource
    (original-fn path)))

(alter-var-root #'cljs.js-deps/goog-resource (fn [f] (partial goog-resource f)))
Comment by David Nolen [ 09/Oct/14 3:35 PM ]

I'd be happy to remove the hack for Google Closure if someone can demonstrate it's no longer needed. If I recall we now remove the empty files from the thirdy party JAR so this shouldn't be a problem anymore.

Comment by Joseph Moore [ 03/Dec/14 11:43 PM ]

Adding the google-closure library jar to my war file in immutant (from lein immutant war) fixed lighttable's nrepl under wildfly as well, thank you James





[CLJS-902] Give defined type constructors names for debugging's sake Created: 03/Dec/14  Updated: 03/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File 0001-CLJS-902-Give-ctors-names-for-debugging-purposes.patch    
Patch: Code

 Description   

I've wished more than once for the ability to use (.. x -constructor -name) during debugging to be able to tell what type an object has. The attached patch makes that work.






[CLJS-900] Parameterize caching strategy Created: 03/Dec/14  Updated: 03/Dec/14

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: 0
Labels: None


 Description   

Currently the caching strategy is hard coded to a disk based one. It would be desirable in many situations for the caching to be in memory. We should decouple the caching strategy and support disk / memory out of the box.






[CLJS-851] simplify :none script inclusion if :main supplied Created: 05/Sep/14  Updated: 03/Dec/14

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

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

Attachments: Text File 0001-Emit-require-for-main-namespace-do-deps-file.patch     Text File 0002-Emit-goog.base-configuration-constants.patch     Text File 0003-Emit-goog.base-into-deps-file.patch     Text File 0004-Emit-all-deps-into-a-single-file.patch     Text File 0005-Update-samples-to-new-none-opt-usage.patch    
Patch: Code

 Description   

If :main namespace supplied - under :none optimization settings :output-to file should goog.require it. This would also allow script inclusion to be unified regardless of the level of optimization supplied, i.e.:

<script type="text/javascript" src="foo.js"></script>

Instead of

<script type="text/javascript" src="out/goog/base.js"></script>
<script type="text/javascript" src="foo.js"></script>
<script type="text/javascript">goog.require("foo.core");</script>


 Comments   
Comment by David Nolen [ 05/Sep/14 5:10 PM ]

This does mean concatenating the contents of goog.base into the deps file.

Comment by Thomas Heller [ 06/Sep/14 8:31 AM ]

Just a quick Note: Not sure what your plan is for :main but it in my experience it is important to make it a collection. A larger app quickly gains multiple entry points.

Comment by David Nolen [ 06/Sep/14 9:38 AM ]

I don't see the point of making it a collection, users can simply load other entry points themselves via the "main" namespace.

Comment by Andrew Rosa [ 14/Oct/14 5:28 PM ]

Is someone already working on this issue? I will familiarize with the compiler code to try tackle this feature.

Comment by Andrew Rosa [ 15/Oct/14 10:17 PM ]

@dnolen I made the changes to compiler for auto require an specified :main namespace (but still optional). After that I started to experiment adding the goog.base code directly into the generated js, just to see what happens.

What I found is that we will need to disable the Closure default deps.js file. So we need to copy the goog.base code and after that include all dependencies, including the Closure ones. This work is need to avoid two issues:

  • We need to explicit set the base path for loading, since the auto-discovery method used by Closure searches for a <script str="base.js">.
  • By default, Closure automatically includes a <script> containing the goog/deps.js file to include itself. This causes race conditions during our require('main-ns'), since only half of our dependencies was added to dependency graph. Explicitly requiring everything will avoid this bug.

I could change the compiler to accommodate all this changes, but first like to communicate you the deep effects on our current behavior. What do you think?

Comment by Thomas Heller [ 16/Oct/14 2:24 AM ]

I have implemented this feature in shadow-build [1] and use the following strategy:

Assuming our output file is named "app.js" for an optimized build, it will look basically like this when compiling with :none.

1) imul.js fix
2) closure global settings

I found that these are required

var CLOSURE_NO_DEPS = true;
var CLOSURE_BASE_PATH = '/assets/js/src/';

The CLOSURE_BASE_PATH is required for the include scripts to find the javascripts for each namespace.

3) After the settings the goog/base.js is emitted into the file.
4) After that the contents of dependency settings are emitted (basically goog/deps.js) plus all of our namespaces. They look something like this

goog.addDependency("clojure/set.js",['clojure.set'],['cljs.core']);

When closure attempts to load a given namespace it will simply emit a <script src="CLOSURE_BASE_PATH + path/to/ns.js">, I found that using an absolute path is the best way to ensure that files are loaded correctly since the "path detection" feature of the goog/base.js file fails since we do not include a goog/base.js

5) Then finally you can just add the

goog.require('main-ns');

Hope that helps.

[1] https://github.com/thheller/shadow-build

Comment by Andrew Rosa [ 16/Oct/14 7:35 AM ]

Thank you Thomas for your very nice writeup.

In my tests I found that we could leave CLOSURE_BASE_PATH alone IF we made everything relative to our output file. Do you think forcing it to some path is more reliable?

Comment by Thomas Heller [ 16/Oct/14 1:08 PM ]

Yes definitly. The location of the javascript files usually is "fixed" once the app is configured. Either you put it locally at something like "/js" or something like a CDN, so you configure it once and forget about it. But thats my own personal opinion, I have no idea how other people are handling it. I had some problems with relative paths but can't remember exactly what they were only that CLOSURE_BASE_PATH fixed it.

Comment by Andrew Rosa [ 24/Oct/14 4:52 PM ]

Just to give an update here: I already have a patch for this issue, but depends on the resolution of CLJS-874.

Comment by Andrew Rosa [ 08/Nov/14 7:59 PM ]

Here are the patches to fix this issue. I made smaller patches for each "step" to light the burden of code review.

  • 0001: this issue creates a new compiler option :main, which receives a symbol with the "main namespace" similar to Clojure. If supplied, will emit a require at the end of our custom deps file.
  • 0002: here I put the Closure configuration constants to make the loading works. I set the goog/base.js dir as a relative path so the change will be less intrusive to cljs compiler.
  • 0003: Emit whole goog.base
  • 0004: combine all deps into our single file, to avoid async dependency loading between google closure libs and our deps.
  • 0005: Update all sample documentation and remove the old "*-dev.html" files.

Thomas: thank you for the help and the examples. In the end I removed the need to know absolute paths by just considering we can aways reach the goog/base directory. This leads to less code involved to make the feature available.

Please let me know if there is any problem with those patches, so I could fix them. This is my first contribution to Clojure(Script), so I still need to grasp the workflow here.

Comment by Martin Klepsch [ 27/Nov/14 10:55 AM ]

@Andrew, I just applied your patches and got the Twitterbuzz example running.
You've done great work, lot's of people will appreciate this change.

One issue I encountered doing that:

In the Twitterbuzz Readme there's a statement as follows:

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "samples/twitterbuzz/out"})
[...]
The reason we set the `:output-dir` is because the `index.html` script tag is specifically pointing to that directory.

When compiling with these options index.html tries to load deps from "[...]clojurescript/samples/twitterbuzz/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js" which is not correct.

After that I just compile with

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "out"})

This places an "out" dir into the clojurescript project directory. Now, when opening index.html, twitterbuzz.js looks for dependencies like this: "[...]clojurescript/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js". This correctly loads the earlier compiled js and the application works. I'm not sure how this is supposed to work as I've mostly just used lein-cljsbuild to date but you probably have an idea

Some minor things

  • The referenced patch CLJS-874 doesn't apply cleanly anymore. I'm too unfamiliar working with patches to help with this unfortunately.
  • Is there any reason to specify :main in advanced compilation mode? (It has been added to the advanced mode section in the twitterbuzz example.)

Really looking forward to see this merged!

Comment by Andrew Rosa [ 29/Nov/14 9:28 PM ]

Thanks @Martin for testing the patches, I kindly appreciate that.

I need to take some time to investigate the issue with this specific example. I'm not confident with the behavior of some examples, like the hello-js one which looks like broken (even on master). Probably I will provide a patch to review and possibly update the examples in another ticket, so we can have a more understandable environment to test this patches.

About the patch CLJS-874, it was made prior the last rework I've made here and probably will not be needed anymore. I need to confess that, if I knew at the time how to do multiple patches I neither need to created it in the first place.

At the last, the :main option isn't really used in advanced compilation. I just put it there to have some sort of "consistency" across all the examples - the only thing that change between them will be the optimization level. Do you think that this generates more confusion than good?

Comment by Mike Thompson [ 30/Nov/14 6:47 PM ]

I love this idea. Ironing out these sorts of stumbling blocks is so important to achieving wider cljs adoption. When I was starting with CLJS at the beginning of the year, I accumulated a healthy set of paper cuts from this sort of stuff. WAY too much time was wasted.

But ... two things:

1. I'd be concerned that the concatenation of base.js and deps.js might slow things down. The lovely thing about `:none` currently is that it delivers sub-second compile times. That's so important to a good productive workflow. But perhaps I'm being paranoid and adding in a base.js pre-pend won't add much time.

2. You really don't need to know what :main is. You can take a sledgehammer to the process by appending the following to the generated file:

```
for(var namespace in goog.dependencies_.nameToPath)
goog.require(namespace);
```

This requires everything in. Everything. (Which is the same outcome as using :whitespace or :simple) And that turns out to be essential in a unit-testing scenario where there isn't a single root namespace. Further explanation:

https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L110-L111

Aside: I worry that this might not be backwards compatible and that it should be achieved by introducing a new `:optimization` target of `:debug` (and deprecating :none ??). Just a thought.

Comment by Thomas Heller [ 01/Dec/14 7:51 AM ]

@Mike Don't worry, prepending base+deps.js is not noticeable performance wise (less than a millisecond).

I tried defining what a "main" is for quite some time in shadow-build but the best I came up with: It's a namespace that contains functions called from outside your CLJS Code (eg. via HTML, aka API functions). It is a very valueable thing to have since its the only way to tell the CLJS compiler (and runtime) which namespaces are actually required. If you have a "main" namespace its deps (and the deps of the deps, ...) can be identified and namespaces that aren't actually needed can be skipped. The Closure dead-code elimination does this for :advanced as well, we just need a little help for :none.

IMHO every app should have at least one "main" ns, requiring everything is a hack not a solution.

As for your clojurescript.test example, that is a shortcoming of the tools.

I'm happy to provide help with shadow-build if anyone is interested, should be pretty simple to add a :test target. To be honest adding more features/options to cljs.closure/build (which lein-cljsbuild uses) is a recipe for disaster as it is already way too complex.

Comment by Andrew Rosa [ 01/Dec/14 10:01 AM ]

I'm also against introducing new options. I really want is eventually deprecate some of the unneeded ones.

I think that the :main namespace is clean, and could avoid issues like having some code evaluated in a namespace that is removed by Closure compiler in production. Not sure how common that could be in ClojureScript, but a valid buggy scenario in terms of global mutable state.

Anyway, if we follow the alternative of requiring everything, I think that could be done without dabbling with goog.base internals. We already have all app namespaces to be able to emit the deps.js

@Thomas your experience with shadow-build has shown you any need of "multiple entry-points" or only a single :main ns is enough? I have a feeling that the need to include all test namespaces is somewhat necessary because how clojurescript.test works, but I'm not familiar how the Clojure version works.

Comment by Thomas Heller [ 01/Dec/14 12:50 PM ]

@Andrew: See my first comment on this issue, in my experience an app will have more than one entry point. "clojurescript.test" would be one example why.

Comment by Martin Klepsch [ 01/Dec/14 3:14 PM ]

@thomas could you further explain the behaviour you'd expect from passing multiple namespaces to :main?

With my current understanding I don't see a need for multiple arguments to the :main option as Andre proposed it.
If you want a build that has another entry point (which also loads tests or similar) why not just make it a different build?

Comment by Thomas Heller [ 01/Dec/14 4:10 PM ]

It's always rather difficult to explain something without a good real world example, but explaining what I do in my app is too app specific. Anyways, my web app has a couple of different features. Lots of different pages and some use one or more features provided by CLJS code. It is not a single-page Application, rather an "old" web app with just some dynamic components sprinkled in.

Let me try an example:

(ns my-app.feature1)
(defn ^:export fancy-component [])

(ns my-app.feature2)
(defn ^:export fancy-component [])

(ns my-app.feature3)
(defn ^:export fancy-component [])

These would all be "main" namespaces by my definition.

I do not want a (ns my-app (:require [my-app [feature1 feature2 feature3]])) since that would couple all components together. One feature provided by shadow-build is that it is able to "split" the produced javascript into multiple files (modules) so you may load them optionally. So I can even split each feature into a seperate .js file loadable on demand, AFTER advanced compilation that is.

I do this in my web app and instead of feeding everyone a hefty 800kb javascript file, everyone gets 90kb and loads the rest when needed. This greatly improved page load times.

The problem with multiple builds is that EVERY build will contain cljs.core, which is quite heavy. You'd end up with

build1: cljs.core, my-app.feature1
build2: cljs.core, my-app.feature2
build3: cljs.core, my-app.feature3

instead of

module1: cljs.core (+ code shared by feature1,2,3)
module2: my-app.feature1
module3: my-app.feature2
module4: my-app.feature3

If you have experience with jQuery, multiple builds would be the equivalent to including jQuery with every jQuery plugin. But I'm getting off topic, thats about modules not about :main.

Hope I made some sense in regards to multiple mains, its just something I use alot but for me it is coupled with modules so YMMV.

Comment by Mike Thompson [ 01/Dec/14 4:15 PM ]

First, I'm a bit surprised. I thought there would be joy at the thought of NOT having to nominate a :main.

At its core, CLJS-851 appears to be about harmonizing `:none` with other options. The initial comment above explains how the HTML files for :none have to be different from the other options :simple etc. We don't like that. BUT the proposal to add :main into project.clj seems set to reintroduce a difference again between :none and the other options, but just in a different place. And there seems to be no need for that difference. :simple and :whitespace bring in all the code. So does :optimized except it is first tree-shaken down by Google Closure. So these 3 options do not need a main. Why should :none have a :main when the other options don't. I can see no downside to pulling in everything and making :none harmonious with the other.

@Andrew I agree that we don't need to use the internals of goog to require everything in. The necessary list of namespaces to require are present at the time of writing deps, so a list of goog.requires could be added, one for each namespace compiled.

Second, @Thomas could you explain more why you think cemerick's framework is wrong to be rootless? In my experience when unittesting you actively do not want a root. You want a directory full of test clj files, with that set of files growing and shrinking over time. Different members of the team might be adding and removing them at each step. You want your tests to be "discovered" by your "test runner" and then executed. I don't want to have to be manually maintaining a central list of tests. In my experience, tools like "Nose" under Python come with a "runner" which discovers all the tests by walking the directory of your project, but that approach isn't an option in the CLJS world. Instead, the compiler has to scoop up all the tests from the directory, compile them and then they have to be "required in", ONLY at that point can the tests be "discovered" in a CLJS world. At least that's my impression.

Proposal: if `:main` isn't there, then bring it all it as per my suggestion. If :main is there then only it gets required.

Comment by Thomas Heller [ 01/Dec/14 4:49 PM ]

@Mike: Without getting too much into the specifics of shadow-build: The feature of discovering tests and running them for you would be trivial, even without making a single change in shadow-build itself.

"... that approach isn't an option in the CLJS world" is simply not true. It is true for cljs.closure/build since its a function combining multiple very complex tasks into one simple map of options. lein-cljsbuild thereby suffers the same fate.

I haven't done much work documenting or promoting shadow-build and to be honest I'd much rather see tools like lein-cljsbuild built on top of shadow-build rather than trying to turn it into a lein plugin itself. Quite frankly to me it is perfect the way it is. I can step into every stage of the CLJS compilation/optimization and do stuff other tools simply cannot do, but that comes at the price of not being very beginner friendly. Documentation might help but I'm really bad at writing that stuff.

Anyways, I would inclue :main for EVERY build, ESPECIALLY :advanced!

I have a rather large codebase (way over 10k LOC CLJS) with several distinct builds. If I had to compile every CLJS file for every build everything would build forever. Some smaller builds just require a couple files (say 10), why feed 300+ into the Closure Compiler to eliminate what a simple :main could have done (and even skipped the CLJS->JS compilation).

I have this stuff in production for about 2 years now, but with custom tools for about as long. I cannot speak for the lein-cljsbuild world whether :main would be a good idea there or not. I don't even care since shadow-build is not affected by this change at all, but given my experience with my own app/codebase I would recommend making :main accept a collection of namespaces.

Comment by Mike Thompson [ 01/Dec/14 5:29 PM ]

@Thomas I'm looking at this through the cljsbuild point of view. I appreciate that shadow build might be able to do test discovery, but unfortunately I don't have the option of using it. So I'm explaining this from the perspective of a cljsbuild user, who is seeking a better outcome.

Comment by Andrew Rosa [ 01/Dec/14 7:33 PM ]

Nice to see this conversation going forward @Mike and @Thomas.

Anyone here can confirm that the advanced compilation behaves like a concatenation of all user defined namespaces? If so, we could change our strategy to mimic the behavior on :none - consistency is a gold IMO, and I like the idea of transparency in optimization levels. Thanks @Mike to point it out.

@Thomas, I guess that the way you suggest to modularize the builds is not possible even for :advanced compilations. I prefer to have a separate issue to discuss and address that feature (which looks interesting for use cases like you presented). And sorry to make you answer the same question again.

The conditional :main is worth the cost of added complexity? How it will affect :advanced compilation?

Comment by Thomas Heller [ 02/Dec/14 4:39 AM ]

Advanced Optimization does not behave like a concatenation, Closure handles the optimization like this (greatly simplified):

1) You provide a list of inputs (Javascript files) and externs
2) Closure parses these into AST inputs
3) basic analysis to extract goog.provide/require information
4) build dependency graph, topological sort
5) magic (if :advanced)
6) generate output

shadow-build does something similar

1) scan all source paths for "resources" (.js, .cljs)
2) extract goog.provide/require from .js
3) read first form of .cljs files (expects it to be 'ns), get provide/require from that
4) build dependency graph, topological sort
5) compile CLJS->JS (if reachable by any :main)
6) feed into Closure Compiler
7) output

Basically cljs.clojure/build does the same, except for #3 which results in CLJS compiling in a somewhat random order (eg. it compiles one 'ns and all its deps until everything is compiled). Without a :main everything will be compiled even if not used anywhere in your project. Closure Advanced Compilation can still remove dead-code since it will analyze the AST and realize that parts (or everything) of a given namespace are never used, :none can not do that without a :main.

If you have 300 source files (CLJS+JS) the load time of a web page (in dev) will be rather high (we are talking seconds here, remember worrying about a couple ms), especially with open dev tools since that will also load an additional 300 source maps. Loading files you don't need therefore is a huge waste of time especially if you reload (or even phantomjs clojurescript.test) often. I cannot stress enough how important :main is for this stuff, but it largely depends on the size of your codebase and the structure of your app. Small example: the Admin "App" of my application will do 714 requests on page load (JS+source maps) and transfer 3.3mb, page load is 3,54s on my rather beefy MacPro (in production this cuts down 13 requests, 343kb and page load in 700ms). Without the :main every page in my app would do 714 requests (or even more) on page load. Not a great development experience. Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads? REPL might provide a different experience but I found it impossible to properly build/test complex input forms when using a REPL, much rather edit/reload.

@Andrew: Agreed that the module discussion has no place here, just wanted to point out that the :main option becomes mandatory then. To be honest this whole discussion only stresses the problem of cljs.closure/build: trying to combine everyone's expectation of how a build should look into a single function is tough. So in the long run something that lets you split these parts will be easier (hint: shadow-build) but the good part is that ClojureScript itself does not even need to deal with all this stuff. A "simple" CLJS->JS compiler would do, the Closure bits are optional after all (even more so for non-browser targeted builds). But discussing what should be in CLJS and what shouldn't is another issue as well.

Comment by Martin Klepsch [ 02/Dec/14 1:21 PM ]

Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads?

No. Figwheel only reloads changed files. So the slowdown should be a lot smaller once the page is loaded.

Besides that: @thomas thanks for all your very thoughtful messages here, just reading them already taught me a ton about Clojurescript compilation!

Comment by Andrew Rosa [ 02/Dec/14 6:22 PM ]

Thank you @Thomas for all this explanation, quite insightful. You convinced me that the way to go is requiring :main anyway. If we fell the necessity, we could add the "require all" behavior later. I also like the idea of having a dependency graph to compile only required stuff, but is something for another discussion

Just a final tweak for us to think about: the :main option could receive a list of namespaces or just a single one? Conceptually, I biased for the later. A single namespace could allow us to eventually add something like a -main function, useful for node apps for example - just an idea, of course.

I just updated the patches to fix a single issue with :main not being mungled before require, which caused problems with namespaces including dashes (like foo-bar)

Comment by Mike Thompson [ 02/Dec/14 6:35 PM ]

Okay, my final observations:

  • naming
    Given :main is to be used to indicate a namespace, should its name be changed to make that clearer?
    After all this is about automating: 'goog.require(something)' but the name :main seems unrelated.
    To me :main carries connotations of main functions, not root namespaces.
    I would suggest that ":auto-require" would be a better name and I believe names (cumulatively) matter.
  • Special Marker
    No matter the name chosen, to handle my unittest use-case, would it be possible to have a
    special marker value for :main like, say, "*" which triggers the require everything approach.
    As I've indicated, that would be a big win from where I'm standing.

Thanks to everyone.

Comment by Andrew Rosa [ 02/Dec/14 6:41 PM ]

Good call @Mike. I guess :main was chosen to provide some "parity" with Clojure - that's why I suggest the auto-calling -main feature.

For the unit test case, do you think that it could be done as one enhancement on clojurescript.test? It seems like the test runner should be capable of finding the right namespaces... not sure if it does not do that for limitations of the platform.

Comment by Thomas Heller [ 02/Dec/14 7:16 PM ]

Closure calls them Entry Points, I got used to :main.

Can't really say much about clojurescript.test, only that testing generally is rather complex hiding this behind a "require all" marker will probably not be enough. I know I don't always want to run every single test, usually only those affected by the file I just changed is enough. But getting off-topic again, proper Tooling is hard.

Again for :main, my advice would be a collection but just offer a fallback and turn a single symbol into a collection of one. That covers all cases 0,1,more ... No :main just does not emit any requires which is what we have now.

:main my-app.feature1
:main [my-app.feature1]
:main [my-app.feature1 my-app.feature2]
Comment by Andrew Rosa [ 02/Dec/14 7:27 PM ]

@Thomas no defined :main will still will include base.js and deps.js, but not requiring anything. Right? At least is that what I would expect.

Yeah, tooling is very hard.

Comment by Thomas Heller [ 03/Dec/14 4:20 AM ]

No :main might as well behave like it does now, this way the feature is completely optional and will not break any existing code. Thats usually worth something, don't know how much because as I said, :main is mandatory in shadow-build.

ClojureScript itself should probably be more conservative than that. I imagine there are alot of projects that still use /out/goog/base.js + other in their HTML, that won't change over night.





[CLJS-899] AOT cache core.cljs analysis Created: 02/Dec/14  Updated: 02/Dec/14

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

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





[CLJS-898] Cache analysis results to disk Created: 02/Dec/14  Updated: 02/Dec/14

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

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





[CLJS-896] Investigate AOTed ClojureScript JAR Created: 02/Dec/14  Updated: 02/Dec/14

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

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





[CLJS-895] Hello-js sample is broken when following README instructions Created: 02/Dec/14  Updated: 02/Dec/14

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

Type: Enhancement Priority: Trivial
Reporter: Andrew Rosa Assignee: Andrew Rosa
Resolution: Unresolved Votes: 0
Labels: documentation

Attachments: Text File 0001-Create-sample-to-demonstrate-only-extern-usage.patch    

 Description   

Currently we have on hello-js the usage of externes libraries. But it not only mix different aspects of cljsc as also have some breakage if we follow the README instructions

So, instead of adding more complexity into it, here I suggest us to extract the extern functionality into it's own sample project, which is much more approachable for newcomers that want to grok cljsc usage. This new sample project is included in my first patch, so anyone could see the end result.

If this patch is desirable, I could move around the other samples and split them into separate, focused aspect (simple hello, calling cljs from js, fixing the dom sample). Only the twitter buzz that I'm not sure how to approach it - I guess it has issues with API limitations of the last Twitter updates.






[CLJS-894] Hot reloading compiled CLJS code into the browser breaks source maps. Created: 02/Dec/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Chrome, FF


Attachments: Text File fix_source_map_reloading.patch    
Patch: Code

 Description   

When one dynamically reloads code into a running application in the browser (Chrome). The browser doesn't reload the source map.

This can be fixed by appending a unique id to the source map epilogue which can be found at the bottom of a compiled file.

This forces the browser to reload the source map, however ... it doesn't fetch the new cljs source identified in the source map.

So a fix for that is to add yet another unique id to the paths in the source map.

The result is that hot loaded code has correct source and mapping in the browser (Chrome, FF).

This is what started the issue:
https://github.com/bhauman/lein-figwheel/issues/51

Here is a twitter conversation about it.
https://twitter.com/jlongster/status/539868749739094016

This is a fairly non invasive solution. It really has very little impact on devs who are not hot reloading.
Well actually it prevents the browser from accidentally caching source.

While I would prefer the browser vendors not cache in this situation, this patch significantly improves the coding experience.



 Comments   
Comment by David Nolen [ 02/Dec/14 3:44 PM ]

fixed https://github.com/clojure/clojurescript/commit/254e54876dfeae8c50d885010e730cdeaef26c99

Comment by Glen Mailer [ 02/Dec/14 3:53 PM ]

I'm not very familiar with these code paths, will this lead to non-deterministic builds?





[CLJS-893] Nodejs target shouldn't insert shebang by default Created: 02/Dec/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Dusan Maliarik Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: bug, compiler


 Description   

If people want, they will prepend the shebang themselves, or run the file using node, where-ever it is on PATH. This is not good default behavior, and breaks things when the intention is to compile a node.js module.



 Comments   
Comment by David Nolen [ 02/Dec/14 12:52 PM ]

Whether it was a good or bad default is water under the bridge at this point. Do you have any suggested solutions? :node-module comes to mind for a new target that doesn't append the shebang. Another option would be something to explicitly suppress it.

Comment by Dusan Maliarik [ 02/Dec/14 1:20 PM ]

Sorry, my fault
https://github.com/clojure/clojurescript/commit/00a9b7bde0c8823175560b453a00e7be09ddd250

I just have to :hashbang false. Unfortunately I was unable to find a complete list of supported options to the compiler. Is it documented somewhere?

Comment by David Nolen [ 02/Dec/14 1:29 PM ]

It is not but the wiki is publicly editable - nothing is stopping anyone from creating a page that documents all the knobs and their relationships.





[CLJS-773] Use unchecked-*-int functions for real 32-bit math Created: 26/Feb/14  Updated: 02/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: numerics
Environment:

r2173



 Description   

Currently the unchecked-* functions and macros simply alias the primitive js operators. It would be nice if the unchecked-*-int family of functions and macros implemented C/Java-like signed int operations with silent overflows (just like in Clojure) using asm.js coersion idioms. This should also allow us to share such code between clojure and clojurescript without worrying about their different numerics.

A use case is that porting hash algorithms from java to clojurescript is trickier and more verbose than it needs to be.



 Comments   
Comment by David Nolen [ 08/May/14 6:43 PM ]

This sounds interesting, would like to see more thoughts on approach, benchmarks etc.

Comment by David Nolen [ 02/Dec/14 5:46 AM ]

Bump, this enhancements sound simple & fine.

Comment by Francis Avila [ 02/Dec/14 1:26 PM ]

I'll have time to do this in about a week. The implementation is straightforward (basically use xor 0 everywhere). The goal is correctness, but I expect performance to be as good as or better than it is now on most platforms. I'm not sure if advanced mode will drop intermediate truncations or what impact this has on performance.

Some higher-level numeric analysis using the asm.js type system is possible but I doubt it's worth it.





[CLJS-776] re-matches is incorrect Created: 28/Feb/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The re-matches function does not have the correct semantics: it performs a search (not match) against the string and returns nil if the string and matched-string are unequal. This is not the same as true matching, which is like inserting "^" and "$" at the beginning and end of the pattern.

Example in Clojure:

user=> (re-find #"0|[1-9][0-9]+|0[xX][0-9a-zA-Z]+" "0x1")
"0"
user=> (re-matches #"0|[1-9][0-9]+|0[xX][0-9a-zA-Z]+" "0x1")
"0x1"

Compare Clojurescript:

ClojureScript:cljs.user> (re-find  #"0|[1-9][0-9]+|0[xX][0-9a-zA-Z]+" "0x1")
"0"
ClojureScript:cljs.user> (re-matches #"0|[1-9][0-9]+|0[xX][0-9a-zA-Z]+" "0x1")
nil

This bug is (one of the) reasons why CLJS-775.

I'm not completely sure what to do here. My first thought is to have re-matches inspect the -source property of its regex input, wrap the string with "^$", then carefully copy all flags over to a new regexp.

Questions:

  1. Are there any valid patterns where this is not safe? E.g., where we could not put ^ first? Is "^^abc$$" ok?
  2. Can we avoid cloning if ^ and $ are already the first and last chars of the pattern?
  3. How does multiline mode play in to this, if at all?
  4. regexinstance.lastIndex is a piece of mutability on regex instances (or the RegExp global on older browsers) which is used as a string offset for multiple invocations of exec() on the same string. I have no idea what to do if re-* gets a regex with the global flag set. (BTW, this is a very good reason to reject CLJS-150: allowing clojure to accept the global flag makes regular expression objects stateful, and would completely screw up re-seq for example.)


 Comments   
Comment by Francis Avila [ 24/Jun/14 7:37 AM ]

I would like to propose a somewhat radical suggestion that would: fix this issue and CLJS-810, put us in a better position to resolve CLJS-485 CLJS-746 CLJS-794 (clojure.string/replace woes), allow us to add some regex-as-a-value niceties to patterns in js (CLJS-67 and CLJS-68), and bring clojurescript's regular expression handling closer to clojure's by implementing more of the re-* functions.

Example implementation (not a patch) at this cljsfiddle: http://cljsfiddle.net/fiddle/favila.regexp

Essential points:

  1. Create a Pattern object, created by re-pattern, which provides methods to create regexps for search (re-find) or exact match (re-matches) or repeated searches (re-seq, re-matcher + re-find). Each of these must be a different RegExp object in javascript even though they are similar regular expression strings. The re-find and re-matches patterns can be cached. All can generate RegExps lazily.
  2. regular expression literals emit these Pattern objects instead of RegExp objects.
  3. Create a Matcher object to correspond to the currently-unimplemented re-matcher. It combines a global-flagged RegExp object, a search string, and a done flag. If it keeps the last match (similar to java), cljs can also implement re-groups.
  4. Make re-seq use the Matcher object and thus the .lastIndex that native RegExps provide for global matches. (Its implementation no longer requires string slicing after every match.)
  5. If re-find is given a native RegExp object instead of a pattern, it will use it as-is. This matches current behavior.
  6. If re-matches is given a native RegExp object and it isn't suitable for exact-matching, a new RegExp is cloned from the input RegExp with ^ and $ prepended and appended and the global flag added. (This technique is used in clojure.string/replace, but incorrectly.)

Thoughts?

Comment by David Nolen [ 02/Dec/14 5:46 AM ]

This sounds interesting but I'm somewhat concerned about the interop story. I think people will expect functions to take regular RegExps as well as Pattern. You haven't mentioned this issue here?

Comment by Francis Avila [ 02/Dec/14 1:16 PM ]

I apologize if some of my thoughts are vague; I haven't thought about this in a while.

First note that a narrow class of RegExps are effectively "pure": If they do a full-string match (e.g. start with ^ and end with $) and have the global flag set to false then their lastIndex will always seem to be 0.

Interop possibilities:

  • Patterns and RegExps can be created from one another, so coercion is always an option. E.g. re-pattern can accept a RegExp and some other (cljs-specific) function can coerce from Pattern or Matcher to RegExp. (Or maybe re-matcher can return a RegExp-compatible object--see below.)
  • RegExp given to cljs re-*: "Pure" regexes can be used directly, otherwise we create a Pattern and/or Matcher. (I don't remember the details, but the fiddle should cover them.)
  • Pattern used as a RegExp: Patterns can expose all the properties of RegExp instances. If the pattern is pure, it can implement .test and .exec. .lastIndex will always be 0. Not sure what to do about impure patterns: throw exception, act pure anyway, return a new object?
  • Matcher used as a RegExp: A Matcher can exactly replicate a RegExp instance, maybe even use the same prototype. Using it like a RegExp will mutate the object and disturb its internal state, but as long as it's either used consistently as a RegExp or consistently as a Matcher this won't matter. Notes:
    • Matcher holds the matched string in Java. Javascript trusts you to always supply the same string (e.g. in a while loop).
    • Java's Matcher holds the last match (used by re-groups). Javascript's RegExp does not.
    • Javascript's RegExp will automatically reset when lastIndex reaches the end of the source string. Java's Matcher won't.
    • Matcher must be a wrapper and not a normal RegExp because of these three extra bits of state.
    • The return value of re-matcher is only consumed by the 1-arg form of re-find and re-groups.
    • re-seq can use a matcher internally, but since it is private it doesn't have to.
    • Should other Java methods of Matcher be implemented?
  • Pattern given to String.prototype.match, .replace, .search, and .split: I haven't thought about this. Considerations:
    • Problem code is any cljs code using an object created via pattern literals or re-pattern and using it as an argument to these String methods. If they use clojure.string methods instead they would be fine.
    • Such code is also impossible in java clojure: only (.split s "pattern-str") is the same in java/clj and js/cljs and it will continue to work (without flags) on both platforms. Possibly we could just make people fix such code since it is platform-specific, but I need to see how widespread this is.
    • The fix for such code is to either:
      • Use a pattern->regexp coercion function we will provide.
      • Construct those regexps directly with js/RegExp.
      • Use clojure.string functions instead of String methods. This also has the advantage of being portable between clj and cljs.
    • Possibly we can patch the RegExp constructor or mess with the String prototype chain to do pattern->regexp coercion automatically, but this is a violent solution.

Correct me if I'm wrong, but in Clojure (java) code it is extremely uncommon to use Pattern and Match methods or to use them with String methods directly. For the most part they are treated as opaque objects used via re-* and clojure.string/*. Code written in the same style in cljs would be unaffected, and we can declare any other use as platform-specific and require explicit creation of RegExps (and don't bother to make Matcher or Pattern act like RegExps). This is my preferred approach for interop if there isn't too much use of RegExp.prototype.test, .exec, and String.prototype.match, .replace, .search, and .split.





[CLJS-749] Changing ClojureScript versions may break browser REPL Created: 14/Jan/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Osbert Feng Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File cljs-749.patch    
Patch: Code

 Description   

By default in a project using ClojureScript, the .repl/ directory is used to store compiled CLJS for the browser REPL. This compilation is skipped if the directory already exists (src/clj/cljs/repl/browser.clj:205:create-client-js-file). However, it really should be re-compiled if the version of ClojureScript is upgraded/changed or else the browser REPL may fail with some very difficult to interpret error messages. At the moment, it seems people simply know to delete .repl/ when changing ClojureScript versions (https://groups.google.com/forum/#!topic/clojure/C-H4gSWIUwc) but this can be extremely tough on new users when they upgrade ClojureScript for the first time.

We could append clojurescript-version to the directory name. Unfortunate that it generates a new directory each time a new version of CLJS/BrowserREPL combo is used, but shoudl not occur too often and makes it very explicit that :working-dir should be tied to CLJS version. Also developers utilizing ClojureScript though lein checkouts will still have to delete .repl/ since clojurescript-verion is only set by script/build.

See attached patch.

NOTE: I do not have a CA agreement on file, but one is in the mail.

NOTE: Sorry if this is bad form, but as a preceding commit, in cases where clojurescript-version is unbound I changed (clojurescript-version) to return "" instead of ". This is so that when clojurescript-version is unbound .repl/ will be used instead of .repl-./ Let me know if this should be considered as a separate issue.

Alternatively, we could remove the exists check entirely, and instead recompile client.js every time (repl-env) is called at the cost of slowing down browser REPL startup.



 Comments   
Comment by David Nolen [ 02/Dec/14 5:48 AM ]

Seems like a fine fix but this patch needs to be rebased to master. Thanks.

Comment by Osbert Feng [ 02/Dec/14 12:42 PM ]

Please see the updated patch. Thanks!

Comment by David Nolen [ 02/Dec/14 12:48 PM ]

Fixed https://github.com/clojure/clojurescript/commit/50cc86ff3c4c39181a198a4f9be788c149eaae00
https://github.com/clojure/clojurescript/commit/85773301cf12541a053890643d1d943a6ed361de
https://github.com/clojure/clojurescript/commit/d25aea69697cca2ef5fa8b9a6f7e4fd089685ace

Comment by David Nolen [ 02/Dec/14 12:49 PM ]

In the future squashed patches are preferred. Thanks for your contribution!





[CLJS-869] When preamble is not found in source directory, compiler does not report it Created: 03/Oct/14  Updated: 02/Dec/14  Resolved: 08/Oct/14

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

Type: Defect Priority: Minor
Reporter: Timothy Pratley Assignee: Timothy Pratley
Resolution: Completed Votes: 0
Labels: cljsbuild, compiler, patch

Attachments: Text File CLJS-869.patch    

 Description   

file target is not validated
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/closure.clj#L525-L531
externs might need checking also.

Steps:
in project.clj add :preamble ["nosuchfile"] to the :compiler map

Expected:
"Could not find preamble file X in source path"

Actual:
Stacktrace gives no indication of the problem (without reading the compiler code)::

Compiling "resources/public/js/device_manager.js" failed.
java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:544)
at clojure.java.io$fn_8628$G8610_8635.invoke(io.clj:69)
at clojure.java.io$reader.doInvoke(io.clj:102)
at clojure.lang.RestFn.invoke(RestFn.java:410)
at clojure.lang.AFn.applyToHelper(AFn.java:154)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:626)
at clojure.core$slurp.doInvoke(core.clj:6390)
at clojure.lang.RestFn.invoke(RestFn.java:410)
at cljs.closure$preamble_from_paths$fn__3018.invoke(closure.clj:524)
at clojure.core$map$fn__4245.invoke(core.clj:2557)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:484)
at clojure.core$seq.invoke(core.clj:133)
at clojure.core$apply.invoke(core.clj:624)
at cljs.closure$preamble_from_paths.invoke(closure.clj:524)
at cljs.closure$make_preamble.invoke(closure.clj:529)
at cljs.closure$optimize.doInvoke(closure.clj:592)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:626)
at cljs.closure$build.invoke(closure.clj:980)
at cljs.closure$build.invoke(closure.clj:923)
at cljsbuild.compiler$compile_cljs$fn__3200.invoke(compiler.clj:58)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:57)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:159)
at user$eval3326$iter_33443348$fn3349$fn_3361.invoke(form-init6680721970243583147.clj:1)
at user$eval3326$iter_33443348$fn_3349.invoke(form-init6680721970243583147.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:484)
at clojure.core$seq.invoke(core.clj:133)
at clojure.core$dorun.invoke(core.clj:2855)
at clojure.core$doall.invoke(core.clj:2871)
at user$eval3326.invoke(form-init6680721970243583147.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6703)
at clojure.lang.Compiler.eval(Compiler.java:6693)
at clojure.lang.Compiler.load(Compiler.java:7130)
at clojure.lang.Compiler.loadFile(Compiler.java:7086)
at clojure.main$load_script.invoke(main.clj:274)
at clojure.main$init_opt.invoke(main.clj:279)
at clojure.main$initialize.invoke(main.clj:307)
at clojure.main$null_opt.invoke(main.clj:342)
at clojure.main$main.doInvoke(main.clj:420)
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)



 Comments   
Comment by Timothy Pratley [ 05/Oct/14 1:02 PM ]

Reports a warning

Preamble resource file not found: <file1> <file2> <file3>

Comment by David Nolen [ 08/Oct/14 4:59 PM ]

fixed https://github.com/clojure/clojurescript/commit/9fd6bf5bd55421c3d5becacc5230ed661d6fb3c3





[CLJS-735] Set literal and hash-set cleanup Created: 28/Dec/13  Updated: 02/Dec/14  Resolved: 30/Dec/13

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

Type: Defect Priority: Minor
Reporter: Logan Campbell Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Clojurescript 0.0-2127


Attachments: Text File cljs-735-dead-code-cleanup.patch    

 Description   

Some sets drop values, causing loss of data.

The smallest example I've been able to find is:

#{[1 4] [2 4] [3 4] [0 3] [1 3] [2 3] [3 3] [3 2] [4 2]}
Drops: [1 4] [3 4] [1 3] [3 3]

hash-set also drops values. Though they are different.

(hash-set [1 4] [2 4] [3 4] [0 3] [1 3] [2 3] [3 3] [3 2] [4 2])
Drops: [2 4] [0 3] [2 3] [3 2]

Re-ordering the values appears to make no difference.

I have found no instance where sorted-set drops values.



 Comments   
Comment by Francis Avila [ 29/Dec/13 6:04 PM ]

This is a bug in PersistentHashSet.fromArray that causes every other item to be skipped when the input is longer than the PersistentArrayMap.HASHMAP_THRESHOLD (which is 8). Demonstration:

ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7] [8])
#{[2] [4] [6] [8] [0]}
ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7])   
#{[1] [2] [3] [4] [5] [6] [7] [0]}

Working on a patch and test.

Comment by Francis Avila [ 29/Dec/13 6:11 PM ]

Nevermind, looks like David fixed this already.

Comment by Francis Avila [ 29/Dec/13 8:02 PM ]

Some dead code was left in PersistentHashSet.fromArray (no-clone arg and arr let-binding). Attached a patch that cleans this up.

Comment by David Nolen [ 30/Dec/13 8:51 AM ]

Thanks for the patch but remember we need CAs to apply them.

The current hash-set construction implementation misses an optimization opportunity. We could build hash-sets much faster if we know that we have only unique constants at compile time.

Comment by David Nolen [ 30/Dec/13 8:52 AM ]

Need to implement the hash-set construction optimization, the old removed approach is faster when we know we have a set literal containing only constants.





[CLJS-716] Lookup for Date keys does not work in PersistentMaps and PersistentSets Created: 06/Dec/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Sunil Gunisetty Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug
Environment:

Clojurescript version "0.0-2080"
Browser : Firefox 25.0.1
: Chrome 31.0.1650.63



 Description   

Lookup of js/Date objects fails, if there are more than 8 elements in the map. Works correctly if the map contains 8 elements or less.

Examples :

1. Map with more than 8 elements

cljs.user> (def test-map
{
:a 1
:b 2
#inst "2013-12-19T05:00:00.000-00:00" 3
:d 4
:e 5
#inst "2013-12-06T05:00:00.000-00:00" 6
:g 7
:h 8
:i 9
:j 10
})

cljs.user> (test-map #inst "2013-12-19T05:00:00.000-00:00")
nil

2. Map with 8 elements

cljs.user> (def test-map
{
:a 1
:b 2
#inst "2013-12-19T05:00:00.000-00:00" 3
:d 4
:e 5
#inst "2013-12-06T05:00:00.000-00:00" 6
:g 7
:h 8
})

cljs.user> (test-map #inst "2013-12-19T05:00:00.000-00:00")
3



 Comments   
Comment by David Nolen [ 06/Dec/13 5:07 PM ]

This is because JS Dates don't hash consistently, http://dev.clojure.org/jira/browse/CLJS-523

Comment by David Nolen [ 02/Dec/14 5:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/66e9d688cc864a6f8a8ce45aad0ea173915334b2





[CLJS-410] support ^:expose annotation Created: 28/Oct/12  Updated: 02/Dec/14  Resolved: 19/Nov/13

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

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

Attachments: Text File CLJS-410.patch    

 Description   
(def ^:export m 1)
(defn foo [g]
  (g m))

Because of constant propagation GClosure may replace m in foo with 1. It may be better to for :export to implicitly :expose but we need to check the output.

http://developers.google.com/closure/compiler/docs/js-for-compiler



 Comments   
Comment by Olle Martensson [ 22/Jul/13 5:33 AM ]

def annotated with ^:export is now implicitly treated as an exposed property by the closure compiler.

The goog.exportSymbol part for def seemed to be redundant and thus removed.

Comment by David Nolen [ 19/Nov/13 9:20 PM ]

There are workaround for this.





[CLJS-878] Missing preamble file causes unhelpful stack trace Created: 30/Oct/14  Updated: 02/Dec/14  Resolved: 31/Oct/14

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

Type: Defect Priority: Trivial
Reporter: Darrick Wiebe Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: cljs, errormsgs


 Description   

I think it would be helpful to throw an exception with the missing file path rather than the current

java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
.

Right now you need to work your way down about 10 lines into the stacktrace to find

cljs.closure$preamble_from_paths$fn__3097.invoke(closure.clj:526)
which is the only hint as to what has happened.



 Comments   
Comment by David Nolen [ 31/Oct/14 2:42 AM ]

This is a simple enhancement, a patch is welcome. Thanks.

Comment by David Nolen [ 31/Oct/14 2:44 AM ]

Actually this already appears fixed in master https://github.com/clojure/clojurescript/commit/9fd6bf5bd55421c3d5becacc5230ed661d6fb3c3

Comment by David Nolen [ 31/Oct/14 2:45 AM ]

See CLJS-869





[CLJS-687] error/warning when deftype/record constructor used as a function Created: 19/Nov/13  Updated: 02/Dec/14  Resolved: 19/Nov/13

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

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


 Comments   
Comment by David Nolen [ 19/Nov/13 5:11 PM ]

fixed,https://github.com/clojure/clojurescript/commit/792b2b87c9982a76093fcc3278dd246e8672627b





[CLJS-856] On Node.js, (seq (array 1 2 3)) throws exception Created: 12/Sep/14  Updated: 02/Dec/14  Resolved: 15/Sep/14

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

Type: Defect Priority: Major
Reporter: Peter Schwarz Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: nodejs
Environment:

Os X
Node.js 0.10.31
Clojurescript 0.0-2311



 Description   

Calling (seq (array 1 2 3)) throws the following exception:

Error: 1,2, 3 is not ISeqable
    at seq (cljs/core.cljs:38:9)
    at <cljs repl>:1:79
    at <cljs repl>:5:3
    at Socket.eval (eval at <anonymous> ([eval]:1:20), <anonymous>:70:29)
    at Socket.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:764:14)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:426:10)
    at emitReadable (_stream_readable.js:422:5)
    at readableAddChunk (_stream_readable.js:165:9)

However, calling (seq (aclone (array 1 2 3))) correctly results in (1 2 3)

Viewing it in a repl, one does see the following:

ClojureScript:cljs.user> (array 1 2 3)
#<1,2,3>
ClojureScript:cljs.user> (aclone (array 1 2 3))
#js [1 2 3]

Not sure if that is helpful, but the array created does seem to slightly different.



 Comments   
Comment by David Nolen [ 15/Sep/14 12:24 PM ]

In Node.js JS instances may be created in other VM contexts. The same is true for moving JS objects between frames in the browser. You need to handle this case yourself via extend-type.





[CLJS-832] Cryptic error with macro loading Created: 03/Aug/14  Updated: 02/Dec/14  Resolved: 10/Sep/14

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

Type: Defect Priority: Minor
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

Windows 7 64-bit



 Description   

I'm getting a very cryptic error with ClojureScript and can't track down the exact cause. It seems to be something to do with macro loading in cljs.core...

Probably this is something I'm doing wrong... but either way I think the error message needs improving!

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation. All rights reserved.
..............
C:\Users\Mike\git\pinnacle>lein cljsbuild auto
Compiling ClojureScript.
Compiling "resources/public/js/app.js" from ["src/main/cljs"]...
←[31mCompiling "resources/public/js/app.js" failed.←[0m
←[31mclojure.lang.ExceptionInfo: ←[39m←[31m←[39m
←[35m core.clj:4327 clojure.core/ex-info←[39m
←[32m analyzer.clj:268 cljs.analyzer/error←[39m
←[32m analyzer.clj:1523 cljs.analyzer/analyze←[39m
←[32m analyzer.clj:1520 cljs.analyzer/analyze←[39m
←[32m compiler.clj:963 cljs.compiler/parse-ns[fn]←[39m
←[32m compiler.clj:963 cljs.compiler/parse-ns[fn]←[39m
←[32m compiler.clj:958 cljs.compiler/parse-ns←[39m
←[32m compiler.clj:953 cljs.compiler/parse-ns←[39m
←[32m compiler.clj:1041 cljs.compiler/to-target-file←[39m
←[32m compiler.clj:1073 cljs.compiler/compile-root←[39m
←[32m closure.clj:341 cljs.closure/compile-dir←[39m
←[32m closure.clj:381 cljs.closure/eval2990[fn]←[39m
←[32m closure.clj:292 cljs.closure/eval2927[fn]←[39m
←[32m closure.clj:395 cljs.closure/eval2977[fn]←[39m
←[32m closure.clj:292 cljs.closure/eval2927[fn]←[39m
←[32m compiler.clj:44 cljsbuild.compiler.SourcePaths/fn←[39m
←[35m core.clj:2485 clojure.core/map[fn]←[39m
←[36m LazySeq.java:42 clojure.lang.LazySeq.sval←[39m
←[36m LazySeq.java:60 clojure.lang.LazySeq.seq←[39m
←[36m RT.java:484 clojure.lang.RT.seq←[39m
←[35m core.clj:133 clojure.core/seq←[39m
←[35m core.clj:617 clojure.core/apply←[39m
←[35m core.clj:2514 clojure.core/mapcat←[39m
←[36m RestFn.java:423 clojure.lang.RestFn.invoke←[39m
←[32m compiler.clj:44 cljsbuild.compiler/cljsbuild.compiler.SourcePa
ths←[39m
←[32m closure.clj:955 cljs.closure/build←[39m
←[32m closure.clj:923 cljs.closure/build←[39m
←[32m compiler.clj:58 cljsbuild.compiler/compile-cljs[fn]←[39m
←[32m compiler.clj:57 cljsbuild.compiler/compile-cljs←[39m
←[32m compiler.clj:159 cljsbuild.compiler/run-compiler←[39m
←[33m NO_SOURCE_FILE:1 user/eval3325[fn]←[39m
←[33m NO_SOURCE_FILE:1 user/eval3325[fn]←[39m
←[36m LazySeq.java:42 clojure.lang.LazySeq.sval←[39m
←[36m LazySeq.java:60 clojure.lang.LazySeq.seq←[39m
←[36m RT.java:484 clojure.lang.RT.seq←[39m
←[35m core.clj:133 clojure.core/seq←[39m
←[35m core.clj:2780 clojure.core/dorun←[39m
←[35m core.clj:2796 clojure.core/doall←[39m
←[33m NO_SOURCE_FILE:1 user/eval3325←[39m
←[36m Compiler.java:6619 clojure.lang.Compiler.eval←[39m
←[36m Compiler.java:6609 clojure.lang.Compiler.eval←[39m
←[36m Compiler.java:6582 clojure.lang.Compiler.eval←[39m
←[35m core.clj:2852 clojure.core/eval←[39m
←[35m main.clj:308 clojure.main/eval-opt←[39m
←[35m main.clj:327 clojure.main/initialize←[39m
←[35m main.clj:362 clojure.main/null-opt←[39m
←[35m main.clj:440 clojure.main/main←[39m
←[36m RestFn.java:421 clojure.lang.RestFn.invoke←[39m
←[36m Var.java:419 clojure.lang.Var.invoke←[39m
←[36m AFn.java:163 clojure.lang.AFn.applyToHelper←[39m
←[36m Var.java:532 clojure.lang.Var.applyTo←[39m
←[36m main.java:37 clojure.main.main←[39m
←[31mCaused by: ←[39m←[31mjava.lang.NullPointerException: ←[39m←[31m←[39m
←[32m core.clj:49 cljs.core/import-macros[fn]←[39m
←[35m core.clj:2485 clojure.core/map[fn]←[39m
←[36m LazySeq.java:42 clojure.lang.LazySeq.sval←[39m
←[36m LazySeq.java:60 clojure.lang.LazySeq.seq←[39m
←[36m RT.java:484 clojure.lang.RT.seq←[39m
←[35m core.clj:133 clojure.core/seq←[39m
←[35m core.clj:2490 clojure.core/map[fn]←[39m
←[36m LazySeq.java:42 clojure.lang.LazySeq.sval←[39m
←[36m LazySeq.java:67 clojure.lang.LazySeq.seq←[39m
←[36m RT.java:484 clojure.lang.RT.seq←[39m
←[35m core.clj:133 clojure.core/seq←[39m
←[35m core.clj:687 clojure.core/concat[fn]←[39m
←[36m LazySeq.java:42 clojure.lang.LazySeq.sval←[39m
←[36m LazySeq.java:60 clojure.lang.LazySeq.seq←[39m
←[36m Cons.java:39 clojure.lang.Cons.next←[39m
←[36m RT.java:598 clojure.lang.RT.next←[39m
←[36m Compiler.java:6606 clojure.lang.Compiler.eval←[39m
←[36m Compiler.java:7064 clojure.lang.Compiler.load←[39m
←[36m RT.java:370 clojure.lang.RT.loadResourceScript←[39m
←[36m RT.java:361 clojure.lang.RT.loadResourceScript←[39m
←[36m RT.java:440 clojure.lang.RT.load←[39m
←[36m RT.java:411 clojure.lang.RT.load←[39m
←[35m core.clj:5530 clojure.core/load[fn]←[39m
←[35m core.clj:5529 clojure.core/load←[39m
←[36m RestFn.java:408 clojure.lang.RestFn.invoke←[39m
←[32m analyzer.clj:210 cljs.analyzer/load-core←[39m
←[32m analyzer.clj:1529 cljs.analyzer/analyze[fn]←[39m
←[32m analyzer.clj:1525 cljs.analyzer/analyze←[39m



 Comments   
Comment by Mike Anderson [ 03/Aug/14 10:26 PM ]

Here is the ns declaration in the app.cljs namespace:

(ns pinnacle.app
(:require-macros [cljs.core.async.macros :refer [go alt!]]
[secretary.core :refer [defroute]])
(:require [goog.events :as events]
[cljs.core.async :refer [put! <! >! chan timeout]]
[markdown.core :as md]
[om.core :as om :include-macros true]
[om.dom :as dom :include-macros true]
[secretary.core :as secretary]
[cljs-http.client :as http]
[pinnacle.utils :refer [guid]])
(:import [goog History]
[goog.history EventType]))

Comment by Eldar Gabdullin [ 14/Aug/14 4:14 PM ]

Same for me.

Comment by David Nolen [ 18/Aug/14 9:40 AM ]

Please produce a minimal reproducible case - no dependencies other than ClojureScript itself. Thanks.

Comment by Vadim Platonov [ 22/Aug/14 4:35 AM ]

Had the same error. This is due to Clojure version in the project not matching the Clojure version required by Clojurescript.

Please check your Clojure version. Clojurescript as of 0.0-2261 requires Clojure 1.6.0.





[CLJS-736] Functions folder and reducer broken for types nil and array + fix for typo Created: 29/Dec/13  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Jonas De Vuyst Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-736-alt.patch     Text File CLJS-736.patch     Text File CLJS-736-patch-1-redux.patch     Text File CLJS-alt-satisfies.patch    
Patch: Code and Test

 Description   

1. This currently doesn't work:

(->> nil
(r/map identity)
(r/reduce + 0))
; org.mozilla.javascript.JavaScriptException: Error: No protocol method IReduce.-reduce defined for type null

The reason for this is that reducers created by r/reducer or r/folder, invoke -reduce (of IReduce) directly. They thereby bypass the special case for nil in the function r/reduce.

2. An entirely analogous problem exists for collections of type array.

3. The patch CLJS-700 mistakenly defined coll-fold for the type cljs.core/IPersistentVector. This should have been cljs.core/PersistentVector. (There exists no protocol IPersistentVector in ClojureScript.)

I will shortly attach a patch that addresses all of the above problems by implementing IReduce for nil and array. The patch also includes unit tests.



 Comments   
Comment by Jonas De Vuyst [ 29/Dec/13 2:22 PM ]

Alternative patch in which r/reduce and r/fold treat arrays and nil as special cases – as opposed to having arrays and nil implement IReduce and CollFold.

The functions r/reducer, r/folder, and the protocol methods of r/Cat now call r/reduce and r/fold instead of calling -reduce and coll-fold directly.

This patch also fixes a bug in the coll-fold implementation for Cat, which previously used (reducef) as the initial value rather than (combinef). The new code is copied and pasted from the Clojure implementation and uses the fork-join stubs.

Comment by David Nolen [ 30/Dec/13 8:23 AM ]

The implements? should probably be a satisfies? in the second patch. Have you run any benchmarks of before/after the patch?

Comment by Jonas De Vuyst [ 30/Dec/13 11:24 AM ]

If I understand correctly then (satisfies? x y) is roughly equivalent to (or (implements? x y) (natively-satisfies? x y)).

If native types (nil, array, object currently) are treated as special cases then implements? seems more appropriate.

satisfies? works also, however, so I have attached a new 'alt' patch.

Comment by Jonas De Vuyst [ 30/Dec/13 11:26 AM ]

The first patch is in fact faster when running the following code:

(time (->> (repeat 1000 (vec (range 1000)))
vec
(r/mapcat identity)
(r/map inc)
(r/filter even?)
(r/fold +)))

This takes about 700 msecs. Using the first patch this terminates 100-300 msecs faster. This is after repeated (but informal) testing.

I guess the worry is that the first patch would slow down other random code since it involves extending the types nil, array, and object. I'm not sure what exactly I should test for though.

(Note that the 2nd and 3rd patch also contain a fix for Cat and include more unit tests. The first patch should preferably not be applied as-is.)

Comment by David Nolen [ 30/Dec/13 11:35 AM ]

Yeah you're timing too many things, including vec, range, lazy sequences. Also testing a small N. Take a look at the reducers example on the Mori README - https://github.com/swannodette/mori. Thanks.

Comment by Jonas De Vuyst [ 30/Dec/13 12:52 PM ]

I tried running the following code:

(let [coll (vec (repeat 1000 (vec (range 10))))]
  (time (doseq [n (range 1000)]
               (->> coll
                    (r/mapcat identity)
                    (r/map inc)
                    (r/filter even?)
                    (r/fold +)))))

Some of the last results I got were:

1st patch: 75680 msecs
2nd patch: 76585 msecs

Truth be told, although the first patch seemed to win most of the times, sometimes the second patch was faster.

One other thing I tried was removing the implements?/satisfies? check from the second patch and overriding the protocol method coll-fold for the type object instead (as in the first patch). This 'hybrid' approach generally (but not always) seemed to result in a slowdown.

I'm not sure how I should proceed. Should I perhaps just run both patches simultaneously for several minutes?

Comment by David Nolen [ 30/Dec/13 1:21 PM ]

This is still a bad way to do timing, you're recording the cost of range and seq'ing. Use dotimes.

Comment by Jonas De Vuyst [ 30/Dec/13 4:33 PM ]

Hm. I guess the lazy sequence does lead to a lot of allocations.

Alright, I rewrote my test and ran it a few more times. I now also tested on both vectors and arrays.

Patch 1 needed a slight tweak. When coll-fold is invoked, patch 1 only specifies a fallback for type object (i.e. r/reduce is called). I had to add the same fallback for type array. (This is weird!)

So here are the results.

For vectors:

(let [coll (vec (repeat 100 (vec (range 100))))]
  (time (dotimes [n 3000]
          (->> coll
              (r/mapcat identity)
              (r/map inc)
              (r/filter even?)
              (r/fold +)))))

Patch 1: 205872 msecs
Patch 2: 210756 msecs

For arrays:

(let [coll (into-array (repeat 100 (into-array (range 100))))]
  (time (dotimes [n 3000]
          (->> coll
              (r/mapcat identity)
              (r/map inc)
              (r/filter even?)
              (r/fold +)))))

Patch 1: 123567 msecs
Patch 2: 119704 msecs

I ran my tests a few times and the results were pretty consistent. Patch 1 is faster for vectors and patch 2 is faster for arrays.

This makes sense.

In patch 1 reducer will call -reduce directly. In patch 2, reducer first calls r/reduce, which calls -reduce if the collection is a vector and array-reduce if it's an array. Hence patch 2 contains an extra function call in the case of vectors, but avoids invoking a protocol method on a native type in the case of arrays.

Using macros (or copy and paste) the extra function call can be avoided. Would that be worth trying or is it more important to keep the code clean?

I just realized that patch 2 is semantically slightly different from what Clojure does, although perhaps this is a bug in Clojure: <https://groups.google.com/forum/#!searchin/clojure-dev/kv-reduce/clojure-dev/bEqECvbExGo/iW4B2vEUh8sJ>. My suggestion to use a macro (or copy and paste) to avoid the extra function call in patch 2, could also fix this discrepancy.

Comment by David Nolen [ 30/Dec/13 4:42 PM ]

How are you benchmarking this? With V8? JavaScriptCore? SpiderMonkey? In the browser? What optimization settings, etc.

Comment by Jonas De Vuyst [ 30/Dec/13 4:48 PM ]

I used repljs (Rhino?). I'll test again in a more realistic setting tomorrow.

Comment by David Nolen [ 30/Dec/13 4:54 PM ]

Yeah, benchmarking with Rhino isn't informative.

Comment by Jonas De Vuyst [ 31/Dec/13 1:40 AM ]

I compiled the same code (with n=3000) using cljs with "{:optimizations :advanced}".

I then tested it in the latest stable releases of Firefox, Chrome, and Safari. I closed all my browsers. For each browser I then followed the following procedure:

  • Open the browser
  • Open the developer console
  • Run the benchmark for patch 1
  • Run the benchmark for patch 2
  • Run the benchmark for patch 1 and write down the result
  • Run the benchmark for patch 2 and write down the result
  • Close the browser

Firefox:

  • Patch 1. Vectors: 26057 msecs
  • Patch 1. Arrays: 25026 msecs
  • Patch 2. Vectors: 26258 msecs
  • Patch 2. Arrays: 36653 msecs
  • Summary: Patch 1 is faster for vectors and arrays

Chrome:

  • Patch 1. Vectors: 7804 msecs
  • Patch 1. Arrays: 7092 msecs
  • Patch 2. Vectors: 7754 msecs
  • Patch 2. Arrays: 6768 msecs
  • Summary: Patch 2 is faster for vectors and arrays

Safari:

  • Patch 1. Vectors: 167230 msecs
  • Patch 1. Arrays: 108780 msecs
  • Patch 2. Vectors: 173940 msecs
  • Patch 2. Arrays: 110012 msecs
  • Summary: Patch 1 is faster for vectors and arrays

I'm not sure what to make of this.

Comment by Jonas De Vuyst [ 31/Dec/13 2:47 AM ]

I have attached a new version of the first patch.

This patch fixes an issue with r/Cat. (This issue was also addressed in the second and third patch. A unit test is included.).

This patch also fixes r/fold for arrays.

To summarize, a choice needs to be made between the following patches.

  • CLJS-736-patch-1-redux.patch
  • CLJS-736-alt.patch (uses implements?) / CLJS-alt-satisfies.patch (uses satisfies?)

The implementation details are patch-1-redux is more similar in spirit to the Clojure source code. The alt patches are more similar in spirit to the ClojureScript source code.

As explained above, the alt patches are semantically a bit different from the original Clojure source—but it's not clear which behavior is 'right'.

Comment by David Nolen [ 16/Jan/14 5:27 PM ]

The benchmarks would be more informative if they explained the performance before and after that patch.

Comment by Jonas De Vuyst [ 18/Jan/14 11:55 AM ]

r/reduce previously didn't work for nil or JavaScript arrays.

One reason why I have trouble recommending a patch is that I don't know what use case you would like to optimize for.

Comment by David Nolen [ 18/Jan/14 12:30 PM ]

Yes but now that we have new logic we can at least test the lack of regression on the other types.

Comment by David Nolen [ 18/Jan/14 12:40 PM ]

Ok I tried to apply this patch and run ./script/benchmarks in the repo but the patch will no longer apply. Can we rebase the patch on master. Thanks. If you also want to give the benchmarks a shot follow these instructions to install the JS engines - http://github.com/clojure/clojurescript/wiki/Running-the-tests. Then you can also run the benchmarks at the command line. I see there aren't any reducers benchmarks, I will add some.





[CLJS-803] Spurious undeclared Var warning in REPL Created: 05/May/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Paul Butcher Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

OS X Mavericks, Safari 7.0.3, Chrome 34.0.1847.131



 Description   

With the following minimal Compojure/ClojureScript project:

https://github.com/paulbutcher/csrepl

Compile with "lein cljsbuild once", then run "lein trampoline cljsbuild repl-listen". In another window, run "lein ring server".

The src/csrepl/core.cljs file defines an atom called app-state. This can be successfully examined in the REPL as follows:

$ lein trampoline cljsbuild repl-listen
Running ClojureScript REPL, listening on port 9000.
To quit, type: :cljs/quit
ClojureScript:cljs.user> csrepl.core/app-state
#<Atom: {:text "hello world"}>

But, if you switch to the csrepl.core namespace and do the same, a spurious "undeclared Var" warning is generated:

ClojureScript:cljs.user> (ns csrepl.core)
nil
ClojureScript:csrepl.core> app-state
WARNING: Use of undeclared Var csrepl.core/app-state at line 1 <cljs repl>
#<Atom: {:text "hello world"}>
ClojureScript:csrepl.core> (:text @app-state)
WARNING: Use of undeclared Var csrepl.core/app-state at line 1 <cljs repl>
"hello world"

Further, once you're in the csrepl.core namespace, the same warning is generated even when using the fully qualified name:

ClojureScript:csrepl.core> csrepl.core/app-state
WARNING: Use of undeclared Var csrepl.core/app-state at line 1 <cljs repl>
#<Atom: {:text "hello world"}>





[CLJS-655] Require main entry point (file) to serve as compilation root Created: 01/Nov/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Duplicate Votes: 1
Labels: None


 Description   

Per David Nolen in #clojure, a single file should serve as a compilation root, rather than the compiler needing to scan a given directory for files.

This leaves a couple of questions:

1. Are discovered dependencies then only ever loaded from the classpath?
2. If so, what does command-line (i.e. bin/cljsc compilation look like? Force people to export CLASSPATH appropriately, provide a set of alternative source paths via arguments, or other?

This would yield a fair bit of breakage downstream, e.g. cljsbuild.



 Comments   
Comment by David Nolen [ 01/Nov/13 7:10 PM ]

It's probably worth maintaining the source directory option of cljs.closure/build for some time with a deprecation warning to reduce breakage.

Comment by David Nolen [ 02/Dec/14 8:23 AM ]

superseded by CLJS-851





[CLJS-506] Flag to disable minification in advanced mode Created: 19/May/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-506-Add-a-debug-flag-to-the-compiler-options.patch    

 Description   

Problem:
Some code has different semantics when compiled in advanced mode.
Clojurescript tries to minimize those cases, but examples are usage of aget or forgotten externs files.
There exits closure compiler options to generate names based on original expressions, even with advanced optimizations.
However, there exists no clojurescript compiler option for that.

Proposal:
Add a :debug compiler option to clojurescript, which can be set to true to turn the following closure compiler options on:

  • generatePseudoNames true
  • anonymousFunctionNaming AnonymousFunctionNamingPolicy/UNMAPPED

Attached patch implements this, as a result the compiled test suite is indeed quite readable.



 Comments   
Comment by Herwig Hochleitner [ 19/May/13 10:11 AM ]

Patch 0001 depends on patches 0001 and 0002 from CLJS-480 because of changes to the test script.

Comment by David Nolen [ 29/Jul/13 10:32 PM ]

Dependencies between tickets is highly undesirable as there's no guarantee when a particular patch will get reviewed and applied.

Comment by David Nolen [ 02/Dec/14 6:17 AM ]

I'd rather see a patch that supplies :anon-fn-naming-policy as a compiler option. Thanks.

Comment by David Nolen [ 02/Dec/14 8:18 AM ]

fixed https://github.com/clojure/clojurescript/commit/318068f176daeb80e8d7dc7680c7cab05fc48069





[CLJS-622] defining zero-arity protocol method produces incomprehensible error message Created: 16/Oct/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: George Fraser Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojurescript 0.0-1913


Attachments: File cljs-622-20131104.diff     File protocol-warning-20131017-2.diff     File protocol-warning-20131017.diff     File protocol-warning-20131018.diff    

 Description   

In Clojure:
> (defprotocol P (f []))
IllegalArgumentException Protocol fn: f must take at least on arg [...]

In ClojureScript
> (defprotocol P (f []))
[ 100 lines of incomprehensible stack traces ]

Would be nice if it gave a better error message.



 Comments   
Comment by Travis Thieman [ 17/Oct/13 11:02 AM ]

Patch: 20131017

Moves the exception to the defprotocol macro itself and throws an exception matching Clojure's.

Comment by David Nolen [ 17/Oct/13 2:30 PM ]

It's preferable to throw ex-info, look around the compiler for an example. Also this error should include file, line and column information - again look around for ex-info examples to see what I mean. Thanks!

Comment by Travis Thieman [ 17/Oct/13 3:59 PM ]

v2 of the 20131017 patch uses ex-info to throw a more descriptive error message. To support this, ana/cljs-file is now bound in closure.clj/compile-file.

Comment by David Nolen [ 17/Oct/13 4:13 PM ]

The `binding` to setup `cljs.analyzer/cljs-file` is not necessary. If it is necessary you will need to show why it is. `cljs.compiler/compile-file` calls `cljs.compiler/compile-file*` which already does this.

Comment by Travis Thieman [ 17/Oct/13 4:26 PM ]

I encountered this as an issue because of the specific way I was testing compilation. I create a test file and compiled it using "/bin/cljsc test.cljs". This compiles from source files.

However, when it calls compile-file, no output file is specified, instead writing to stdout, e.g. opts contains {:output-to :print} and does not contain :output-file. As a result, the second branch of the if gets called, which compiles a sequence of forms instead of a file. As a result, the cljs-file never gets bound, even though there are input files.

The binding should still be moved to only affect the compile-form-seq branch.

(defn compile-file
  "Compile a single cljs file. If no output-file is specified, returns
  a string of compiled JavaScript. With an output-file option, the
  compiled JavaScript will written to this location and the function
  returns a JavaScriptFile. In either case the return value satisfies
  IJavaScript."
  [^File file {:keys [output-file] :as opts}]
  (binding [ana/*cljs-file* (.getPath ^java.io.File file)]
    (if output-file
      (let [out-file (io/file (output-directory opts) output-file)]
        (compiled-file (comp/compile-file file out-file opts)))
      (compile-form-seq (ana/forms-seq file)))))
Comment by Travis Thieman [ 18/Oct/13 10:05 AM ]

Patch: 20131018

Kept the ex-info throw, but removed the additional fix to closure.clj. I'll make a separate issue for the compilation problem.

Comment by David Nolen [ 18/Oct/13 10:23 AM ]

Upon further reflection, it's undesirable for this to throw. We should just convert it into a warning. I need to refactor warnings a bit so patch for this ticket should hold off until I get around to that.

Comment by Travis Thieman [ 18/Oct/13 10:42 AM ]

If the exception is not thrown here, a more generic exception is still thrown further down the line after the defprotocol macroexpansion, something about an invalid dot form. Is the plan to also fix that so that (defprotocol P (f [])) would fully compile? If not, what are the reasons for not wanting to throw here?

Comment by David Nolen [ 28/Oct/13 8:51 AM ]

CLJS-636 must be addressed first.

Comment by David Nolen [ 04/Nov/13 8:22 AM ]

CLJS-636 is resolved can we get an updated patch?

Comment by Travis Thieman [ 04/Nov/13 9:21 AM ]

Patch: 20131104

Add warning type :illegal-protocol-method and throw it on zero-arity protocol methods.

Comment by Chas Emerick [ 19/Mar/14 9:41 AM ]

This patch no longer applies, due to application of the patch from CLJS-627. Can you attach an updated patch?

Comment by David Nolen [ 02/Dec/14 8:08 AM ]

fixed https://github.com/clojure/clojurescript/commit/0ea0ca0c73f827f56436d738e8fca1a9c9b99f2c





[CLJS-469] Bad Exception message when multimethod has no dispatch-fn Created: 12/Feb/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File multi-fn-inamed.diff    
Patch: Code

 Description   

When a multi-fn has no dispatch method for a given value the current exception string prints the cljs.core/name function instead of the actual name of the mf. Minor bug but makes it kinda hard to track down which multi-fn actually failed to dispatch.

The attached patch fixes that but directly accessing the name property of the multi-fn which is not very clean but better than the current error. AFAICT cljs doesnt have the clojure.lang.Named protocol, which would probably be cleaner.



 Comments   
Comment by Thomas Heller [ 12/Feb/13 6:12 AM ]

Corrected the .patch

Comment by David Nolen [ 19/Feb/13 8:54 AM ]

ClojureScript now has an INamed protocol

Comment by Thomas Heller [ 20/Feb/13 3:42 PM ]

Updated the .patch to implement INamed for cljs.core/MultiFn, turning its name into a real symbol.

Tests pass but I dont know if that part of the code is actually tested.

Comment by Thomas Heller [ 20/Feb/13 3:43 PM ]

Oh, I'm not quite sure that the way I resolved the namespace for the symbol is totally correct. It works but I had to dig a bit.

Comment by David Nolen [ 02/Dec/14 6:28 AM ]

Rebased patch welcome, thanks!

Comment by Thomas Heller [ 02/Dec/14 7:44 AM ]

Uh, forgot about this. The important part of this issue was fixed a while back, the exception no long contains the source code to the cljs.core/name function.

The name however still is not fully qualified, updated the patch to only contain the INamed implementation for cljs.core/MultiFn so it will correctly report its full name.

Comment by David Nolen [ 02/Dec/14 7:47 AM ]

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





[CLJS-853] ^:metadata not working on fns (Cljs 0.0-2322) Created: 10/Sep/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: bug, metadata
Environment:

Clojure: 1.7.0-alpha2
ClojureScript: 0.0-2322
tools.reader: 0.8.8


Attachments: Text File 0001-CLJS-853-propagate-read-time-metadata-on-fn-and-reif.patch    

 Description   

Hi there, just ran into some unexpected behaviour:

;;; Clojure
(meta ^:foo [])                      ; {:foo true}
(meta ^:foo (fn []))                  ; {:foo true}
(meta (with-meta []    {:foo true})) ; {:foo true}
(meta (with-meta (fn []) {:foo true})) ; {:foo true}

;;; ClojureScript 0.0-2322
(meta ^:foo [])                      ; {:foo true}
(meta ^:foo (fn []))                  ; nil         <--
(meta (with-meta []    {:foo true})) ; {:foo true}
(meta (with-meta (fn []) {:foo true})) ; {:foo true}

Also tried with a random set of older dependencies:
Clojure: 1.6.0
ClojureScript: 0.0-2277
tools.reader: 0.8.5

Same result, so this seems to have been around for a while.
Does this seem like it might be a Cljs or tools.reader bug?
Shall I file somewhere else?

Thanks a lot, cheers!



 Comments   
Comment by David Nolen [ 10/Sep/14 5:44 PM ]

Definitely a bug and a patch is most welcome.

Comment by David Nolen [ 02/Dec/14 5:22 AM ]

Hrm, seems like this might be a tools.reader issue?

Comment by David Nolen [ 02/Dec/14 5:22 AM ]

Mind taking a look at this? Thanks!

Comment by Nicola Mometto [ 02/Dec/14 6:21 AM ]

This isn't a tools.reader issue, but I know the cause since it caused me some pain in tools.analyzer.
:fn and :reify nodes in clojure capture the form metadata and attaches it (evaluated) to the compiled object.

In other words, the read-time metadata is propagated at runtime for fn*/reify* just like it is for collection literals.

To work around this in tools.analyzer I wrap the :fn and :reify nodes in a wrapping-meta call, in cljs it's not significantly harder, here's a patch to fix it.

Comment by Nicola Mometto [ 02/Dec/14 6:25 AM ]

Since because of the indentation issue it's a bit hard to figure out what's being changed in analyze-wrap-meta, the only significant changes in that part of the diff are:

+  (analyze-wrap-meta
-        protocol-impl (-> form meta :protocol-impl)
-        protocol-inline (-> form meta :protocol-inline)
+         protocol-impl (-> form meta ::protocol-impl)
+         protocol-inline (-> form meta ::protocol-inline)
+         form (vary-meta form dissoc ::protocol-impl ::protocol-inline ::type)]
Comment by David Nolen [ 02/Dec/14 7:44 AM ]

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





[CLJS-707] script/bootstrap downloads unspecified version of closure-compiler Created: 29/Nov/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Ryan Berdeen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

script/bootstrap downloads http://dl.google.com/closure-compiler/compiler-latest.zip (https://github.com/clojure/clojurescript/blob/r2080/script/bootstrap#L64), while the project.clj specifies a specific version.

As a result, you'll get a different version of the compiler when using following the Quick Start guide. The latest version of the Closure Compiler was compiled for Java 7, causing cljsc to fail with a java.lang.UnsupportedClassVersionError under Java 6.



 Comments   
Comment by David Nolen [ 29/Nov/13 4:02 PM ]

Does Closure Compiler no longer support Java 6?

Comment by Ryan Berdeen [ 30/Nov/13 1:16 PM ]

The release notes for v20131118 say "Move compiler to Java 7": https://groups.google.com/d/topic/closure-compiler-discuss/_T5Aai2sg68/discussion.

This release can still be built for Java 6 using ant jar -Dant.build.javac.source=1.6 -Dant.build.javac.target=1.6, but the current master makes use of Java 7 features in one class.

The Java 6/Java 7 support seems like it should be it's own issue. It's just how I discovered the problem, which is that running the bootstrap script doesn't give a consistent result. All of the other dependencies apart from closure-compiler have explicitly set versions, so I'm not sure I understand the reasoning. Shouldn't it download from central.maven.org with a version that matches project.clj?

Comment by David Nolen [ 30/Nov/13 1:44 PM ]

project.clj is just a convenience for people actually hacking on the compiler for now.

Comment by David Nolen [ 02/Dec/14 6:23 AM ]

We simply stay abreast of the Closure Compiler.





[CLJS-475] Node.js target fails with optimizations set to :none or :whitespace Created: 21/Feb/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Paul Gearon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: Compiler, bug
Environment:

Clojure 1.5.0-RC16
Clojurescript 0.0-1586
java version "1.7.0_04"
Java(TM) SE Runtime Environment (build 1.7.0_04-b21)
Java HotSpot(TM) 64-Bit Server VM (build 23.0-b21, mixed mode)
OSX Mountain Lion 10.8.2


Attachments: GZip Archive out-none.tar.gz     GZip Archive out-whitespace.tar.gz     File pr.js-none     File pr.js-whitespace    

 Description   

Compiling a hello world program for Node.js works fine if using optimizations of :advanced or :simple, but if using :none or :whitespace then an error will be reported for either "goog undefined" or "goog.string" undefined respectively.

The program is shown here:

(ns pr.core)
(defn -main []
  (println "Hello World!"))
(set! *main-cli-fn* -main)

This program is in src/cljs/pr/core.cljs. The repl line used to compile is:

(cljs.closure/build "src/cljs" {:output-to "src/js/pr.js" :target :nodejs :pretty-print true :optimizations :none})

When compiled with optimizations of :none, the output is:

$ node src/js/pr.js 

/Users/pag/src/test/clj/pr/src/js/pr.js:1
(function (exports, require, module, __filename, __dirname) { goog.addDependen
                                                              ^
ReferenceError: goog is not defined
    at Object.<anonymous> (/Users/pag/src/test/clj/pr/src/js/pr.js:1:63)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.runMain (module.js:492:10)
    at process.startup.processNextTick.process._tickCallback (node.js:244:9)

When running with optimizations of :whitespace the output is:

$ node src/js/pr.js 

/Users/pag/src/test/clj/pr/src/js/pr.js:493
goog.string.Unicode = {NBSP:"\u00a0"};
                    ^
TypeError: Cannot set property 'Unicode' of undefined
    at Object.<anonymous> (/Users/pag/src/test/clj/pr/src/js/pr.js:493:21)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.runMain (module.js:492:10)

When running with optimizations of either :simple or :advanced, the output is:

$ node src/js/pr.js 
Hello World!

I have included the two javascript output files that match the above errors.



 Comments   
Comment by Paul Gearon [ 21/Feb/13 4:40 PM ]

Remaining generated files

Comment by David Nolen [ 25/Feb/13 3:46 PM ]

This is a known bug. We need goog.require/provide to actually mean something to Node.js. I'm not sure how this can be made to work. I've been hoping for a patch for this since ClojureScript was first announced, but I haven't seen anything yet.

Comment by David Nolen [ 02/Dec/14 6:20 AM ]

long ago fixed in master.





[CLJS-457] Implement notion of "unbound", i.e. uninitialized variables, in ClojureScript to mimic Clojure's behaviour Created: 15/Jan/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Frank Siebenlist Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

clojurescript r1552



 Description   

The following REPL snippets show different behavior for "unbound" vars in cljs and clj:

--------
ClojureScript:cljs.user> (def a nil)
nil
ClojureScript:cljs.user> (def b)

ClojureScript:cljs.user> (= a b)
true
--------
user=> (def a nil)
#'user/a
user=> (def b)
#'user/b
user=> (= a b)
false
--------

As a possible solution, D. Nolen suggested that we could "simulate this by creating a Unbound type and initializing def'ed vars without init expressions to instances of it."



 Comments   
Comment by David Nolen [ 02/Dec/14 6:20 AM ]

The usefulness of this is dubious.





[CLJS-453] ArrayVector for small vectors Created: 04/Jan/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

master cljs, chrome


Attachments: Text File 0001-ArrayVector-New-vector-implementation-for-small-vect.patch    

 Description   

Just like we have an ArrayMap for small maps, I propose to have an ArrayVector for small vectors.

Use cases:

  • pair of values, e.g. coordinates, triples and other tuple values
  • returning multiple values from a function and subsequent destructuring in a caller fn

ArrayVector has 100x faster vector creation compared to PersistentVector.
With an ^ArrayVector hint, it offers more than 10x faster destructuring. Without it, it is still about 40% faster.

Example of such destructuring:

(defn foo [a b] 
  [(+ a b) (- a b) (* a b)])

(defn bar []
  (let [[plus minus times] ^ArrayVector (foo 1 2)]
    (str "bla bla" plus "blaah" minus)))

I've attached a patch with complete implementation of such vector, supporting all basic functionalities as well as transients. This patch also replaces default vector implementation with ArrayVector, falling back to PersistentVector for large vectors.

ArrayVector implementation can also be found at array-vec branch at https://github.com/wagjo/clojurescript/tree/array-vec



 Comments   
Comment by David Nolen [ 04/Jan/13 4:54 PM ]

Thanks! This interesting, it would be helpful to see a more comprehensive set of benchmarks on jsperf.com.

Comment by Jozef Wagner [ 19/Jul/13 1:09 PM ]

I have prepared benchmarks for ArrayVector. One is at http://jsperf.com/cljs-arrayvector/2 and the other one is at http://wagjo.github.io/benchmark-cljs/ . Note that both benchmarks use same code, at https://github.com/wagjo/benchmark-cljs and use advanced compilation and bleeding edge ClojureScript.

Results on my linux machine in chrome are following:

Results for benchmark small-vector/create for 10000 iterations, median from 300 runs
000.031ms - baseline
000.128ms - array
000.396ms - array vector
000.878ms - persistent vector

Results for benchmark small-vector/access for 10000 iterations, median from 50 runs
000.044ms - baseline
000.244ms - array
000.930ms - array vector
001.110ms - persistent vector
000.254ms - destructure array vector
001.224ms - destructure persistent vector

Results for benchmark small-vector/conj for 10000 iterations, median from 100 runs
000.035ms - baseline
001.193ms - array
001.794ms - array vector
002.119ms - persistent vector

Edit: Table is updated with more precise data.

Comment by David Nolen [ 23/Jul/13 1:02 PM ]

These are interesting results, will look into this some more. I will say that type hinting like that is not officially supported, ^not-native is experimental but more likely to be something we allow.

Comment by Michał Marczyk [ 23/Jul/13 9:27 PM ]

If I understand correctly, the benefit in destructuring ArrayVector}}s comes from directly accessing the underlying array. We could do the same with short PVs if this "shortness" information could be conveyed through a type hint. Not saying that we necessarily should, though there might be an upside with hinting {{^shortvec rather than ^ArrayVector from a semantic POV (hinting a property vs. hinting implementation).

In any case, I suppose there would be very nearly no cost to introducing a separate AV type (conj beyond 32 elements would simply do exactly what it does right now; conj! needs to allocate a new TV, but the cost should be acceptable), so, if it speeds things up...

I'll run some experiments and get back to this ticket with further comments.

Comment by David Nolen [ 16/Nov/13 11:38 PM ]

I'm not excited about more type hints I've landed a performance improvement for conj, PVs run circles around AVs now. PV construction time can be made competitive with AV construction time trivially by making the compiler inline instantiation.

This leaves lookup time, I wonder if we could close the gap? It would be nice to rerun the benchmarks after we land some more PV enhancements.

Comment by David Nolen [ 17/Nov/13 12:00 PM ]

PersistentVector creation time is now nearly as fast as ArrayVector in master. -nth is still a bit slower under V8, but surprisingly under WebKit Nightly PVs appear to outperform AVs here.

Comment by Jozef Wagner [ 19/Nov/13 3:47 AM ]

Thank you. I've updated the benchmarks which confirm that PV are now on par with AV in terms of creation and conjoining. The lookup time optimization boils down to this core, which can be changed to support PVs too.

Comment by David Nolen [ 19/Nov/13 11:03 AM ]

We're not going to add special cases like that. It's in the works to add more inference to the compiler so that protocol backed fns will inline more simply with less hinting.

Comment by David Nolen [ 02/Dec/14 6:19 AM ]

I believe Zach Tellman's proposal for tuples will likely land and be the approach we adopt.





[CLJS-478] ClojureScript js->clj not converting Created: 26/Feb/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Mike Longworth Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

ClojureScript 0.0-1586 build #22, Chrome, OSX



 Description   

I've updated to 0.0-1586 build #22 from a much older release: 0.0-1450

I'm now geting a problem with (js->clj token) not converting token to a map; this worked before I upgraded.

token is generated by the google OAuth 2 process: "gapi.auth.authorize"

I don't think the problem is with js->clj rather something about token so it isn't recognised as a js/Object.

(println (expose token true)) gives:

access_token = ya29.AHES6ZSgnk3Ws5bB-2aDx41Bbr335hKugjZJfcNAs83d121S306fxy64
token_type = Bearer
expires_in = 3600
client_id = 52351124085.apps.googleusercontent.com
scope = https://www.googleapis.com/auth/drive.file,https://www.googleapis.com/auth/drive.metadata.readonly,https://www.googleapis.com/auth/drive.readonly
g_user_cookie_policy = undefined
cookie_policy = undefined
issued_at = 1361807171
expires_at = 1361810771

This looks ok.

however
> token

gives:

"Error evaluating:" token :as "esef.client.evidence.token"
#<TypeError: Object [object Object] has no method 'toString'>
TypeError: Object [object Object] has no method 'toString'

> (type token)

gives a blank line. I think this is why js->clj isn't converting.

> (alength token)

gives a blank line.

(.-token_type token)

Correctly gives ya29.AHES6ZSgnk3Ws5bB-2aDx41Bbr335hKugjZJfcNAs83d121S306fxy64

I haven't managed to recreate 'token' with a minimal test case but I'm fairly new to javascript. It's impractical to to include my OAuth 2 code because its tied into all my google security.
If you can suggest anything that will provide you with more info on 'token' I'll try and help.

As a work-around I manually create the MAP by explicitly reading each property, I couldn't get a list of properties.



 Comments   
Comment by David Nolen [ 26/Feb/13 11:00 AM ]

type just returns the constructor property of an Object. For some reason this is returning undefined instead of value. It's likely that this has nothing to do with ClojureScript and more to do with Google Closure. Have you investigated whether the OAuth token constructor has changed in Closure itself?

Comment by Mike Longworth [ 26/Feb/13 11:21 AM ]

I don't think the token object or the apis have any significant relationship with closure, they are loaded dynamically from google (gapi.client.load):
https://code.google.com/p/google-api-javascript-client/wiki/ReferenceDocs#OAuth_2.0_Token_Object
https://code.google.com/p/google-api-javascript-client/wiki/GettingStarted

Comment by David Nolen [ 26/Feb/13 11:25 AM ]

Did type return something for you instead of undefined in 1450?

Comment by Mike Longworth [ 26/Feb/13 11:40 AM ]

Sorry I can't answer that; js->clj just worked so I didn't probe too deeply but looking at the code in js->clj It looks like it must have. I tried the original implementation of js->clj before the recent changes but confirmed this still suffers from the same problem (type x) js/Object):

(defn js->clj
"Recursively transforms JavaScript arrays into ClojureScript
vectors, and JavaScript objects into ClojureScript maps. With
option ':keywordize-keys true' will convert object fields from
strings to keywords."
[x & options]
(let [{:keys [keywordize-keys]} options
keyfn (if keywordize-keys keyword str)
f (fn thisfn [x]
(cond
(seq? x) (doall (map thisfn x))
(coll? x) (into (empty x) (map thisfn x))
(goog.isArray x) (vec (map thisfn x))
(identical? (type x) js/Object) (into {} (for [k (js-keys x)]
[(keyfn k)
(thisfn (aget x k))]))
:else x))]
(f x)))

Comment by David Nolen [ 29/Jul/13 10:39 PM ]

There's currently not enough information on this ticket.





[CLJS-505] reduce-kv doesn't work on this dataset Created: 12/May/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Ben Burdette Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

Chromium browser on kde mint



 Description   

reduce-kv seems to fail with certain datasets in clojurescript but works in clojure. plain reduce works in both. Test dataset and functions:

(def tst '{0 (0.16245458703559382 0.16299961144753652), 1 (0.17095488518209803 0.1662484375243468), 2 (0.19709700082585824 0.1876248353737042 0.18162592547515274), 3 (0.21941419798490325 0.21807914100386067 0.21533585716040135 0.21307519864852958 0.20703453840254027 0.20459935129415552), 4 (0.2254558759699653 0.22553167887455705 0.2234426184638531), 5 (0.23427611508459756 0.2325434766605363 0.22990656317710662 0.22720478209110223), 6 (0.236386202300682 0.237142940385559 0.23819488667547528 0.23768692982616965 0.23701568705350426 0.2519226069678777), 7 (0.23496439825060444 0.2366135730772095 0.2358487071572212 0.2343505595436482 0.2339754113241729 0.23574632954272087 0.2342899279492302 0.23394402094793673 0.2253325456308313 0.23115349467838542 0.23764956541775945 0.24569178483873644 0.24935940319340053), 8 (0.20547261591662125 0.20726089154784122 0.21592818189465432 0.21750737743637422), 9 (0.18799738410585773 0.195400717149256 0.19631498833296726), 10 (0.1719302696659648 0.17805792401151166 0.18237267730425885), 11 (0.1623645568284537 0.16481390708298796 0.16716904262359314), 12 (0.16156804839942243 0.16211838120784386), 13 (0.16995648347966535 0.16559349825186068), 14 (0.19176771652550495 0.17696322624338934), 15 (0.23973485828855146 0.22420582245618542 0.2145975801770993 0.20208630026741867), 16 (0.2946896009888785 0.28676176338007714 0.28325454941805994 0.2716914099321759 0.26202250017945194 0.25452401319317824 0.2458771184353157), 17 (0.33300741045445653 0.3282046117335755 0.32330639472087175 0.3191873692036449 0.31290612056170736 0.30418343504691503 0.3004101566486822), 18 (0.341869650592079 0.34431149263906713 0.3457698150448536 0.34421452862514235 0.3429726966458358 0.3388988973880783 0.33579154357727864), 19 (0.2968748091051211 0.3023852506442652 0.3121637065261914 0.31141190562184173 0.32205821745972246 0.32898706631919056 0.33322752162957553 0.3396066565159988), 20 (0.2317683974879252 0.24538548478582375 0.2645130722935203 0.27362218406430894), 21 (0.20128274621982342 0.21819083560963268), 22 (0.1732991500114435 0.1811216605973363 0.19151484878018085), 23 (0.16575238432187128)})

; this returns nil in clojurescript, returns a sorted dataset in clojure.
(defn sortpi [pi]
(reduce-kv (fn [c k v] (assoc c k (sort v))) {} pi))

(sortpi tst)

; this version works in both clojurescript and clojure:
(defn sortpi2 [pi]
(reduce (fn [c [k v]] (assoc c k (sort v))) {} pi))

(sortpi2 tst)



 Comments   
Comment by Michał Marczyk [ 12/May/13 10:49 PM ]

Works for me on master. Which version of ClojureScript are you using?

Comment by David Nolen [ 18/Aug/13 5:23 PM ]

Lowering priority until we get more information.





[CLJS-507] Persistent Data Structure Benchmark Created: 20/May/13  Updated: 02/Dec/14

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

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


 Description   

Put together an easy to run series of persistent data structure benchmarks that can be easily run. For example we would like to submit to this Mozilla ticket http://bugzilla.mozilla.org/show_bug.cgi?id=874174






[CLJS-555] CLONE - Implement ratios Created: 27/Jul/13  Updated: 02/Dec/14

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

Type: Enhancement Priority: Trivial
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Clojure.java contains support for Ratio types. It would be nice to have them in Clojurescript as well, but as far as I can tell this would be new development (please comment if I'm wrong). That is, there is no implementation of Ratio types in GClosure so this feature would need to be implemented from the ground up. In additional to the Ratio type, the following corresponding functions would also need implementations:

  • `ratio?`
  • `numerator`
  • `denominator`
  • `rationalize`

Plus the ratio type would need to be rolled into the existing mathematical frameworkings.

Imported from github issue #66



 Comments   
Comment by David Nolen [ 02/Dec/14 6:14 AM ]

Lower priority, unsolvable without numerics overhaul.





[CLJS-610] Duplicate protocols in defrecord are silently ignored Created: 06/Oct/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Russell Mull Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojurescript 1913


Attachments: File duplicate-protocols-20131015.diff    

 Description   

It is currently not an error to define a type with two implementations of the same protocol; which implementation is actually used appears to be arbitrary. This shouldn't be allowed, as it leads to mysterious misbehavior like the following:

(defrecord MyCountableThing [count]
  ICounted
  (-count [_] count))

Calling count on any instance of MyCountableThing will of course return 1, which is the the answer given by the default implementation of ICounted in the defrecord macro. I'm not arguing here that the above code should necessarily work; indeed the semantics of defrecord make it reasonable for it not to. But it should be an error rather than doing nothing.



 Comments   
Comment by Travis Thieman [ 15/Oct/13 3:51 PM ]

Patch: 20131015

Warns on duplicate protocols in defrecord and deftype invocations. This also found a few instances in cljs/cljs.core with duplicated code, which I removed.

Clojure allows this without an error and will allow one of the duplicated protocol definitions through to the record. It seemed more reasonable to throw a warning in this case so that valid code in Clojure will remain valid code in ClojureScript.

Comment by David Nolen [ 28/Oct/13 8:51 AM ]

CLJS-636 needs to be addressed first.

Comment by David Nolen [ 02/Dec/14 6:13 AM ]

fixed in master





[CLJS-667] validate extend-type and extend-protocol shape Created: 07/Nov/13  Updated: 02/Dec/14

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

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


 Description   

deftype and defrecord are sugar over the extend-type grouping, this is a source of confusion we should signal an error/warning if extend-type / extend-protocol malformed.






[CLJS-703] warn if protocol signature vector is empty Created: 27/Nov/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

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


 Description   

currently get a mysterious exception






[CLJS-678] warn on keyword literals used in identical? test Created: 13/Nov/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

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


 Comments   
Comment by David Nolen [ 02/Dec/14 6:11 AM ]

The change has been in place long enough.





[CLJS-686] Auto-generate externs for foreign libs Created: 19/Nov/13  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

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


 Description   

Chouser has something simple and nice that should work for most use cases - https://gist.github.com/Chouser/5796967






[CLJS-891] Defs in "parent" namespaces clash with "child" namespaces with the same name? Created: 28/Nov/14  Updated: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Russell Dunphy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, namespace
Environment:

Clojurescript 0.0-2371, OSX



 Description   

This has had me totally flummoxed for a few hours, but I have finally been able to create a minimal project replicating the problem based on the mies template: https://github.com/rsslldnphy/cljs-ns-issue.

The problem seems to happen when a "parent" namespace, for example `my-project.foo`, contains a def with the same name as a "child" namespace, eg if there is a namespace `my-project.foo.bar`, and a def named `bar` in `my-project.foo`, and both those namespaces are required by `my-project.core`, then calling functions in `my-project.foo.bar` ends up with an Uncaught TypeError: Cannot read property 'call' of undefined. Sometimes, depending on which ns requires which, I've also seen an Uncaught Error: Namespace "cljs_ns_issue.foo.bar" already declared.

I don't think I'm doing a particularly good job of explaining this so it might be easier to look at the code. The crux is: comment out this line and the code works, leave it in and you get an error.



 Comments   
Comment by Francis Avila [ 28/Nov/14 2:01 PM ]

Clojurescript implements namespaces with Google Closure compiler's require/provide system. Unfortunately that system does not have a hard distinction between names and namespaces like Clojure does but instead is more like a sloppy java classname. The crux of it is that vars and namespaces occupy the same tree of js objects and thus their names may not overlap.

Compare the cljs on the left with the emitted javascript (This isn't exactly what is happening, but only in essence):

(ns my-project.foo ; goog.provide('my_project.foo') // something like window.my_project = {foo: {}}
  (:require my-project.foo.bar)) //goog.require('my_project.foo.bar')

;; the "require" evaluates the other namespace, which sets
;; // window.my_project.foo = {bar:{}};
;; // my_project.foo.bar.baz = "some var in the bar ns";
;; Now window.my_project.foo.bar = {baz: "some var in the bar ns"};

(defn bar [] 1)         ; my_project.foo.bar = (function bar(){return 1;});
;; Now the js object that was the bar namespace is gone, replaced with this function.

(my-project.foo.bar/baz)
; my_project.foo.bar.baz.call() // Uncaught TypeError: Cannot read property 'call' of undefined.

;; Alternatively, if (ns my-project.foo.bar) got evaluated *after* my-project.foo namespace was
;; evaluated, then my-project.foo/bar is defined, and the emitted goog.provide('my-project.foo.bar')
;; throws "Uncaught Error: Namespace "my_project.foo.bar" already declared".

So basically this is a leaky abstraction. In Clojurescript, you cannot define a var whose ns-qualified name matches that of another namespace: the slash between name and namespace is not real.

I think the only possible things that can be done are either:

  • Warn at compile-time if a var and a namespace object-path clash. Obviously there may still be runtime-created vars.
  • Put namespace vars behind some magic name. E.g. (ns foo.bar)(def baz 1) becomes goog.provide('foo.bar.__NS__'); foo.bar.__NS__.baz = 1; This would significantly uglify cljs names exported to js (and break existing code), and the magic name could never be used as a var name.
Comment by David Nolen [ 02/Dec/14 6:07 AM ]

We actually already have some logic for this in place, we track namespace segments for this reason. This is a different manifestation of it than previously encountered. I think any further workarounds are likely more trouble than they are worth (debugging complications) - probably the best thing to do is report a warning if we detect a clash.





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 02/Dec/14

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

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


 Description   

When a this-as expression gets put in a "scoping function", e.g. in a let-binding, the value bound via this-as refers to the scoping function, and not to the outer scope.

Example:

(def foo
  (js-obj
    "bar" "baz"
    "getBarRight" (fn [] (this-as self (.-bar self)))
    "getBarWrong" (fn []
                    (let [bar (this-as self (.-bar self))]
                      bar))))
     
(.log js/console (.getBarRight foo)) ;; => "baz"
(.log js/console (.getBarWrong foo)) ;; => undefined

Whereas foo.getBarRight expands to something like

function() {
  var self = this; // this refers to foo
  return self.bar; // returns "bar"
}

foo.getBarWrong on the other hand expands to

function() {
  var bar = function() {
    var self = this; // this refers to enclosing function
    return self.bar; // returns undefined
  }();
  return bar; // returns undefined
}





[CLJS-689] js/-Infinity munges to _Infinity Created: 20/Nov/13  Updated: 02/Dec/14

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

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


 Description   

A dumb special-casing of js/ munging should fix.






[CLJS-785] :refer-macros in conjunction with :refer not working Created: 17/Mar/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Kevin Marolt Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

The :refer-macros directive isn't working when used in conjunction with the :refer directive. Compiling a ns-form like

(ns foo
  (:require
    [bar :refer [baz] :refer-macros [quux]]))

produces the compiler error

Each of :as and :refer options may only be specified once in :require / :require-macros; offending spec: (bar :refer [baz] :refer [quux])

The problem seems to be with analzyer.cljs/desugar-ns-specs. Invoking

(desugar-ns-specs '((:require [bar :refer [baz] :refer-macros [quux]])))

returns

'((:require-macros (bar :refer [baz] :refer [quux])) (:require (bar :refer [baz])))

instead of

'((:require-macros (bar :refer [quux])) (:require (bar :refer [baz])))

Furthermore, there seems to be a typo in the local remove-sugar function on line 1094 [1]: It should probably be

(let [[l r] (split-with ...)] ...) ;; no '&' before 'r'

instead of

(let [[l & r] (split-with ...)] ...)

Otherwise, something like

(desugar-ns-specs '((:require [bar :include-macros true :as b])))

becomes

((:require-macros (bar :as b)) (:require (bar)))

instead of

((:require-macros (bar :as b)) (:require (bar :as b)))

[1] https://github.com/clojure/clojurescript/blob/78d20eebbbad17d476fdce04f2afd7489a507df7/src/clj/cljs/analyzer.clj#L1094



 Comments   
Comment by David Nolen [ 02/Dec/14 5:43 AM ]

Is this still a problem in master? Thanks.





[CLJS-794] RegExp flags are being dropped by `string/replace` Created: 09/Apr/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

`clojure.string/replace` accepts either a string or pattern argument to match against.

For pattern arguments, the current implementation discards the original RegExp and creates a new one:
`(.replace s (js/RegExp. (.-source match) "g") replacement)`

This is killing any flags on the original pattern (case insensitivity, for example). As a result, things like `(str/replace "Foo" #"(?i)foo" "bar")` currently fail. The result is "Foo", it should be "bar".

Can I submit a patch that'll check for and preserve other (i/m/y) flags?

Thanks



 Comments   
Comment by David Nolen [ 02/Dec/14 5:42 AM ]

A patch is welcome for this. Thanks.





[CLJS-799] Having a namespace end with ".cljs" produces wrong source map Created: 16/Apr/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Sven Richter Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: maps, namespace, source
Environment:

Windows 7
JDK 1.7
CLJS version: 0.0-2138 and 0.0-2156 (probably hits other versions too, but I only tested these two)



 Description   

When an clojurescript namespaces ends with ".cljs" I cannot see the source file in google chrome.
Repro steps:

1. Create a new luminus project with: lein new luminus cljsbug +cljs +http-kit
2. Change the project.clj cljsbuild -> compiler setting to:

:compiler
{:output-to "resources/public/js/site.js"
:output-dir "resources/public/js/out"
:optimizations :none
:source-map true
}

3. Change cljsexample.html file to:

<script type="text/javascript" src="js/out/goog/base.js"></script>
<script type="text/javascript" src="{{servlet-context}}/js/site.js"></script>
<script type="text/javascript">goog.require("cljsbug.main");</script>

4. Now start the server with "lein run -dev" and "lein cljsbuild auto"
5. Open localhost:3000/cljsexample
6. Check for the source file in google chrome

It should be there now and correct.
Now to reproduce the problem do this:

7. Change the namespace of the main.cljs file to: ns cljsbug.main.cljs
8. Change the cljsexample.html goog.require line to:

<script type="text/javascript">goog.require("cljsbug.main.cljs");</script>

9. Restart the cljsbuild with: lein do cljsbuild clean, cljsbuild auto
10. Reload the /cljsexample page in google chrome and the source mapping wont be there anymore.



 Comments   
Comment by Sven Richter [ 16/Apr/14 2:38 PM ]

Just to clear things up. Steps 1 to 6 are not needed to reproduce the problem. It is sufficient to go through steps 7 to 10 on any project that uses a similar cljsbuild setting.
It is important that optimizations are set to :none.

Short repro version.

1. Have cljs project with the following settings:
:compiler
{:output-to "resources/public/js/site.js"
:output-dir "resources/public/js/out"
:optimizations :none
:source-map true
}

2. Change any cljs file namespace and add ".cljs" to the namespace.
3. Have the new namespace required in the html file by google like this: goog.require("cljsbug.main.cljs")

4. Open a page in the browser and see that the source maps are missing.

Comment by David Nolen [ 02/Dec/14 5:41 AM ]

A patch for this is welcome!





[CLJS-882] cljs.reader/read-string throws errors when reading keywords that begin with integers Created: 05/Nov/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Joseph Fahey Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: keywords, reader
Environment:

Using clojurescript 0.0-2371. Error seen with both phantomjs and Chrome



 Description   

(cljs.reader/read-string ":1") throws an error: TypeError: 'null' is not an object (evaluating 'a[(0)]')
at read_keyword (:474)



 Comments   
Comment by Joseph Fahey [ 05/Nov/14 4:10 PM ]

read-keyword, in reader.cljs, matches against symbol-pattern, which disallows symbol names that begin with numbers. Symbols can't begin with numeric characters, but keywords actually begin with ":", so in a keyword like :1 , "1" is actually the second character.

Comment by Francis Avila [ 05/Nov/14 8:01 PM ]

Your reasoning that in a keyword the number is the second character is precisely one of the unclear points about keyword and symbol parsing. Some implementations say yes, and some do not, and there is no "spec" unambiguous enough to decide the issue.

This issue is a duplicate of CLJS-677. The comments in there go in to much greater detail.

Comment by Joseph Fahey [ 06/Nov/14 2:52 AM ]

I'll leave it at that then.

I would like to add that the current state of affairs is rather confusing, because keywords like :1 seem to work fine in clojurescript, except when deserializing with cljs.reader/read-string, from localStorage for example, which fails without a clear explanation.

Comment by Francis Avila [ 06/Nov/14 12:19 PM ]

Agreed, the current state of affairs is not good but the proper fix would be:

  1. Produce a rigorous formal specification of the reader syntax for Clojure (and variants/subsets for edn, Clojurescript, ClojureCLR). (Including consideration of unicode chars, etc.)
  2. Unifying all reader implementations around these specifications (across multiple projects).
  3. Dealing with code breakage in upstream libraries.

Understandably the core developers would probably think this is a very large effort with a lot of disruption for very little gain. I advise just avoiding edge cases in the spec, like :1, :supposedly:ok:keyword, non-ascii characters, etc, in both code and edn.

Comment by Joseph Fahey [ 07/Nov/14 4:15 AM ]

One last thing about the confusion this causes. The problem I see is that {:1 "one"} compiles without any problem in Clojurescript, (keyword? :1} returns true, and (keyword "1") returns :1. The only time the problem comes up is when using reader/read-string. It seems to me that this should be coherent at least within Clojurescript, even if there are discrepancies with the other implementations.

And when using :keywordize :true with externally supplied data, it is hard to be sure that some of the JSON keys won't begin with a digit. (This is how I stumbled onto this.)





[CLJS-814] clojure.string/reverse breaks surrogate pairs Created: 12/Jun/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: string
Environment:

r2227, NOT under Rhino.


Attachments: Text File cljs-814.patch    
Patch: Code and Test

 Description   

clojure.string/reverse will reverse a string by code units instead of code points which causes surrogate pairs to become reversed and unmatched (i.e., invalid utf-16).

For example, in clojurescript (clojure.core/reverse "a\uD834\uDD1Ec") will produce the invalid "c\uDD1E\uD834a" instead of "c\uD834\uDD1Ea". Clojure produces the correct result because the underlying Java String.reverse() keeps surrogate pairs together.

Note that clojurescript running under Rhino will produce the same (correct) result as clojure, probably because Rhino is using String.reverse() internally.

Attached patch gives clojure.string/reverse the exact same behavior in clj and cljs (including non-commutativity for strings with unmatched surrogates).

(Also, neither clojure nor clojurescript nor java reverse combining characters correctly--the combining character will "move" over a different letter. I suspect this is a WONTFIX for both clojure and clojurescript, but it is fixable with another regex replacement.)



 Comments   
Comment by Francis Avila [ 12/Jun/14 1:27 AM ]

Forget what I said about Rhino, it's just as broken there. I think I got my repls confused or something.

Comment by David Nolen [ 02/Dec/14 5:36 AM ]

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





[CLJS-842] Recursively check IEncodeClojure in js->clj Created: 13/Aug/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Tim Griesser Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File 0001-recursive-iencodeclojure.diff    
Patch: Code

 Description   

Allows an object fulfilling the IEncodeClojure protocol to be nested inside another plain JS object and used in the js->clj transformation.



 Comments   
Comment by David Nolen [ 02/Dec/14 5:31 AM ]

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





[CLJS-874] Add closure/path-relative-to support to relative paths based on directories Created: 19/Oct/14  Updated: 02/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Andrew Rosa Assignee: Andrew Rosa
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 874.patch    
Patch: Code and Test

 Description   

The referred function could not handle directories as base paths, since it only calculate the relative paths via string manipulation. I propose using some checking on file system (through {File#isDirectory()}) to be able to differentiate directories from files.

The current behavior:

(path-relative-to (io/file ".") {:url (deps/to-url (io/file "cljs/core.js"))})
;; => "core.js"

After the patch we will have:

(path-relative-to (io/file ".") {:url (deps/to-url (io/file "cljs/core.js"))})
;; => "cljs/core.js"

This behavior is needed for my patch for CLJS-851. If there is some better alternative to address that, I'm open to make the appropriate changes.



 Comments   
Comment by David Nolen [ 02/Dec/14 5:10 AM ]

Can we get a rebased patch? Thanks.

Comment by Andrew Rosa [ 02/Dec/14 5:24 AM ]

Of course I can provide it @David. But the current CLJS-851 implementation this behavior is not needed anymore. Do you still want this patch or prefer to wait until CLJS-851 resolution?





[CLJS-841] cljs.closure/build file locks Created: 13/Aug/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: joerupen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Windows 7, 8, sublime text 3, lein-cljsbuild 1.0.3



 Description   

I suspect that the following issue of the lein-cljsbuild (https://github.com/emezeske/lein-cljsbuild/issues/322) is caused by clojurescript itself and not the plugin. In short: Is it possible that the cljs.closure/build function keeps file locks on the source files? This would explain why running `lein cljsbuild auto` prevents editors like sublime to save (overwrite) the files.



 Comments   
Comment by Thomas Heller [ 14/Aug/14 4:01 AM ]

cljs.closure/build does no file locking. The file is opened, read completely and then closed.

https://github.com/clojure/clojurescript/blob/95122e1206d4a64f392cff03bfd73712ab7f3a33/src/clj/cljs/closure.clj#L256

Comment by joerupen [ 18/Aug/14 6:11 AM ]

Okay, but how is possible that when I comment out the call to the build function, then lein-cljsbuild maintains no locks on the source files? Without the call, lein-cljsbuild picks up any modifications that I do to my source files and invokes the build (which in that case just does nothing, see https://github.com/emezeske/lein-cljsbuild/issues/322).

I opened this issue because it's really disturbing the development workflow when you cannot use `lein cljsbuild auto`. If it's not due to ClojureScript you can close this issue.

Comment by David Nolen [ 18/Aug/14 9:41 AM ]

Never heard this before, sounds possibly like a host issue that we might not be aware of?

Comment by joerupen [ 18/Aug/14 4:18 PM ]

It happens on win7/8 with jdk 7. This problem seems related to sublime 3, as with e.g. lighttable I have no problem. You can see that java.exe holds open file handles on the source files (sysinternals process explorer). Closing the handles manually works but is just a workaround. The reason is that sublime is actually doing a file 'move' operation. When you run `lein cljsbuild auto` and try to overwrite a source file from windows explorer you get the same problem, however you can still open & modify the file in various editors. So it's not a real file lock, but a file lock that prevents a file-move operation. I really don't know if it's ClojureScript or the plugin itself.

Comment by Thomas Heller [ 19/Aug/14 6:21 AM ]

Not sure how lein-cljsbuild handles the file watching, maybe that keeps files open?

Try replacing your (spit ...) test with (Thread/sleep 5000) or something that takes time, since spit is probably "too fast". Is it happening then?

Comment by joerupen [ 19/Aug/14 9:47 AM ]

I just tried to intercept the arguments in the call to cljs.closure/build and then invoked the build function in a repl using the same args. There was no lock this time, but also the generated js files were wrong. There might be some thread bindings in lein-cljsbuild that I am missing, but it looks like the build function by itself does not lock. I also replaced the code with Thread/sleep but the effect is the same. Only when I comment out the build function I can save my file. I also tried to run the build function in an agent, but without success, locks keep existing. I'll keep exploring some other potential causes in lein-cljsbuild and let you know if I find something useful.

Comment by joerupen [ 20/Aug/14 4:45 AM ]

By introducing a (Thread/sleep 5000) right after the build function I give the JVM some time to relax right after compiling. The locks seem to disappear (or become very rare). Doesn't that should like a finalizer waiting to be called by the garbage collector? If the polling starts immediatly, the locks occur almost instantly. I know that functions like slurp are pretty safe, so maybe this whole issue is caused by some internals of the JVM itself. Since I don't know all the internals of cljs.closure/build I can't possibly exclude it.

@Thomas: I had a look at the lein-cljsbuild source and from what I understood was that every 100ms,

My conclusion is that lein-cljsbuild does not open any file for reading (at least not in my simple project, no macros, no crossovers, just a simple hello world if you will). It only does directory listings and queries for modified timestamps. Under the hood, querying a file's attributes might very well translate into a call for opening & reading the file and this is done very often. If you have some ideas how I could attempt to locate the root cause, a hint would be appreciated As a workaround I can only recommend to switch to lighttable.

Comment by Thomas Heller [ 20/Aug/14 4:02 PM ]

100ms seems a little overkill to me since no human will make meaningful changes in that time, but that should not matter. A pending finalizer should not be the cause since cljs.closure/build uses with-open which closes the reader when done.

But I use neither Windows, Sublime Text or lein-cljsbuild so I'm just guessing anyways.

If you are feeling adventurous you could try https://github.com/thheller/shadow-build (shameless-plug) but I never tried it on Windows so it might actually blow up or something.

Comment by joerupen [ 22/Aug/14 7:28 AM ]

On https://github.com/emezeske/lein-cljsbuild/issues/322 I have posted a workaround. I basically copy the source cljs files to a temp folder and run `cljs.closure/build` from that folder. Interestingly I can always delete that folder after invoking build. This indicates that this issue only occurs in combination with the lein-cljsbuild plugin (due to the watch behavior). As far as the build function is concerned I'd recommend to close this issue in jira.

Comment by David Nolen [ 02/Dec/14 5:14 AM ]

Not a ClojureScript thing specifically.





[CLJS-877] to-string of js/Date objects is inconsistent Created: 30/Oct/14  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

0.0-2371, Chrome



 Description   

When converting a js/Date object to a string, different things happen depending on whether the object is nested in a data structure or not.

For example:

(def date (js/Date.))
(str date) ;; => "Thu Oct 30 2014 16:49:50 GMT-0400 (EDT)"
(str [date]) ;; => "[#inst \"2014-10-30T20:49:50.999-00:00\"]"


 Comments   
Comment by David Nolen [ 02/Dec/14 5:00 AM ]

The behavior is identical to java.util.Date in Clojure. Not saying it isn't a bug but a discussion should probably be started elsewhere first.





[CLJS-847] TypeError in cljs.core/str using Safari 6.0.5 Created: 29/Aug/14  Updated: 24/Nov/14  Resolved: 31/Aug/14

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

Type: Defect Priority: Major
Reporter: Kevin Neaton Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-847.patch    

 Description   

In some versions of Safari 6 (at least 6.0.4 and 6.0.5) calls to cljs.core/str may throw a very cryptic Type Error: type error. This error has occurred repeatedly in our production cljs app over the last ~3 months but I have not been able to reproduce it locally, or boil it down to a simple test case. This appears to be due to the nature of the bug itself. I was however, able to workaround the issue by patching cljs.core/str to use the '' + x form of coercion instead of calling x.toString directly.

Other js projects have encountered this issue and adopted the same solution:

This shouldn't hurt performance and might actually improve it in some browsers:

I'll submit the patch we are using shortly.



 Comments   
Comment by David Nolen [ 31/Aug/14 9:41 AM ]

thanks for the report this is now fixed in master https://github.com/clojure/clojurescript/commit/08b4b1585cf0ef739e903985ee4c6b7fc6c47642.

Comment by Kevin Neaton [ 31/Aug/14 1:30 PM ]

My pleasure. Thanks for the quick turnaround!

Comment by Nikita Beloglazov [ 23/Nov/14 5:30 PM ]

Seems like using concatenation breaks some cases when valueOf and toString methods are overriden. Example:

var obj = {
    toString: function() { return 'hello'; },
    valueOf: function() { return 42; }
};
console.log(String(obj)); => 'hello'
console.log(obj.toString()); => 'hello'
console.log('' + obj); => '42'

So ClojureScript str function will return '42' for that object, but in fact it should return 'hello'. How about using String() to convert object to strings? I have no idea whether it breaks in Safari or not, may be Safari breaks only on toString() and not String().

Comment by Kevin Neaton [ 23/Nov/14 11:09 PM ]

Since this issue is closed, you might consider creating a new one. That said, I suspect `String` would work as well but I have no way to check, since the original error is so difficult to reproduce.

Comment by David Nolen [ 24/Nov/14 7:27 AM ]

Please open a new issue, thanks.

Comment by Nikita Beloglazov [ 24/Nov/14 9:52 AM ]

Sorry, should have thought about that myself. Done: http://dev.clojure.org/jira/browse/CLJS-890





[CLJS-870] :preamble should place newline between files Created: 09/Oct/14  Updated: 20/Nov/14  Resolved: 20/Nov/14

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

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

Attachments: Text File cljs-870.patch    

 Description   

To catch cases where JS libs don't end in a newline.



 Comments   
Comment by Maksim Soltan [ 18/Nov/14 9:26 AM ]

Fixed how preambles are joined, also updated test to cover this patch.

Comment by David Nolen [ 18/Nov/14 9:31 AM ]

Max this looks good, have you submitted your CA? Thanks.

Comment by Maksim Soltan [ 19/Nov/14 2:11 AM ]

Yes, also I updated my full name in JIRA to match information in CA.

Comment by David Nolen [ 20/Nov/14 11:15 AM ]

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

Comment by Maksim Soltan [ 20/Nov/14 2:10 PM ]

Thanks!





[CLJS-888] Make better use of newlines in emitted javascript Created: 17/Nov/14  Updated: 20/Nov/14  Resolved: 20/Nov/14

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

Type: Enhancement Priority: Minor
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-888-Better-placement-of-newlines-in-emitter.patch     Text File 0002-CLJS-888-Omit-redundant-around-emitted-recur.patch    

 Description   

Problem Statement

Javascript debuggers are pretty line-oriented and so it's often very difficult to follow control flow without source maps. Even with source maps it's often not feasible to properly work with breakpoints in emitted Javascript.

Proposal

Emit more newlines and do so in places that would make sense when hand-coding javascript. The attached patch came from experience in debugging on IE and makes the emitter break up lines pretty neatly, with the exception of deeply nested literals.



 Comments   
Comment by Herwig Hochleitner [ 17/Nov/14 4:11 PM ]

Patch 0002 is not strictly in scope of the ticket title, but fits within an emitter cleanup, so I think it could be reviewed as part of this ticket.

Comment by David Nolen [ 20/Nov/14 11:24 AM ]

fixed
https://github.com/clojure/clojurescript/commit/124998c05dea844e892cd28b40f89eecdd442e1a
https://github.com/clojure/clojurescript/commit/bf2d2413dcb46b2cec9a00e37af407006634c804





[CLJS-873] Non-higher-order calls to cljs.core/array-map sometimes return PHMs, should always return PAMs Created: 14/Oct/14  Updated: 20/Nov/14  Resolved: 20/Nov/14

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

Type: Defect Priority: Major
Reporter: Michał Marczyk Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-873-non-higher-order-calls-to-array-map-should-.patch    

 Description   

Non-higher-order calls to cljs.core/array-map are backed by a macro which emits hash maps for > 8 key-value pairs. However, array-map should always return array maps - that is the contract promised by the docstring (in both Clojure and ClojureScript) and the established behaviour in Clojure.

Example:

ClojureScript:cljs.user> (type (array-map :a true :c true :d false :b true :z false :h false :o true :p true :w false :r true :t false :g false))
cljs.core/PersistentHashMap

The map above comes from this StackOverflow question.



 Comments   
Comment by Michał Marczyk [ 14/Oct/14 12:21 PM ]

(Automatically) rebased on top of current master.

Comment by David Nolen [ 20/Nov/14 11:19 AM ]

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





[CLJS-863] Invalid arity error when calling multimethod with 0-arity dispath fn Created: 24/Sep/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

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

Type: Defect Priority: Minor
Reporter: Immo Heikkinen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Starting from 0.0-2227


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

 Description   

Clojure supports dispatch functions with zero arity, so should ClojureScript.



 Comments   
Comment by Immo Heikkinen [ 13/Nov/14 11:46 PM ]

Is there a reason for not merging this?

(Sorry for the misleading title, of course it should be "Invalid arity error when 0-arity multimethod".)

Comment by David Nolen [ 17/Nov/14 7:53 AM ]

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





[CLJS-886] Add a contains-key? function to the ITransientAssociative protocol Created: 14/Nov/14  Updated: 17/Nov/14

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

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


 Description   

I propose to add a contains-key? function to the ITransientAssociative protocol equivalent to the function with the same name in IAssociative.



 Comments   
Comment by David Nolen [ 17/Nov/14 7:49 AM ]

This is a departure from the interface design in Clojure. I recommend starting a discussion on clojure-dev.





[CLJS-579] Use leiningen to handle clojurescript dependency resolution && classpath string generation Created: 28/Aug/13  Updated: 15/Nov/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-let-lein-handle-classpath-string-generation-dependen.patch    

 Description   

After every bump in one of clojurescript's dependency, it is needed to manually run script/clean && script/bootstrap in order to use the new versions.

By leveraging `lein classpath`, we can automate this process removing the need for any manual intervention.



 Comments   
Comment by Nicola Mometto [ 28/Aug/13 7:45 PM ]

The following patch also removes script/bootstrap script/clean and script/test-compile since those files are no more relevant.

I haven't tested the batch version of the scripts since I don't have windows on my machine, it would be a good idea to have somebody test those before merging.

Comment by David Nolen [ 04/Sep/13 11:07 PM ]

Is there a revised minimal patch? Thanks!

Comment by Nicola Mometto [ 05/Sep/13 6:10 AM ]

http://dev.clojure.org/jira/browse/CLJS-578 only adds project.clj, is this what you're asking for?





[CLJS-883] Add nthrest Created: 07/Nov/14  Updated: 13/Nov/14  Resolved: 13/Nov/14

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

Type: Enhancement Priority: Trivial
Reporter: Ben Moss Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File add_nthrest.patch    
Patch: Code

 Description   

Discovered last night at the Clojurescript meetup that nthrest is missing from CLJS! Found this https://groups.google.com/forum/#!topic/clojurescript/MZQUInUGCRc, but nothing seems to have been done.



 Comments   
Comment by David Nolen [ 13/Nov/14 9:16 AM ]

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





[CLJS-880] With empty collections, specify! extends all instances, not just this one Created: 01/Nov/14  Updated: 13/Nov/14  Resolved: 13/Nov/14

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

Type: Defect Priority: Minor
Reporter: Marcus Lewis Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2371"]



 Description   

Some pretty surprising behavior:

my.ns> (defprotocol IFoo)
my.ns> (satisfies IFoo {})
false
my.ns> (specify! {} IFoo)
my.ns> (satisfies IFoo {})
true

The IFoo specification leaks onto essentially all other equal-valued instances. As a result, one of my functions was overwriting this specification for all existing instances with a single specify!.

Non-empty collections behave as expected:

my.ns> (defprotocol IFoo)
my.ns> (satisfies IFoo {:a "a"})
false
my.ns> (specify! {:a "a"} IFoo)
my.ns> (satisfies IFoo {:a "a"})
false



 Comments   
Comment by David Nolen [ 05/Nov/14 6:11 AM ]

I'm not sure that it's worth making any changes for this. We optimize empty literals and specify! is a lower level operation. specify is should be preferred in nearly every case - the use of specify! seem gratuitous in the above examples. Is there a more compelling argument?

Comment by Marcus Lewis [ 05/Nov/14 11:29 AM ]

Here's the code in question. On a side note, I've since abandoned this function, and use React mixins instead. https://github.com/mrcslws/saccade/blob/45d5ac2f2dd5396b7cf27339e4b436d1bf236e15/src/cljs/saccade/components/helpers.cljs#L11

I worked around the bug via (let [faker (reify)]). If "faker" was initialized to an empty collection, very confusing things happened.

I used specify! because in this case (implementing protocols case-by-case) it was much easier to read. specify is doable, but you'd have to reduce over a vector of functions, one function per protocol, each conditionally returning sofar or (specify sofar IFoo) based on (satisfies? IFoo actual). It's a fun intellectual exercise, but I couldn't find a good non-religious reason to do it.





[CLJS-510] Problems with :optimizations :whitespace & :output-wrapper true Created: 28/May/13  Updated: 13/Nov/14  Resolved: 13/Nov/14

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

Type: Defect Priority: Minor
Reporter: Aku Kotkavuo Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None
Environment:

OS X 10.8.3, Google Chrome 28


Attachments: Text File CLJS-510.patch    

 Description   

When I have both output wrapper and :whitespace optimizations enabled, I get the following error when loading the produced output:

goog.provide("goog.string");
goog.provide("goog.string.Unicode");
goog.string.Unicode = {NBSP:"\u00a0"};
^- Uncaught TypeError: Cannot set property 'Unicode' of undefined

Any ideas on what is going wrong? The error disappears if I use :simple or :advanced optimizations or if I set :output-wrapper to false (combined with any optimization level).



 Comments   
Comment by Aku Kotkavuo [ 28/May/13 10:14 AM ]

ClojureScript version used is 0.0-1806.

Comment by Aku Kotkavuo [ 28/May/13 10:30 AM ]

I'm also seeing this:

GET http://localhost:7070/deps.js 404 (Not Found)
goog.writeScriptTag_
goog.importScript_
goog.importScript_(goog.basePath + "deps.js")

I have no idea what deps.js is, something related to Google Closure Compiler?

Comment by David Nolen [ 19/Nov/13 9:23 PM ]

Is this problem still present? Thanks.

Comment by Immo Heikkinen [ 11/Mar/14 3:59 AM ]

The problem is still present in clojurescript master. To reproduce, compile https://github.com/clojure/clojurescript/tree/master/samples/hello using

cljsc src {:output-wrapper true :optimizations :whitespace} > hello.js
Comment by Herwig Hochleitner [ 13/Oct/14 5:28 PM ]

I just saw this with [org.clojure/clojurescript "0.0-2371"]

The last line of the following, throws the error, when compiled with :output-wrapper

goog.provide("goog.string");
goog.provide("goog.string.Unicode");
goog.define("goog.string.DETECT_DOUBLE_ESCAPING", false);
goog.string.Unicode = {NBSP:"\u00a0"};

Notice how if fails right after providing goog.string, so it might be a bug with google closure.

Comment by Immo Heikkinen [ 14/Oct/14 12:10 AM ]

I noticed that if you move

var goog = goog || {};

outside the wrapper function, the problem goes away. Looks like tt's some kind of JavaScript scope issue.

Comment by David Nolen [ 14/Oct/14 5:22 AM ]

I suspect that the whitespace and output wrapper combination is not compatible. I'm inclined to close this one as it's a Closure limitation not a ClojureScript one. I would be open to an error when these two options are combined.

Comment by Immo Heikkinen [ 27/Oct/14 6:06 AM ]

Patch for throwing error when :output-wrapper and :optimizations :whitespace are combined

Comment by David Nolen [ 31/Oct/14 2:51 AM ]

Before proceeding this ticket needs more information. I would like confirmation that it is actually a Closure limitation first. The easiest way to confirm is to use Google Closure directly on some simple JavaScript.

Comment by Immo Heikkinen [ 11/Nov/14 4:51 AM ]

With the following JS file:

$ cat hello.js
goog.provide("hello");
goog.require("goog.string");

alert("Hello!");

the following compile command (using the same versions of closure-library and closure-compiler as clojurescript master)

$ java -jar closure-compiler/build/compiler.jar --js closure-library/closure/goog/base.js --js closure-library/closure/goog/string/string.js --js hello.js --compilation_level=WHITESPACE_ONLY --formatting=PRETTY_PRINT --output_wrapper=';(function(){%output%})();' --js_output_file=hello-compiled.js

produces a file the fails in the same way.

If I remove goog.string dependency, the compiled JS file works fine. If I change it to something else (e.g. goog.object), it gives a similar kind of error.

Comment by David Nolen [ 13/Nov/14 9:08 AM ]

Thank you for the confirmation!

Comment by David Nolen [ 13/Nov/14 9:11 AM ]

fixed https://github.com/clojure/clojurescript/commit/805348d55c987493709c89a5910d679d56cf47e4





[CLJS-884] with-redefs don't restores previous values when used inside go block Created: 09/Nov/14  Updated: 09/Nov/14

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

Type: Defect Priority: Trivial
Reporter: Vladimir Iakovlev Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojure 1.6, clojurescript 0.0-2371, core.async 0.1.346.0-17112a-alpha


Attachments: Text File cljs_884.patch    

 Description   

For example I have code and test:

(defn test-fn [] :original)

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))

And test failed in last `is`:

FAIL in (test-with-redefs-async) (:)
expected: (= (test-fn) :original)
  actual: (not (= :mocked :original))


 Comments   
Comment by Vladimir Iakovlev [ 09/Nov/14 1:26 PM ]

I attached little patch with fix. With it `with-redefs` forced to use previous value for restoring bindings.

But I don't know how to add test for this issue, because it requires core.async. So I tested with:

(defn test-fn [] :original)

(deftest test-with-redefs-sync
  (is (= (test-fn) :original))
  (with-redefs [test-fn (fn [] :mocked)]
    (is (= (test-fn) :mocked)))
  (is (= (test-fn) :original)))

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))




[CLJS-27] Conditional compilation (or reading) Created: 22/Jul/11  Updated: 07/Nov/14

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

Type: Enhancement Priority: Major
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Unresolved Votes: 12
Labels: None

Attachments: File cljs-27.diff     File cljs-27-v2.diff     File cljs-27-v3.diff     File conditional-compilation-clojure.diff     File conditional-compilation-clojurescript.diff    
Patch: Code and Test
Approval: Vetted

 Description   

As people start trying to write libs that are portable between Clojure and ClojureScript they might need to have a bit of branching on target. N.B. supporting this means a change to Clojure, although it has general utility there as well.

Consider CL #+ #- reader macros - http://www.lispworks.com/documentation/lw50/CLHS/Body/02_dhq.htm

Patch: cljs-27-v3.diff

Related: CLJ-1424, TRDR-14



 Comments   
Comment by Roman Scherer [ 19/Jul/12 8:52 AM ]

The following patches include an implementation of Common Lisp's #+
and #- reader macros to allow conditional compilation/reading for
Clojure and ClojureScript.

The patches add a dynamic variable called features to the
clojure.core and cljs.core namespaces, that should contain the
supported features of the platform in question as keywords.

Unlike in Common Lisp, the variable is a Clojure set and not a list.
In Clojure the set contains at the moment the :clojure keyword, and in
ClojureScript the :clojurescript keyword.

I would like to get feedback on the names that are added to this
variable. Are those ok? Is :jvm for Clojure and :js for ClojureScript
better? Should ClojureScript add something like :rhino, :v8 or
:browser as well?

To run the ClojureScript tests, drop a JAR named "clojure.jar" that
has the Clojure patch applied into ClojureScript's lib directory.

Comment by David Nolen [ 19/Jul/12 12:18 PM ]

This is an enhancement so it probably requires a design page and extended discussion before it will go anywhere. Until that happens I'm marking this is as low priority.

Comment by Roman Scherer [ 19/Jul/12 1:45 PM ]

Ok. If someone could give me write access to the Clojure Dev Wiki I would be happy to start such a design page.

Comment by David Nolen [ 19/Jul/12 5:50 PM ]

If you've sent in your CA request permissions on the clojure-dev mailing list.

Comment by Roman Scherer [ 21/Jul/12 5:45 AM ]

I started a design page for this ticket in the Clojure Dev wiki:
http://dev.clojure.org/display/design/Feature+Expressions

Comment by Stuart Halloway [ 27/Jul/12 1:48 PM ]

Posting my comments over on the design page...

Comment by Alex Miller [ 06/Aug/14 7:42 AM ]

Latest patch updates into current ClojureScript and use of tools.reader/tools.analyzer etc. The reader changes are all in the accompanying tools.reader patch in TRDR-14. This patch adds support to allow a new option "features" which is expected to be a set of keywords. build will inject :cljs into this set. The feature set is maintained in clojure.tools.reader/*features*. set! is enhanced to special-case an update to *features* in the code (presumably a rarely-used feature).

Because tools.reader needs the supporting patch, I left in several changes that pull in a new version of tools.reader - I don't actually expect those to be the correct versions but they are there as a reminder to update all of the proper places.

Comment by Alex Miller [ 11/Sep/14 9:04 AM ]

cljs-27-v2.diff adds ability to load .clj files as well as .cljs files when compiling.

Comment by Alex Miller [ 07/Nov/14 10:55 AM ]

Fresh patch, switch to take .cljc files.





[CLJS-876] merged sourcemap doesn't account for output-wrapper Created: 25/Oct/14  Updated: 05/Nov/14

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

Type: Defect Priority: Major
Reporter: J. David Lowe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 876.alternative.patch     Text File 876.patch    

 Description   

When merging sourcemaps, the cljs.closure/optimize code correctly accounts for "preamble" lines that get prepended to closure compiler output. However, it fails to account for the extra lines added when :output-wrapper is true. As a result, the merged sourcemap is incorrect whenever :output-wrapper is true. (And for extra action-at-a-distance: :output-wrapper is implicitly set to true by lein-cljsbuild whenever :optimizations is :advanced.)

Too tired to make a patch tonight, but I'll go through the CA process and send a patch in a day or two, if nobody gets to it before then.



 Comments   
Comment by J. David Lowe [ 27/Oct/14 2:42 PM ]

Here are two patches, which both fix the problem.

876.patch is tiny, but I think it's probably a step in the wrong direction, because all future output prefixes will need the same fix applied to them.

876.alternative.patch is a deeper change, but I think is a better solution: it makes it much more apparent that all output prefixes should be added via make-preamble. The downside (IMO) is that the output-wrapper prefix and suffix are now separated a bit in the code.

Comment by David Nolen [ 05/Nov/14 6:46 AM ]

This patch approach looks OK but something more minimal is preferred to simplify patch reviewing. Please submit a patch which makes the most minimal set of code changes (it's preferred not to move code) use declare if necessary. Thanks!





[CLJS-875] Compiler error on :" Created: 22/Oct/14  Updated: 05/Nov/14  Resolved: 05/Nov/14

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

Type: Defect Priority: Major
Reporter: Stuart Mitchell Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: compiler, errormsgs
Environment:

Ubuntu 14.04
org.clojure/clojurescript "0.0-2277"



 Description   

(str "this will break" :"the parser")
This simple form (note the misplaced ':' causes the compiler to barf with the following error
Caused by: clojure.lang.ExceptionInfo: EOF while reading string

no mention of the ':' and in larger blocks of code the unmatched brackets several lines away is flagged

in clojure the error is
RuntimeException Invalid token: : clojure.lang.Util.runtimeException (Util.java:221)
RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:221)

Note it mentions the errant ':'



 Comments   
Comment by Stuart Mitchell [ 22/Oct/14 11:04 PM ]

I would like the error reporting from the compiler improved in this case, so that it mentions that the problem is caused by the ':'.

Comment by Nicola Mometto [ 23/Oct/14 7:53 AM ]

This appears to be an issue caused by tools.reader, I'm working on a fix for it

Comment by Nicola Mometto [ 23/Oct/14 9:17 AM ]

I just pushed a 0.8.10 release of tools.reader that fixes this issue

Comment by David Nolen [ 05/Nov/14 6:33 AM ]

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





[CLJS-881] array-map does not check for duplicate keys Created: 01/Nov/14  Updated: 05/Nov/14  Resolved: 05/Nov/14

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

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-881-p1.patch    

 Description   

array-map has a macro version that works correctly, but the function version will happily create a map with duplicate keys:

(keys (array-map :foo 1 :foo 2)) ;; => (:foo)
(keys (apply array-map [:foo 1 :foo 2])) ;; => (:foo :foo)


 Comments   
Comment by Gary Fredericks [ 01/Nov/14 9:09 PM ]

Attached CLJS-881-p1.patch, which contains a test and switches array-map to use the .fromArray method the same way the macro does.

Comment by David Nolen [ 05/Nov/14 6:31 AM ]

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





[CLJS-879] Add function `update` from Clojure 1.7 Created: 31/Oct/14  Updated: 31/Oct/14  Resolved: 31/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 31/Oct/14 2:48 AM ]

fixed https://github.com/clojure/clojurescript/commit/15fbbf5d63fbc860e0fe4f7d45c7f00a27ebc0ba

Comment by Daniel Skarda [ 31/Oct/14 3:03 AM ]

David, thanks for being so quick!

You are faster than my `git format-patch` Are not you supposed to be on tour with Hairy Sands?

Thank you for fast fix.





[CLJS-710] port clojure.pprint Created: 02/Dec/13  Updated: 27/Oct/14

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

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


 Comments   
Comment by Shaun LeBron [ 15/Aug/14 1:29 PM ]

working on this here: https://github.com/shaunlebron/cljs-pprint

Comment by Shaun LeBron [ 05/Oct/14 12:40 PM ]

I'm ceasing development on the port.

fipp should be ported in place of pprint in cljs. pprint is slow and complex, whereas fipp is fast, simple, and very customizable.

Brandon Bloom is awaiting a go-ahead from the cljs community on whether the fipp port should be completed:
https://github.com/brandonbloom/fipp/issues/7

Comment by David Nolen [ 05/Oct/14 12:55 PM ]

fipp only covers a very small (though important part) of clojure.pprint's functionality. I think it's fine that fipp is a standalone library that user's can adopt in their own projects if they desire. This doesn't change the desire for a full port of clojure.pprint.

Comment by Shaun LeBron [ 27/Oct/14 1:13 PM ]

k, resuming.





[CLJS-675] QuickStart example not working properly Created: 10/Nov/13  Updated: 15/Oct/14

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

Type: Defect Priority: Minor
Reporter: Anton Logvinenko Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

To reproduce only have to follow simple steps as described in https://github.com/clojure/clojurescript/wiki/Quick-Start
git clone git://github.com/clojure/clojurescript.git
cd clojurescript
./script/bootstrap


Attachments: Text File CLJS-675.patch    

 Description   

Section "Using ClojureScript on a Web Page"
Executing:

./bin/cljsc hello.cljs '{:optimizations :advanced}' > hello.js

produces error message "java.io.FileNotFoundException: out/constants_table.js (No such file or directory)"

Just creating "out" directory seems to be fixing the problem. The directory is then deleted after the compilation.

mkdir out
./bin/cljsc hello.cljs '{:optimizations :advanced}' > hello.js

Also, "out" directory is mentioned in .gitignore.



 Comments   
Comment by Anton Logvinenko [ 10/Nov/13 5:28 AM ]

UPD: The "out" directory is not deleted after the compilation. It's simply ignored by git.

Comment by Steven Kallstrom [ 15/Oct/14 4:05 PM ]

I encountered the same issue. Patch submitted.





[CLJS-857] change deftype*/defrecord* special forms to include their inline methods decls Created: 15/Sep/14  Updated: 15/Oct/14  Resolved: 15/Oct/14

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

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

Attachments: Text File 0001-change-deftype-defrecord-special-forms-to-include-th.patch     Text File 0001-CLJS-857-change-deftype-defrecord-special-forms-to-i.patch    
Patch: Code

 Description   

Currently the deftype* and defrecord* special forms don't include their inline method declarations, this means that code like:

(deftype foo [x] Object (toString [_] x))

is macroexpanded into something that looks like:

(deftype* foo [x] pmask) (extend-type foo Object (toString [_] x)) ...

The issue with this approach is that we want to treat "x" in the extend-type expression as if it was inside the lexical scope of the deftype*, to do so the analyzer attaches a metadata flag to the extend-type call with the deftype* local fields.

This is not ideal and complicates needlessly the analyzer, I propose to simply move the extend-type call as a parameter to the deftype* special form, this way no special handling for local fields will be needed:

(deftype foo [x] pmask (extend-type foo Object (toString [_] x)))

since the extend-type code is effectively inside the lexical scope of the deftype.

Everything said above applies to defrecord* aswell, this patch implements what I proposed and refactors a bit the code in the analyzer to remove the code duplication in the defrecord* and deftype* parse implementations.

In addition, the current approach was unfeasable to implement for tools.analyzer.js so taking this patch would mean making cljs.analyzer and tools.analyzer.js special-form compatible.



 Comments   
Comment by Nicola Mometto [ 15/Oct/14 9:50 AM ]

Updated patch no longer causes warnings at compile time

Comment by David Nolen [ 15/Oct/14 11:23 AM ]

fixed https://github.com/clojure/clojurescript/commit/9a7f9e8d28a5c57778c821c3816b8dd668328110





[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 14/Oct/14

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 871.patch    
Patch: Code and Test

 Description   

Types defined with deftype/defrecord which have a default field will incorrectly return nil with property access. The following example will return nil.

(deftype Foo [default])

(let [foo (Foo. "bar")]
  (.-default foo))


 Comments   
Comment by Joel Holdbrooks [ 13/Oct/14 4:19 PM ]

Patch attached. I should point out that I had to borrow js-reserved from the compiler namespace and the warning message provided hard codes the munged symbol information instead of reusing the compiler's munge fn.

Comment by Joel Holdbrooks [ 13/Oct/14 9:41 PM ]

For the sake of history, I should provide more context to this patch (I'm unable to edit the issue title for some reason). It isn't just .-default it is any field name that is also a JavaScript identifier (eg. public, private, if).

Comment by David Nolen [ 14/Oct/14 5:26 AM ]

Please lift js-reserved and any helpers like munge into the shared namespace cljs.util so that logic an be shared and hard coding avoided. Thanks.

Comment by Joel Holdbrooks [ 14/Oct/14 5:03 PM ]

Are you sure, David? That might make this patch a bit more noisy. If it's not a problem I'm happy to do it.

Comment by David Nolen [ 14/Oct/14 6:06 PM ]

I'm sure, I'd like to avoid this kind of code duping. Cleaner in the end and better moving forward.





[CLJS-872] Ordered collections printed as if they were unordered at the REPL Created: 13/Oct/14  Updated: 14/Oct/14

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

Type: Defect Priority: Trivial
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is due to cljs.repl's read-then-print processing of string representations of return values that come back from the JS env. As of release 2371, the relevant code fragment lives here:

https://github.com/clojure/clojurescript/blob/r2371/src/clj/cljs/repl.clj#L156

A larger snippet to demonstrate this:

ClojureScript:cljs.user> (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14)
{1 2, 3 4, 5 6, 7 8, 9 10, 11 12, 13 14}
ClojureScript:cljs.user> (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)
{7 8, 1 2, 15 16, 13 14, 17 18, 3 4, 11 12, 9 10, 5 6}
ClojureScript:cljs.user> (seq (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18))
([1 2] [3 4] [5 6] [7 8] [9 10] [11 12] [13 14] [15 16] [17 18])
ClojureScript:cljs.user> (hash-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)
{7 8, 1 2, 15 16, 13 14, 17 18, 3 4, 11 12, 9 10, 5 6}

This issue seems to be the most likely cause of the problem described in this StackOverflow question. It would be nice to print ordered collections in the "expected" way to prevent user confusion.



 Comments   
Comment by David Nolen [ 14/Oct/14 5:23 AM ]

How is this handled in Clojure?

Comment by Michał Marczyk [ 14/Oct/14 7:40 AM ]

The built-in REPL simply prints string representations of values to *out* without attempting to read them back in first.

Comment by David Nolen [ 14/Oct/14 7:54 AM ]

Well the result of REPL evaluation is a string unlike Clojure, in order to get a proper print at the ClojureScript REPL it seems necessary to read back the result. I will say it's not clear to me array-map promises anything about order anyway.

Comment by Michał Marczyk [ 14/Oct/14 8:02 AM ]

It does – see http://clojure.org/data_structures, where it says

ArrayMaps
When doing code form manipulation it is often desirable to have a map which maintains key order. An array map is such a map - it is simply implemented as an array of key val key val... As such, it has linear lookup performance, and is only suitable for very small maps. It implements the full map interface. New ArrayMaps can be created with the array-map function. Note that an array map will only maintain sort order when un-'modified'. Subsequent assoc-ing will eventually cause it to 'become' a hash-map.

This snippet has been there for as long as I can remember; and this particular feature comes up in various online discussions from time to time. The docstrings for array-map (the core function) are unambiguous in their promise to construct array maps in both Clojure and ClojureScript.

As for the issue at hand, I do think it's a very minor one (hence the "Trivial" priority), but it is real – the representation printed at the REPL is out of line with the string returned from pr-str on the same value. In fact, one way to fix this would be to use pr-str representations computed on the CLJS side. (There may be some complications here, though, which we'd have to work out.)

Comment by David Nolen [ 14/Oct/14 8:07 AM ]

Thanks for pointing out the API promise. OK, yeah I don't really have a good idea for how do this but a patch that isn't too ugly is welcome





[CLJS-864] ClojureScript's 'and' does not always short-circuit Created: 24/Sep/14  Updated: 10/Oct/14  Resolved: 09/Oct/14

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

Type: Defect Priority: Major
Reporter: Zachary Kanfer Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

my dependencies from my project.clj:

:dependencies [[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2342"]
[org.clojure/core.async "0.1.319.0-6b1aca-alpha"]]

I'm building on Linux 3.13.0-35



 Description   

I have a section of code: and false (function-that-should-not-be-called). Now, we should never call the function inside the and, but I'm seeing behavior where sometimes we are. It seems to depend on what's going on inside the definition of {function-that-should-not-be-called}. If we define a function this way, everything is fine:

(defn implicit-return-value
  []
  (.log js/console "implicit-return-value called")
  (dom/setTextContent (dom/getElement "executed")
                      "We called (implicit-return-value)"))

When I say "everything is fine", I mean the function is not called when this code is executed: (and false (implicit-return-value).

On the other hand, if we define a function this way, everything is not fine:

(defn explicit-return-value
  []
  (.log js/console "explicit-true-return called.")
  (dom/setTextContent (dom/getElement "executed")
                      "We called (explicit-return-value)")
  true)

When we execute this code: (and false (explicit-return-value), we do call the function explicit-return-value.

I have made a minimal project using the latest ClojureScript. The output of that project is located on my webpage. The project itself is on bitbucket. I'm not able to reproduce in a ClojureScript repl, and I haven't played around with optimizations to see when it does and does not happen.



 Comments   
Comment by Francis Avila [ 25/Sep/14 2:48 AM ]

Most likely the problem is in core.async's go macro, not clojurescript. You should bring this issue up with them.

Other avenues of investigation:

  • Are go blocks required to trigger the problem? (Seems so from what is said about repl.)
  • Does similar code (and inside go block with compile-time-known return value) exhibit the same behavior in Clojure?
Comment by Francis Avila [ 25/Sep/14 2:51 AM ]

Indeed, it seems the same issue has a ticket in core.async JIRA: ASYNC-91

Comment by David Nolen [ 09/Oct/14 3:35 PM ]

This is a core.async issue

Comment by Zachary Kanfer [ 10/Oct/14 1:20 AM ]

Thanks for the suggestions to help narrow down where the problem is. I'll see if I can figure out any more information, and update the other ticket.





[CLJS-866] Faulty ns macro desugaring Created: 01/Oct/14  Updated: 10/Oct/14  Resolved: 09/Oct/14

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

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2356, Clojure 1.7.2-alpha2 (but I believe this has been around for some time)


Attachments: Text File CLJS-866.patch    

 Description   

ns desugaring for `:include-macros`, `:refer-macros` seems to be faulty.

Have posted a Gist showing the behaviour: https://gist.github.com/ptaoussanis/e599719d5fb6828a31b1

;; desugar-ns-specs from clojurescript/src/clj/cljs/analyzer.clj
;; (`cljs.analyzer` ns)
 
(desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true])])
;; =>
((:require-macros (foo.bar :as bar))
 (:require        (foo.bar :as bar))) ; Correct
 
(desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
;; =>
((:require-macros (foo.bar :as bar :refer [baz]))
 (:require        (foo.bar :as bar))) ; Seems off, but what's the intended behaviour?
 
(desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
;; =>
((:require-macros (foo.bar :as bar :refer [baz]))
 (:require        (foo.bar :as bar :refer [baz])))
 
;; Is the position of `:include-macros true` supposed to influence expansion
;; like this?
 
;; And is the interaction between `:include-macros true` and `:refer` as
;; intended? I would have expected something like this:
(desugar-ns-specs '[(:require [foo.bar :as bar :include-macros true :refer [baz]])])
;; =
(desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :include-macros true])])
;; =>
((:require-macros (foo.bar :as bar)) ; No :refer
 (:require        (foo.bar :as bar :refer [baz])))
 
;;; --------------------------------------------------------------------------
 
;; There's an additional problem with `:refer` + `:refer-macros`:
 
(desugar-ns-specs '[(:require [foo.bar :as bar :refer [baz] :refer-macros [qux]])])
;; =>
((:require-macros (foo.bar :as bar :refer [baz] :refer [qux])) ; Faulty
 (:require        (foo.bar :as bar :refer [baz])))
 
;; I believe the correct expansion should be:
((:require-macros (foo.bar :as bar :refer [qux]))
 (:require        (foo.bar :as bar :refer [baz])))

So to recap, there seems to be two issues:
1. `:refer-macros` is producing an invalid expansion.
2. `:include-macros true` is position-sensitive, and is dragging `:refer`s along into `:require-macros`. I suspect that this may not be the intended behaviour?

Would be happy to provide a patch if you can confirm that the alternative behaviour I'm suggesting in the Gist is correct.

Thanks, cheers!



 Comments   
Comment by Shaun LeBron [ 02/Oct/14 1:01 AM ]

I wasn't able to use both 'refer' and 'refer-macros' in the same spec due to the same problems.

Here is the patch with your test case results:
https://gist.github.com/shaunlebron/43f79cab0d8705a6352e

Comment by David Nolen [ 02/Oct/14 6:39 PM ]

Feedback about the patch appreciated! Can we get the patch attached to the ticket? Thanks!

Comment by Peter Taoussanis [ 03/Oct/14 1:41 AM ]

I can confirm that Shaun's patch works (thanks Shaun!).

Have attached a simplified version of his patch that I think is a little clearer: https://gist.github.com/ptaoussanis/e599719d5fb6828a31b1#file-cljs-866-patch

Comment by David Nolen [ 09/Oct/14 3:30 PM ]

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

Comment by Peter Taoussanis [ 10/Oct/14 12:01 AM ]

Much appreciated, cheers!





[CLJS-862] re-pattern is inconsistent with Clojure Created: 19/Sep/14  Updated: 09/Oct/14  Resolved: 09/Oct/14

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File 862.patch    
Patch: Code and Test

 Description   

(re-pattern #"foo") fails with "TypeError: re-find must match against a string." This is because, unlike Clojure, there is no check to see if the argument, s, is a RegExp first.



 Comments   
Comment by Joel Holdbrooks [ 19/Sep/14 4:08 PM ]

Patch attached.

Comment by Timothy Pratley [ 08/Oct/14 10:45 PM ]

Tested this out, it applied cleanly with:
patch -p1 < 862.patch
And works as advertised.
I recommend merging it.

Comment by Timothy Pratley [ 08/Oct/14 10:50 PM ]

opps I clicked the 'assign to me' by accident, and can't assign it back to Joel (he doesn't appear in the drop down). In any case I think this patch is pretty uncontroversial.

Comment by David Nolen [ 09/Oct/14 3:33 PM ]

fixed https://github.com/clojure/clojurescript/commit/532a04cdfb4fcdf9eedc37d4d423fef2f170860f





[CLJS-742] Compilation with :output-file option set fails Created: 03/Jan/14  Updated: 08/Oct/14

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

Type: Defect Priority: Trivial
Reporter: Timothy Pratley Assignee: Timothy Pratley
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJS-742.diff    
Patch: Code

 Description   

./bin/cljsc hello.cljs '{:output-file "foo.js"}'

Expected:
a file called foo.js

Actual:
Compiler throws an internal error

Notes:
The correct option is :output-to
:output-file might be removable (it is broken, nobody is using it?)
Further analysis required to determine if it is useful to keep.



 Comments   
Comment by Timothy Pratley [ 03/Jan/14 12:14 PM ]

Fixes handling of compiling one IJavaScript or many

Comment by Timothy Pratley [ 08/Oct/14 11:03 PM ]

Patch is attached, updating ticket to reflect that





[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 03/Oct/14

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

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


 Description   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.






[CLJS-867] specify with Object methods requires multi-arity style definition Created: 02/Oct/14  Updated: 02/Oct/14

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

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


 Description   
Object
(foo [] ...)

Throws an `nth not supported on this type: Symbol` exception.

Object
(foo ([] ...))

Is required even though this is not true for protocol implementations.






[CLJS-861] In Clojurescript IE8 cljs.reader can't read numbers Created: 18/Sep/14  Updated: 26/Sep/14  Resolved: 26/Sep/14

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

Type: Defect Priority: Major
Reporter: Zubair Quraishi Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: patch
Environment:

IE8 Web Browser (used in my company)


Attachments: Text File cljs_CLJS-854.patch     PDF File Clojure_CA.pdf     File reader.cljs    
Patch: Code

 Description   

This change is made because IE8 always returns 0 when cljs.reader reads an integer.

This is caused because the matching pattern in most browsers returns nil for unmatched patterns but IE8 returns "" instead of nil



 Comments   
Comment by Zubair Quraishi [ 18/Sep/14 10:33 PM ]

https://github.com/clojure/clojurescript/pull/39#issuecomment-56069471

https://github.com/zubairq/clojurescript/commit/7ec0144c53b47a831f8696f9eaf97cbcfe06b2e7

https://github.com/JulianBirch/cljs-ajax/issues/46

Comment by David Nolen [ 19/Sep/14 11:25 AM ]

Please e-sign the CA here: http://clojure.org/contributing, thanks!

Comment by Zubair Quraishi [ 19/Sep/14 12:14 PM ]

Ok, I e-signed it and I got back a PDF. Do I have to send the PDF or attach it anywhere?

Comment by David Nolen [ 19/Sep/14 3:16 PM ]

You are now listed as a contributor. I need a properly formatted patch please follow these instructions: https://github.com/clojure/clojurescript/wiki/Patches. Thanks.

Comment by Zubair Quraishi [ 20/Sep/14 6:26 AM ]

I'm not sure what I have to do different from the patch I already submitted? I made a branch, ie8-fix-cljs-reader and made the change to:

https://github.com/zubairq/clojurescript/blob/ie8-fix-cljs-reader/src/cljs/cljs/reader.cljs

https://github.com/zubairq/clojurescript/commit/7ec0144c53b47a831f8696f9eaf97cbcfe06b2e7

I used "CLJS-NNN: TICKET TITLE" as the title for the ticket, "CLJS-854: Fix for IE8 cljs.reader to read integers correctly"

and then I did "git format-patch master --stdout > cljs_ticket_number.patch" before I pushed the change, with "git format-patch master --stdout > cljs_CLJS-854.patch".

Am I missing a step or have a made a mistake, or do I just need to do the same thing again?

Comment by Francis Avila [ 20/Sep/14 3:15 PM ]

You never actually attached the patch file to this ticket.

Comment by Zubair Quraishi [ 22/Sep/14 2:08 AM ]

Ok, sorry, I just added the source file. What is the file extension of the patch file? (My first patch ever to a project so I am new to this - I am not a developer anymore so please forgive me not knowing this as I may be a bit out of date in my knowledge)

Comment by Francis Avila [ 22/Sep/14 7:14 AM ]

The file you created with git format-patch is the file you must attach. That is the only reason that command is run. From what you have written here the file is probably called {{cljs_CLJS-854.patch}}. Pushing your change anywhere is unnecessary--all we need is the patch file.

Comment by Zubair Quraishi [ 23/Sep/14 9:35 AM ]

Ok, I have attached the patch now

Comment by David Nolen [ 26/Sep/14 1:46 AM ]

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





[CLJS-854] In Clojurescript IE8 cljs.reader can't read numbers Created: 10/Sep/14  Updated: 26/Sep/14  Resolved: 26/Sep/14

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

Type: Defect Priority: Major
Reporter: Zubair Quraishi Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None


 Description   

In Chrome and Firefox:

(-> "{:s \"string\", :l 42, :k :keyword, :d 1.337}" (cljs.reader/read-string) (prn-str))

returns

{:s "string", :d 1.337, :k :keyword, :l 42}

:but in IE8 it returns:

{:s "string", :d 1.337, :k :keyword, :l 0}

See this thread:

https://github.com/JulianBirch/cljs-ajax/issues/46



 Comments   
Comment by Thomas Heller [ 10/Sep/14 7:37 AM ]

FWIW I just tested this on IE8 via the console: cljs.reader.read_string("42") => 0

Any Integer seems to fail.

Comment by Zubair Quraishi [ 10/Sep/14 7:58 AM ]

What do I do now with this JIRA issue to find out if it can be resolved? Do I need to assign it to someone, or am I supposed to fix it myself, I'm not really sure.

Comment by Nicola Mometto [ 10/Sep/14 12:12 PM ]

cljs.reader has no relation whatsoever with tools.reader, the title is misleading

Comment by Thomas Heller [ 10/Sep/14 12:30 PM ]

@Zubair: you could attempt a fix, I guess the error is somewhere in here:

https://github.com/clojure/clojurescript/blob/master/src/cljs/cljs/reader.cljs#L112

Too busy at the moment to take a closer look myself.

Comment by Francis Avila [ 10/Sep/14 1:09 PM ]

Actually I'd start with the regex that matches int. It has a non-capturing group in it which historically has had some cross-browser inconsistencies. See if that regex by itself matches int literals in IE or not.

Unfortunately I don't have any IEs to test.

Comment by Zubair Quraishi [ 10/Sep/14 2:47 PM ]

Ok, thanks, I could do that. Do you know if there is a guide which can show me how to do this. I saw the official guide here:

https://github.com/clojure/clojurescript/wiki/Windows-Setup

But it seems to use Clojure 1.3. Is this still the way to set it up on Windows?

Comment by Zubair Quraishi [ 10/Sep/14 2:48 PM ]

Or do I just set up and amend clojure reader on its own?

Comment by Zubair Quraishi [ 10/Sep/14 3:11 PM ]

OK, I'm looking in the reader now, so did you mean the line:

(def int-pattern (re-pattern "^([-+]?)(?0)|([1-9][0-9]*)|0[xX]([0-9A-Fa-f]+)|0([0-7]+)|([1-9][0-9]?)[rR]([0-9A-Za-z]+))(N)?$"))

Comment by Zubair Quraishi [ 16/Sep/14 2:51 AM ]

I am still looking at this and trying to find the error cases. Even 0 on its own fails in IE8. See:

http://connecttous.co/connecttous/connecttous.html

: and click on login to see how your browser parses different numbers:

I am still trying to track down the problem area, and currently looking around here:

(defn read-number
  [reader     initch]

  (loop [
         buffer   (gstring/StringBuffer. initch)
         ch       (read-char reader)
         ]
    
    (if (or (nil? ch) (whitespace? ch) (macros ch))
      
      (do
        (unread reader ch)
        (let [s (.toString buffer)]
          (or (match-number s)
              (reader-error reader "Invalid number format [" s "]"))))
      
      (recur (do (.append buffer ch) buffer) (read-char reader)))))
Comment by Zubair Quraishi [ 17/Sep/14 3:17 AM ]

Changing:

(defn- match-int
  [s]
  (let [groups (re-matches* int-pattern s)
        zero (aget groups 2)]
    (if-not (nil? zero)
      0
      (let [a (cond
               (aget groups 3) (array (aget groups 3) 10)
               (aget groups 4) (array (aget groups 4) 16)
               (aget groups 5) (array (aget groups 5) 8)
               (aget groups 6) (array (aget groups 7)
                                      (js/parseInt (aget groups 6) 10))
               :else (array nil nil))
            n (aget a 0)
            radix (aget a 1)]
        (when-not (nil? n)
          (let [parsed (js/parseInt n radix)]
            (if (identical? "-" (aget groups 1))
              (- parsed)
              s)))))))
              ;parsed)))))))

:gives the following result in Chrome:

Parsing {:s "string", :l 42, :k :keyword, :d 1.337}
{:s "string", :l "42", :k :keyword, :d 1.337}
Parsing {:l 42}
{:l "42"}
Parsing 42
"42"

:gives the following result in IE8:

Parsing {:s "string", :l 42, :k :keyword, :d 1.337}
{:s "string", :l 0, :k :keyword, :d 1.337}
Parsing {:l 42}
{:l 0}
Parsing 42
0

:which means that "(defn- match-int [s]" is being passed 0 in IE8. This is called from here:

(defn- match-number
  [s]
  (cond
   (re-matches* int-pattern s) (match-int s)
   (re-matches* ratio-pattern s) (match-ratio s)
   (re-matches* float-pattern s) (match-float s)))

:which means that defn read-number is passing in the wrong text:

(defn read-number
  [reader  initch]
  (loop [buffer (gstring/StringBuffer. initch)
         ch (read-char reader)]
    (if (or (nil? ch) (whitespace? ch) (macros ch))
      (do
        (unread reader ch)
        (let [s (.toString buffer)]
          (or (match-number s)
              (reader-error reader "Invalid number format [" s "]"))))
      (recur (do (.append buffer ch) buffer) (read-char reader)))))

: So more investigation needed by me

Comment by Thomas Heller [ 17/Sep/14 4:50 AM ]

My guess would be that this check "if-not (nil? zero)" fails, terminates early and never actually gets to the parsing bits.

Comment by Zubair Quraishi [ 17/Sep/14 5:13 AM ]

I first thought it may be

if-not (nil? zero)

failing, but you can see that I changed match-int to return the input

s)))))))
;parsed)))))))

So this means that before

if-not (nil? zero)
is even called, match-int is getting the wrong input

Does that make sense, or have I missed something?

Comment by Thomas Heller [ 17/Sep/14 6:00 AM ]

The branch that returns the unparsed value is never reached when the if is true, the else branch (let and your change) have no effect in this case.

Comment by Zubair Quraishi [ 18/Sep/14 3:30 AM ]

Yes, you are right, that code was never called. I changed the definition of match-int to

(defn- match-int
  [s]
  (let [groups (re-matches* int-pattern s)
        zero (aget groups 2)]
    groups))

and in Chrome the result is

Parsing {:s "string", :l 42, :k :keyword, :d 1.337}
{:s "string", :l #js ["42" "" nil "42"], :k :keyword, :d 1.337}
Parsing {:l 42}
{:l #js ["42" "" nil "42"]}
Parsing 42
#js ["42" "" nil "42"]

and in IE8 the result is

Parsing {:s "string", :l 42, :k :keyword, :d 1.337}
{:s "string", :l #js ["42" "" "" "42"], :k :keyword, :d 1.337}
Parsing {:l 42}
{:l #js ["42" "" "" "42"]}
Parsing 42
#js ["42" "" "" "42"]
Comment by Zubair Quraishi [ 18/Sep/14 3:31 AM ]

So my next move will be to search for "" and replace it will nil, and try that

Comment by Zubair Quraishi [ 18/Sep/14 3:43 AM ]

Something like:

(defn- match-int
  [s]
  (let [
        groups2 (re-matches* int-pattern s)
        groups  (into [] (map (fn[x] (if (= x "") nil x)) groups2))
        zero    (aget groups 2)]
    (if-not (nil? zero)
      0
      (let [a (cond
               (aget groups 3) (array (aget groups 3) 10)
               (aget groups 4) (array (aget groups 4) 16)
               (aget groups 5) (array (aget groups 5) 8)
               (aget groups 6) (array (aget groups 7)
                                      (js/parseInt (aget groups 6) 10))
               :else (array nil nil))
            n (aget a 0)
            radix (aget a 1)]
        (when-not (nil? n)
          (let [parsed (js/parseInt n radix)]
            (if (identical? "-" (aget groups 1))
              (- parsed)
              parsed)))))))
Comment by Zubair Quraishi [ 18/Sep/14 3:43 AM ]

where:

groups2  (re-matches* int-pattern s)
groups   (into [] (map (fn[x] (if (= x "") nil x)) groups2))

has been changed

Comment by Zubair Quraishi [ 18/Sep/14 11:52 AM ]

Ok, it seems to work in IE8 now. I have created the change and made a pull request:

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

https://github.com/zubairq/clojurescript/commit/1217ae69e80bf2b2093de6d46ad4b71242fdf0ca

(defn- match-int
   [s]
   (let [groups (re-matches* int-pattern s)
-        zero (aget groups 2)]
+        ie8-fix  (aget groups 2)
+        zero     (if (= ie8-fix "") nil ie8-fix)]
     (if-not (nil? zero)
       0
       (let [a (cond
Comment by Zubair Quraishi [ 18/Sep/14 12:47 PM ]

Resubmitted the patch as:

https://github.com/zubairq/clojurescript/commit/7ec0144c53b47a831f8696f9eaf97cbcfe06b2e7

Comment by Nicola Mometto [ 18/Sep/14 1:13 PM ]

Zubair, as per David's comment on your first PR (https://github.com/clojure/clojurescript/pull/39#issuecomment-56069471), clojurescript doesn't take pull requests, please attach the patch in this ticket

Comment by Colin Yates [ 24/Sep/14 8:27 AM ]

Any update on this? Not supporting IE8 is a blocker for us at the moment.

Comment by Zubair Quraishi [ 24/Sep/14 8:36 AM ]

Yes, a patch has been submitted to to Clojurescript and just waiting on the core team to integrate and release the patch now:

http://dev.clojure.org/jira/browse/CLJS-861

Is it only this numbers issue that is the blocker for you, or are there other IE8 problems you are encountering?

Comment by David Nolen [ 24/Sep/14 8:39 AM ]

Someone need to confirm that the patch on CLJS-861 doesn't have any adverse effects in other JS environments. When a couple people have chimed in I'm more than happy to merge.

Comment by Zubair Quraishi [ 24/Sep/14 8:44 AM ]

Personally I have tested this change with IE8, Desktop Safari on Mac, Firefox on mac and Windows, Chrome on Mac and Windows, Chrome Dev Tools Version, iphone 5 with ios 7 and ipad 2+ (ipad 1 has problems in general with clojurescript). But if someone else can test as well then that would be great

Comment by Colin Yates [ 24/Sep/14 8:46 AM ]

I am unaware of any other IE8 issues but that is because I haven't used it yet. I am currently deciding toolstack.next (will that ever stop being cheesy?) and currently living at http://todomvc.com.

I am astonished at the explosion of JavaScript frameworks/libraries/paradigm shifts since I last looked a few years ago.

ClojureScript is by far my preference but googling around lead me to the google discussion where this was mentioned, and then I came here .

Comment by Zubair Quraishi [ 24/Sep/14 8:58 AM ]

Well, IE8 has other problems far more serious than Javascript issues. You can see the problems on this page (view in IE8 and other browsers):

http://connecttous.co/connecttous/connecttous.html?livedebug=true

: and I have other links here on my Github page for the Clojurescript framework I am making, which I need the apps to be IE8 compatible for, as IE8 is still the standard browser for 90% of people over here (in European companies):

https://github.com/zubairq/coils

Comment by Colin Yates [ 24/Sep/14 10:53 AM ]

Not to hijack the thread or jump off track, but are you saying there are many problems in IE8+ClojureScript or in IE8 in general? I am painfully aware of the general IE8 issues .

Comment by Zubair Quraishi [ 24/Sep/14 11:37 AM ]

IE8 and Clojurescript are fantastic together, this ie8 problem with cljs.reader not reading numbers properly was probably the only showstopper issue I had, otherwise IE8 with Clojurescript has worked no problems for me at all. More general web and HTML issues are the problem with IE8. In fact using the Clojurescript Om library with IE8 actually "Increased" IE8 compatibility for me, maybe because of the way it handles DOM updates

Comment by Colin Yates [ 24/Sep/14 12:13 PM ]

That is really good to know - thanks.

Comment by David Nolen [ 26/Sep/14 1:47 AM ]

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





[CLJS-860] redundant analysis of source files in JARs Created: 18/Sep/14  Updated: 18/Sep/14

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

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


 Description   

Source files in JARs are analyzed once then analyzed again when copied to the output directory.






[CLJS-850] No warning for unresolved vars when using aliased namespace Created: 04/Sep/14  Updated: 18/Sep/14  Resolved: 18/Sep/14

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

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   
(ns foo
  (:require [cljs.core.async :as a]))

(defn bar []
 (a/go
   ...))

This code compiles fine, and then throws an error at runtime, cljs.core.async.a.go.call is not defined.



 Comments   
Comment by David Nolen [ 18/Sep/14 12:26 AM ]

CLJS-858





[CLJS-702] warn if protocol signature doesn't matched declared Created: 27/Nov/13  Updated: 18/Sep/14  Resolved: 18/Sep/14

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

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


 Comments   
Comment by David Nolen [ 18/Sep/14 12:25 AM ]

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





[CLJS-701] warn if protocol used multiple times in a deftype Created: 27/Nov/13  Updated: 18/Sep/14  Resolved: 18/Sep/14

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

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





[CLJS-704] warn if type extended to same protocol multiple times Created: 27/Nov/13  Updated: 17/Sep/14  Resolved: 17/Sep/14

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

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


 Comments   
Comment by David Nolen [ 17/Sep/14 5:58 PM ]

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





[CLJS-859] Downloads for bootstrap script should happen over HTTPS. Created: 17/Sep/14  Updated: 17/Sep/14  Resolved: 17/Sep/14

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

Type: Enhancement Priority: Major
Reporter: David Kinzer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bootstrap, https

Attachments: Text File bootstrap-over-https.patch    
Patch: Code

 Description   

Currently all downloads for the clojurescript bootstrap process are happening over HTTP. If this is switched to HTTPS it adds a layer of security at no real cost.



 Comments   
Comment by David Nolen [ 17/Sep/14 12:20 PM ]

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





[CLJS-855] Combinatorial code explosion with nested functions of unknown arity Created: 12/Sep/14  Updated: 16/Sep/14  Resolved: 16/Sep/14

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

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

ClojureScript 0.0-2322


Attachments: File test-advanced.js     File test-whitespace.js    

 Description   

The following code, when compiled with :advanced optimizations leads to 32 repetitions of the inner number 2. On less trivial examples, it leads to multi-megabyte outputs very quickly. I have attached the compiler output for :whitespace and :advanced for the latest version of the CLJS compiler.

(def good {:a {:b {:c {:d {:e 1}}}}})

(def a identity)
(def b identity)

(def bad (apply (fn [x y] (x (y (x (y (x 2)))))) [a b]))


 Comments   
Comment by Francis Avila [ 12/Sep/14 4:48 PM ]

Is this more a non-native vs native function issue than an arity knowledge issue? Does whitespace compilation explode the same way if compiled with :static-fns true?

A solution might be to only emit the `x.call(null, arg)` form if we don't know whether `x` is a cljs function at compile time. However the arity dispatch wrapper function is not optimizable by v8 right now because of how it uses `arguments` so it would be a performance regression in the non-native case.

But another thing to consider is that google advance compilation output is intended to be compressed: test-whitespace.js and test-advanced.js both gzip to exactly 379 bytes. So maybe this is a non-problem?

Comment by Brendan Younger [ 12/Sep/14 10:46 PM ]

This is an unknown arity issue, not one of native versus non-native. The indirection introduced by the explicit apply function leads to the duplication of code. Compiling with :static-fns doesn't materially change anything with :whitespace or :advanced compilation.

Whether or not gzip is able to undo the code explosion, the browser still has to parse O(2^{nesting depth}) characters which means the parsing time is increased in proportion and, likely, the internal representation of the javascript is as well. For a mobile phone with constrained memory and processing power, this is a big deal. I would argue that any code duplication leading to an exponential increase in code size is a serious defect.

Comment by David Nolen [ 15/Sep/14 12:22 PM ]

Brendan do you have an actual example where this issue matters in practice? Thanks.

Comment by micha niskin [ 16/Sep/14 5:58 PM ]

David, I have seen this with Hoplon applications. Hoplon code tends to have the sort of deeply nested expressions you see there, because the DOM tree can be both quite broad and quite deep.

In a real application we found that when attempting to compile with advanced optimizations GClosure would generate gigabytes and gigabytes of JavaScript and never finish. Without optimizations the application would take five minutes or more to load in FireFox and Safari (Chrome, for some reason, was okay, which threw us off initially.)

In fact, I ended up writing a macro to perform a sort of SSA unfolding and flattening in order to work around the issue. The macro basically transforms this:

(a (b (c (d) (e) (f))))

into this:

(let [x1 (d)
      x2 (e)
      x3 (f)
      x4 (c x1 x2 x3)
      x5 (b x4)
      x6 (a x5)]
  x6)

The macro is here: https://github.com/tailrecursion/hoplon/blob/5.10.23/src/tailrecursion/hoplon.clj#L101-L105 and it's applied automatically to the contents of the <body> of the document here: https://github.com/tailrecursion/hoplon/blob/5.10.23/src/tailrecursion/hoplon.clj#L119-L124

When I wrap large expressions with this macro the problem is mitigated. It's obviously a filthy hack and not an optimal solution by any means, but we can't make real applications without it.

Comment by David Nolen [ 16/Sep/14 6:46 PM ]

Micha thanks for clarifying the issue further. I see the problem and I think we can do the SSA bit when we don't have arity information.

Comment by David Nolen [ 16/Sep/14 8:36 PM ]

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





[CLJS-858] resolve-existing-var does not work for vars from other namespaces Created: 16/Sep/14  Updated: 16/Sep/14  Resolved: 16/Sep/14

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

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen