<< Back to previous view

[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-912] Minor enhancements to bootstrap script Created: 18/Dec/14  Updated: 18/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Alex Dowad Assignee: Unassigned
Resolution: Unresolved 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.





[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-910] JavaScriptCore 0xbbadbeef EXC_BAD_ACCESS when evaluating (list 0 1 ... 18) Created: 16/Dec/14  Updated: 18/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.





[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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 16/Sep/14 6:29 PM ]

fixed https://github.com/clojure/clojurescript/commit/877b1ab2e6bb1c09d1988348d6cb384f8ba16414





[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-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-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-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-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-852] Make group-by faster Created: 06/Sep/14  Updated: 10/Sep/14  Resolved: 10/Sep/14

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

Type: Enhancement Priority: Minor
Reporter: Mike Thompson Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Copy the clj (faster) implementation of group-by across to cljs.

The clj version uses transients and it is between 10% and 15% faster. For my larger workloads, this matters.

To be specific: copy this ...
https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L6790-L6795
to here:
https://github.com/clojure/clojurescript/blob/master/src/cljs/cljs/core.cljs#L8335-L8339



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

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





[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-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-849] Incorrect transients behavior (dissoc! deletes too much) Created: 02/Sep/14  Updated: 04/Sep/14  Resolved: 04/Sep/14

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

Type: Defect Priority: Major
Reporter: Eldar Gabdullin Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: maps, transient

Attachments: Text File 0001-CLJS-849-fix-iteration-bound-in-pack-array-node.patch    

 Description   

The snippet below reproduces the problem

(defn bug [seq]
  (let [m (transient (zipmap seq (repeat 1)))]
    (loop [m m
           [x & rest] seq]
      (when x
        (if (contains? m x)
          (recur (dissoc! m x) rest)
          (throw (js/Error. "What's going on?")))))))

(bug [44 43 42 41 40 39 38 37 36 35 34 33 32 31 30 29 28 27 26 25 24]]


 Comments   
Comment by Eldar Gabdullin [ 02/Sep/14 12:46 PM ]

Ah, transient lookup is not supported in Clojure. Why it is allowed in ClojureScript, then?

Comment by Nicola Mometto [ 02/Sep/14 12:49 PM ]

The fact that contains? doesn't work in clojure is a bug, see http://dev.clojure.org/jira/browse/CLJ-700

Comment by Eldar Gabdullin [ 02/Sep/14 12:51 PM ]

Right, already spotted that.

Comment by Michał Marczyk [ 04/Sep/14 5:02 AM ]

Wow, this is a pretty serious bug – thanks!

It is caused by an incorrect iteration bound in pack-array-node. The attached patch fixes it.

Comment by Michał Marczyk [ 04/Sep/14 5:09 AM ]

Oops, forgot the test. Attaching the same fix + test case.

Comment by Michał Marczyk [ 04/Sep/14 5:31 AM ]

A final tweak.

Comment by David Nolen [ 04/Sep/14 7:31 AM ]

fixed https://github.com/clojure/clojurescript/commit/699b7487805c786b2eebe122ca3f09d7a5205c9e

Comment by Eldar Gabdullin [ 04/Sep/14 11:48 AM ]

Thank you.





[CLJS-848] allow to split output into multiple files Created: 31/Aug/14  Updated: 02/Sep/14

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

Type: Enhancement Priority: Minor
Reporter: Stephan Behnke Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The Google Closure Compiler is able to emit multiple files after compilation, instead of just one, by using the '--modules' flag (see example here: http://stackoverflow.com/a/7827251/213730). Ergo, I would love to see the ability to customize the ClojureScript output that way!

My use case would primarily be to separate the library code (core, core.async etc.) from application code. The reasoning is that the libraries rarely change, but the app code constantly does. Also, when my app gets bigger, I could separate areas of my application into according files; libs.js, app.core.js, app.public.js, app.admin.js etc.

Note: I was about to create an issue on the Leiningen plugin but found there is one already (https://github.com/emezeske/lein-cljsbuild/issues/148). In it the plugin author says this is a compiler issue. But I wasn't able to find an issue on this tracker. So I created this one.



 Comments   
Comment by Thomas Heller [ 01/Sep/14 3:32 AM ]

I build https://github.com/thheller/shadow-build to get the module support.

Docs are a bit lacking at the moment but it should be pretty straightforward to use, happy to help if you have questions.

Comment by Stephan Behnke [ 01/Sep/14 5:31 AM ]

Is 'shadow-build' just overriding the final stage of the compiler and the rest stays up-to-date - or is it a fork?
Is there a reason you didn't try to get support for it in the main compiler (yet)?

Either way, it sounds very interesting! I'll dive into shadow-build next weekend, hopefully

Comment by David Nolen [ 01/Sep/14 3:49 PM ]

I'm actually now think it's a good idea to support this to allow breaking up large ClojureScript libraries that will be consumed by others as a plain JS dependency. I'd be happy to see GClosure modules support land in ClojureScript if someone is willing to start a design page with basic plan of the implementation strategy.

Comment by Thomas Heller [ 02/Sep/14 2:43 AM ]

@Stephan: shadow-build is not a fork, it is a replacement to the cljs.closure namespace (specifically cljs.closure/build) since a single build function is not flexible enough to do what I needed. The compiler/analyzer is untouched and is as up-to-date as you want (you provide which version of cljs you want to use, up to HEAD). The main reason I didn't try to get it into CLJS core itself is time, at the time I wrote it I needed to "get it done". Since then it just has been working so there was no need to have it in core really. I know of a couple other people using it, so it is working for others too.

@David: I'd be happy to maybe use shadow-build as a sort of reference implementation for the whole thing. I'm quite happy with it, some API/naming cleanup aside. But we should go through a proper design process first, since not all choices I made may be ideal for everyone. That being said: GClosure modules are ONLY meant to separate ONE build into multiple files. They are not what javascript commonly calls modules (since we have those basically through namespaces). There are some caveats when trying to share a build with the JS world. I will try to write up why I did what I did in shadow-build and the features it provides. I think all browser targeted builds will want those features when the project reaches a given size.

Comment by David Nolen [ 02/Sep/14 7:43 AM ]

@Thomas thanks. I've created an empty design page here: http://dev.clojure.org/display/design/Google+Closure+Modules

Comment by Thomas Heller [ 02/Sep/14 10:25 AM ]

I didn't know where to start so I did a sort of brain dump of the whole thing. Hope it is enough to get things started, happy to go into more detail.





[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-846] Preserve namespace metada Created: 27/Aug/14  Updated: 28/Aug/14  Resolved: 28/Aug/14

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

Type: Enhancement Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 846.patch    

 Description   

Namespace metadata is currently discarded and not available on cljs-ns/ns. Preserving this metadata is useful for some macros.



 Comments   
Comment by David Nolen [ 28/Aug/14 9:53 AM ]

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





[CLJS-845] incorrect behavior of sequence when given multiple collections Created: 25/Aug/14  Updated: 25/Aug/14  Resolved: 25/Aug/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: 0
Labels: None


 Description   
(def xf (map #(+ %1 %2)))
(sequence xf [0 0] [1 2])

Results in:

TypeError: Cannot find function hasNext in object


 Comments   
Comment by David Nolen [ 25/Aug/14 12:00 PM ]

fixed https://github.com/clojure/clojurescript/commit/4b81710971a1a662fdeb2db525328c3443a72545





[CLJS-844] Optimize js->clj by switching to transducers Created: 22/Aug/14  Updated: 26/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Darrick Wiebe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File use-transducers-in-js-to-clj.patch     Text File use-transducers-in-js-to-clj.patch    

 Description   

This simple patch just switches a couple of lines to use transducers. I made this change after finding that using transducers for these types of mappings is significantly faster in (at least in Chrome).

I've checked that this code is being actually tested and both pass under V8 and JavaScriptCore.



 Comments   
Comment by David Nolen [ 23/Aug/14 1:19 PM ]

Did you any benchmarks on other JS engines?

Comment by Darrick Wiebe [ 23/Aug/14 2:14 PM ]

No. My main concern was to check if there was any startup overhead that might offset the basic efficiency improvements for processing small collections. Using into with a transducer turned out to be faster in every scenario, significantly in most.

Comment by David Nolen [ 23/Aug/14 2:19 PM ]

It would be nice to see a jsperf of before and after on this ticket. Thanks.

Comment by Darrick Wiebe [ 23/Aug/14 2:23 PM ]

Is there a existing one that I can work from?

Comment by David Nolen [ 23/Aug/14 2:35 PM ]

There is not. I generally just make a simple project, create an advanced compiled artifact and copy and paste it into jsperf.

Comment by Darrick Wiebe [ 24/Aug/14 7:24 PM ]

Turns out reducing into a transient is considerably better than using a transducer (which was itself a little faster) for this.

http://jsperf.com/js-clj-transducer-test

The code is at:

https://gist.github.com/pangloss/591d77231fda460c2fbe

Let me know if you want me to prepare an updated patch.

Comment by David Nolen [ 25/Aug/14 7:34 AM ]

Thanks for putting this together. Yes please provide an updated patch.

Comment by Darrick Wiebe [ 26/Aug/14 11:19 AM ]

Not sure whether the convention is to comment that I've uploaded a new patch. Regardless, I uploaded it yesterday.





[CLJS-843] Compiling with :target :nodejs, resulting code loses CL args Created: 22/Aug/14  Updated: 23/Aug/14  Resolved: 23/Aug/14

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

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

Mac OS X 10.9.4, JDK 1.7.0_05, CLJS 0.0.2311



 Description   

Compiling with {:optimizations :advanced :target :nodejs} command line arguments are lost.

Using the following example code:

hello_cljs/core.cljs
(ns hello-cljs.core
    (:require [cljs.nodejs :as nodejs]
              [clojure.string :as s]))
 
(defn say-hello 
  [& args]
  (str (s/join " " (remove nil? (conj args "Hello")))))

(defn -main [ & args ]
  (println "Running...")
  (println (apply say-hello args)))
 
(nodejs/enable-util-print!)
(set! *main-cli-fn* -main)

with :simple optimizations:

> node hello.js me
Running...
Hello me

with :advanced optizations:

> node hello.js me
Running...
Hello

Expected behaviour should be that the results are equivalent.



 Comments   
Comment by David Nolen [ 23/Aug/14 1:32 PM ]

You need to supply an externs file for Node.js.

Comment by Peter Schwarz [ 23/Aug/14 4:24 PM ]

It would probably help to have this mentioned in the documentation, specifically in the node case, since all of the documented examples w.r.t. Node.js use :optimization :advanced.

The need for an externs file would also explain why the nodels.cljs sample doesn't work, as well.

Comment by David Nolen [ 23/Aug/14 4:34 PM ]

I've updated the ClojureScript Quick Start to point out there is no need for advanced optimizations under the Node.js target as well as explaining what is needed if for some reason it is actually necessary.

Comment by Peter Schwarz [ 23/Aug/14 4:43 PM ]

Great. Thanks!





[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-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-840] Different behaviour for read-string in clojure and clojurescript Created: 12/Aug/14  Updated: 10/Sep/14  Resolved: 10/Sep/14

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

Type: Defect Priority: Major
Reporter: Robin Heggelund Hansen Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

Clojure 1.6.0
ClojureScript 2280



 Description   

The following works in clojure: (read-string "{:weapons (:20)}")
But gives following error in cljs: "TypeError: Cannot read property '0' of null"



 Comments   
Comment by Francis Avila [ 12/Aug/14 4:17 PM ]

Duplicate of CLJS-677.

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

As far as I know this is not supported see the reader page on clojure.org





[CLJS-839] Keyword hashing mobile safari (ios7) w/ advanced compilation Created: 11/Aug/14  Updated: 27/Aug/14  Resolved: 27/Aug/14

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

Type: Defect Priority: Major
Reporter: Christian Karlsen Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File cljs-839.patch    

 Description   

I've been debugging some really funky behavior from mobile safari and found the root cause to be keyword hashing.

Version >= 2261 seems to be affected. Please see https://github.com/ckarlsen84/cljs-keyword-hashtest for a quick and dirty minimal test app (`lein ring server`).

Output from some of my runs: https://gist.github.com/ckarlsen84/4c16684c1f9e5ce26711

It seems to only happen with advanced compilation and mobile safari.



 Comments   
Comment by Thomas Heller [ 12/Aug/14 4:34 PM ]

Duplicate of CLJS-830. Nice to have a better example, was way too hard to reproduce in my app.

Comment by David Nolen [ 12/Aug/14 5:03 PM ]

This makes me think it's an operator precedence issue. But who knows, will look into it. Oh can we get details on the hardware? I was actually unable to repro this at least in Thomas Heller's case on an iPhone 5S running iOS 7.

Thanks for the minimal case!

Comment by Christian Karlsen [ 12/Aug/14 6:38 PM ]

I ran the tests with an iPhone4 (MC603KN/A).

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

Christian, am I correct in interpreting the results as actually hashing incorrectly only after several initial runs?

Comment by David Nolen [ 21/Aug/14 5:37 PM ]

In order to investigate, this ticket needs more information. Specifically it would be useful to know if native Math.imul or the shim is being used and to confirm whether the behavior occurs only after multiple runs.

Comment by Christian Karlsen [ 22/Aug/14 2:15 AM ]

Only happens after multiple runs (usually after 10-12 runs iirc). I'm travelling atm but I'll do some more tests during the weekend.

Easiest way to test if i'm using the shim or not?

Christian

Comment by David Nolen [ 22/Aug/14 7:15 AM ]

We def a imul fn based on the presence and correctness of js/Math.imul. Just do imul.toString() or something like that to figure out what you have.

The multiple runs thing makes me suspect the bug only manifests when JavaScriptCore JIT (or more efficient interpreters) kick in. This would also explain why people can't repro with the debugger attached.

Comment by David Nolen [ 26/Aug/14 12:03 PM ]

I've attached a patch (which I am unable to test since I don't have a suitable device), it would be great if anyone encountering this issue could test the patch, thanks.

Comment by Thomas Heller [ 26/Aug/14 2:40 PM ]

I did not try the patch, but I used the imul.js file and just included it in my HTML before ANY other generated javascript. Which should have the same effect as the patch.

It seems to fix the issue, but I'm too tired to do a complete check. Will do a complete check tommorrow. Looks promising though.

Comment by Thomas Heller [ 27/Aug/14 3:14 AM ]

I had the fix deployed over night, but it seems clients are still sending me maps with duplicate keys which means it doesn't work.

Will investigate later.

Comment by Kris Jenkins [ 27/Aug/14 4:12 AM ]

I've tried this test-case on an iPhone 5 (MD299B/A), and it breaks as expected. With the patch against 4b81710, it works.

Comment by Kris Jenkins [ 27/Aug/14 5:06 AM ]

Yep, this patch also fixes the bug we were seeing on our production builds under 2311. Woo-hoo!

Comment by David Nolen [ 27/Aug/14 8:30 AM ]

Kris thanks for the feedback. Looking forward to hearing if Thomas Heller sees positive results after further investigation.

Comment by Thomas Heller [ 27/Aug/14 9:05 AM ]

So far I have not been able to reproduce the bug when the fix is applied.

I forgot to include the fix on some pages which explains why I was still getting invalid maps from Safari clients.

Will keep an eye on the log but so far it seems to be resolved.

Comment by David Nolen [ 27/Aug/14 9:37 AM ]

Thanks everyone for the help on this one, fixed in master and I cut a new release 0.0-2322 https://github.com/clojure/clojurescript/commit/3c0b8e71e09a971a07e3eef70bd74df6f1104d92

Comment by Kris Jenkins [ 27/Aug/14 9:44 AM ]

Fantastic! Thanks David.





[CLJS-838] Leiningen is unable to find artifact clojurescript.jar 0.0-2311 Created: 10/Aug/14  Updated: 10/Aug/14  Resolved: 10/Aug/14

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

Type: Defect Priority: Major
Reporter: Stanislav Perekrestov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Lots of lines are skipped
...
(Retrieving ring-refresh/ring-refresh/0.1.2/ring-refresh-0.1.2.jar from clojars)
(Retrieving watchtower/watchtower/0.1.1/watchtower-0.1.1.jar from clojars)
(Retrieving ring/ring-json/0.3.1/ring-json-0.3.1.jar from clojars)
(Retrieving cheshire/cheshire/5.3.1/cheshire-5.3.1.jar from clojars)
(Retrieving tigris/tigris/0.1.1/tigris-0.1.1.jar from clojars)
(Retrieving clojure-complete/clojure-complete/0.2.3/clojure-complete-0.2.3.jar from clojars)
(Could not find artifact org.clojure:clojurescript:jar:0.0-2311 in central (https://repo1.maven.org/maven2/))
(Could not find artifact org.clojure:clojurescript:jar:0.0-2311 in clojars (https://clojars.org/repo/))
This could be due to a typo in :dependencies or network issues.
If you are behind a proxy, try setting the 'http_proxy' environment variable.



 Comments   
Comment by Stanislav Perekrestov [ 10/Aug/14 3:40 PM ]

Lein 2.4.3
OSX 10.9.4

Comment by David Nolen [ 10/Aug/14 5:38 PM ]

This is not a ClojureScript issue, it's one with Maven Central.





[CLJS-837] Cache namespace env Created: 08/Aug/14  Updated: 11/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-add-support-for-cached-env.patch    

 Description   

See: https://github.com/clojure/tools.analyzer.js/blob/master/src/main/clojure/clojure/tools/analyzer/js.clj#L524-L548



 Comments   
Comment by Nicola Mometto [ 11/Aug/14 8:09 AM ]

I've written an initial patch based on the same approach I used for t.a.js, it's attached as 0001-add-support-for-cached-env.patch
Currently I'm seeing an OOM exception when I invoke backup-env, after loading cljs.core, which is exactly what David told me he was getting when toying with a similar patch.

At a quick glance, it seems like the analyzer's ::namespace map contains significant more fields per var map than tools.analyzer.js's one, so that's probably why backup-env works fine for t.a.js but not for cljs





[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-835] resolve-var incorrectly resolves symbols in the form "local.foo" Created: 06/Aug/14  Updated: 07/Aug/14  Resolved: 07/Aug/14

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

Type: Defect Priority: Trivial
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-fix-resolve-var-handling-of-property-access-on-local.patch    
Patch: Code

 Description   
ClojureScript:cljs.user> (fn [] (let [a 1 ab 2] a.b))
#<
function () {
    var a = (1);
    var ab = (2);
    return ab;
}
>

The return statement should be compiled to "a.b" instead



 Comments   
Comment by David Nolen [ 07/Aug/14 3:47 PM ]

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





[CLJS-834] Cleanup interop syntax used in the runtime lib Created: 04/Aug/14  Updated: 06/Aug/14  Resolved: 06/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Cleanup-interop-syntax-used-in-the-runtime-lib.patch    
Patch: Code

 Description   

This patch cleans up a bunch of interop forms through the standard lib to make it more consistent, without changing the semantics of the code.



 Comments   
Comment by David Nolen [ 06/Aug/14 8:31 AM ]

There is a typo in this patch vents/unlisten

Comment by Nicola Mometto [ 06/Aug/14 8:38 AM ]

Updated the patch fixing the typo

Comment by David Nolen [ 06/Aug/14 2:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/040bcd241dbb928479a4e0326f13bcbb3859390c





[CLJS-833] A named fn shadows `js/fn-name` Created: 04/Aug/14  Updated: 04/Aug/14

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

Type: Defect Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-833-Test-for-fn-name-shadowing.patch    

 Description   

Description

The function

(fn console [] js/console)

will return a reference to itself when called.
This happens because the function is transpiled to

(function console(){return console;})

Solution proposals

Mangle internal function names like let bindings

The internal name of a generated js function should be treated like a let binding, hence gensymed.
Thus the function would be transpiled to something like

(function console_1337(){return console;})

References

Brought up in https://groups.google.com/d/msg/clojure/QZmGrjNVurs/NxFtq8yDCFIJ



 Comments   
Comment by Herwig Hochleitner [ 04/Aug/14 4:10 AM ]

Attached test uses js/eval, because that's one of the shortest global identifiers, that should be available on every js runtime.

Comment by Herwig Hochleitner [ 04/Aug/14 12:43 PM ]

As detailed in the ML thread, CLJS-680 introduced :js-globals in an attempt to fix this issue. :js-globals should be removed along with a proper fix.





[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-831] Extending EventType to js/Element breaks Nashorn Created: 03/Aug/14  Updated: 11/Aug/14  Resolved: 11/Aug/14

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

Type: Defect Priority: Major
Reporter: Dylan Butman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Nashorn


Attachments: Text File cljs_831.patch     Text File cljs_831.patch    

 Description   

js/Element is undefined in Nashorn (and I would assume other browserless environments)

https://github.com/clojure/clojurescript/blob/master/src/cljs/clojure/browser/event.cljs#L33



 Comments   
Comment by David Nolen [ 07/Aug/14 3:49 PM ]

can we rebase this patch on master?

Comment by Dylan Butman [ 09/Aug/14 7:43 PM ]

yea shouldn't be a problem. Is that something I can/should do?

Comment by David Nolen [ 09/Aug/14 8:23 PM ]

Yes create a new patch rebased on master. Thanks!

Comment by Dylan Butman [ 11/Aug/14 11:15 AM ]

attached the new patch

Comment by David Nolen [ 11/Aug/14 2:15 PM ]

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





[CLJS-830] Hashing issue in Mobile Safari clients Created: 30/Jul/14  Updated: 10/Sep/14  Resolved: 10/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: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

Duplicate key errors occurring in Mobile Safari client no minimal case as of yet.



 Comments   
Comment by Thomas Heller [ 30/Jul/14 4:45 PM ]

I made a list of affected User Agents sending me maps with duplicate keys.

Full List available here:
https://gist.github.com/thheller/8cae79cabd9ac74958ca
WARNING: That is a list of ALL user agents that send me a map with duplicate keys. Some of those are from cljs-2268, but the mobile versions definitly still have this problem on 2277. Will try to make a better list. The examples below are from 2277 though.

Examples:
Mozilla/5.0 (iPad; CPU OS 7_1_2 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Version/7.0 Mobile/11D257 Safari/9537.53
Mozilla/5.0 (iPhone; CPU iPhone OS 7_1_2 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Version/7.0 Mobile/11D257 Safari/9537.53
Mozilla/5.0 (Linux; U; Android 4.1.2; de-de; GT-N8010 Build/JZO54K) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Safari/534.30

Comment by David Nolen [ 31/Jul/14 7:03 PM ]

Are you sure this isn't because clients have cached versions of your script this looks exactly like this bug: https://bugs.webkit.org/show_bug.cgi?id=126345. Mobile clients tend to have aggressive script caching policies.

Comment by Thomas Heller [ 31/Jul/14 7:27 PM ]

Yes, definitly not a cache issue. JS files are versioned. Just tried in Private Mode after wiping the cache. Still able to reproduce it.

I can give you an URL and a step-by-step if you want to try it. Its not exactly minimal but its better than nothing.

Comment by David Nolen [ 31/Jul/14 7:29 PM ]

Please do, I'd prefer to get this resolved before cutting another release. Thanks.

Comment by Thomas Heller [ 12/Aug/14 4:35 PM ]

CLJS-839, duplicate Issue with a minimal example!





[CLJS-829] :emit-constants Keywords are emitted without hash value Created: 29/Jul/14  Updated: 29/Jul/14  Resolved: 29/Jul/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 emits-keyword.diff    
Patch: Code

 Description   

The cljs.compiler emits keywords as new cljs.core.Keyword(ns, name, str, hash) except when using :emit-constants which will emit new cljs.core.Keyword(ns, name, str).

The reason is a bit of duplicate code, the patch creates a new function emits-keyword which is then used in both places.

See:
https://github.com/clojure/clojurescript/blob/f0dcc75573a42758f8c39b57d1747a2b4967327e/src/clj/cljs/compiler.clj#L204
https://github.com/clojure/clojurescript/blob/f0dcc75573a42758f8c39b57d1747a2b4967327e/src/clj/cljs/compiler.clj#L1080



 Comments   
Comment by David Nolen [ 29/Jul/14 1:45 PM ]

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





[CLJS-828] Rhino support broken with new google-closure library Created: 25/Jul/14  Updated: 25/Jul/14  Resolved: 25/Jul/14

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

Type: Defect Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

org.clojure/clojurescript "0.0-2268"
org.clojure/google-closure-library "0.0-20140226-71326067"



 Description   

cljs.repl.rhino/repl-env reads two files: goog/base.js and goog/deps.js. Unfortunatelly both are deprecated and empty in the latest version of google-closure-library.

Rhino ends with message:

org.mozilla.javascript.EcmaError: ReferenceError: "goog" is not defined. (bootjs#1)

Startup code has to be updated to reflect these changes.

goog/base.js
// This is a dummy file to trick genjsdeps into doing the right thing.
// TODO(nicksantos): fix this
goog/deps.js
/**
 * @deprecated This file is deprecated. The contents have been
 * migrated to the main deps.js instead (which is auto-included by
 * base.js).  Please do not add new dependencies here.
 */


 Comments   
Comment by Daniel Skarda [ 25/Jul/14 6:37 AM ]

Ups. It seems I have not been investigating the issue enough.

The problem is probably related to #826. Google Closure Library contains correct files, but it is google-closure-third-party which contains dummy files. I will investigate if the patch from #826 closes the issue with Rhino as well.

Comment by Daniel Skarda [ 25/Jul/14 7:21 AM ]

Confirmed. I deleted the files and Rhino works again (and cljs-nashorn as well). Please release the fix for #826 soon.

Comment by David Nolen [ 25/Jul/14 1:29 PM ]

Cutting a release today





[CLJS-827] wrap macro expansion in try/catch Created: 14/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/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: 0
Labels: None

Attachments: Text File cljs_827.patch    

 Description   

This would allow us to report more accurate error file/line location when macroexpansions fails in Clojure, i.e. in the case of `defn`.



 Comments   
Comment by Max Veytsman [ 15/Jul/14 12:28 AM ]

It turned out that there was already a `wrapping-errors` macro that did what we needed. I used it within the body of `macroexpand-1` in order to annotate macro expansion errors with line numbers.

Comment by David Nolen [ 16/Jul/14 7:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/13501318b4967eb5d0e486907e6cb35da81d60b9





[CLJS-826] Closure goog/base.js in third party closure release Created: 09/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/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
Environment:

Recent google/closure-library checkout


Attachments: Text File cljs-closure-release-script-fix.patch    
Patch: Code

 Description   

Not sure when this started but the more recent "org.clojure/google-closure-library-third-party" releases started containing a dummy "goog/base.js" which break build using my shadow-build library. The "make-closure-library-jars.sh" script used to delete that base.js but I guess the directory changed (maybe when the repo moved to github).

Patch just deletes the new location. Could possibly just forget about the old location, happy to provide a patch that replaces the paths instead of adding them.

Proof:

jar -tvf org/clojure/google-closure-library-third-party/0.0-20140226-71326067/google-closure-library-third-party-0.0-20140226-71326067.jar 
     0 Fri Mar 07 15:18:58 CET 2014 META-INF/
    68 Fri Mar 07 15:18:58 CET 2014 META-INF/MANIFEST.MF
   533 Fri Mar 07 15:18:56 CET 2014 AUTHORS
 10173 Fri Mar 07 15:18:56 CET 2014 LICENSE
   293 Fri Mar 07 15:18:56 CET 2014 README
     0 Fri Mar 07 15:18:56 CET 2014 goog/
   101 Fri Mar 07 15:18:56 CET 2014 goog/base.js
   822 Fri Mar 07 15:18:56 CET 2014 goog/deps.js


 Comments   
Comment by Thomas Heller [ 09/Jul/14 7:11 PM ]

https://github.com/clojure/clojurescript/commit/bae40ab9a9924a05a7425a734de2315c3185a5f6

This commit is to blame I guess, so its fine to just change the paths instead of keeping the old ones arround (since they never existed).

Comment by David Nolen [ 16/Jul/14 7:50 AM ]

fixed https://github.com/clojure/clojurescript/commit/13501318b4967eb5d0e486907e6cb35da81d60b9





[CLJS-825] Conflict between cljs/nodejs.cljs & cljs/nodejs.js Created: 09/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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

Type: Defect Priority: Major
Reporter: Boris Kourtoukov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: nodejs


 Description   

This is the gist with errors and bare minimal example: https://gist.github.com/BorisKourt/7b1434ec490cfa6c8397



 Comments   
Comment by Boris Kourtoukov [ 09/Jul/14 2:15 PM ]

Noticed this while working with the main.cljs file linked in the above gist.

As described by David Nolen: cljs/nodejs.cljs cljs/nodejs.js are getting mixed up by the compiler even though they are completely different.

This explains why the error is intermittent, but the warning is consistent.

This is reproducible with an even more minimal example as well. Just wanted to link a basic but functional one.

Comment by John Wang [ 12/Jul/14 12:21 AM ]

As a more convenient test case, this also happens with samples/nodehello.cljs when following the build instructions given in the comments of that file.

Comment by David Nolen [ 16/Jul/14 4:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/95122e1206d4a64f392cff03bfd73712ab7f3a33





[CLJS-824] Unsigned hash for keywords produced with (keyword s) Created: 06/Jul/14  Updated: 16/Jul/14  Resolved: 16/Jul/14

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

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

Attachments: Text File cljs_824-2.patch     Text File cljs_824.patch    

 Description   

ClojureScript will produce unsigned hash values at times (in particular for keywords produced with the keyword fn), and this is inconsistent with hashes produced on literal keywords and inconsistent with Clojure. The hash values will have the same 32-bit hex value, so this is perhaps not that critical for many uses.

Examples:

cljs.user=> (hash :op)
;=> -1882987955

cljs.user=> (hash (keyword "op"))
2411979341

This unsigned value is produced on Safari (shipping and WebKit nightly), and thus is in theory produced via both variants of the imul implementation (see CLJS-823). The unsigned value is also produced by Firefox.

In either case the value is equivalent to 0x8fc3e24d and works for many use cases, but it fails an = test.

user=> (= (hash :op) (hash (keyword "op")))
;=> true

cljs.user=> (= (hash :op) (hash (keyword "op")))
;=> false



 Comments   
Comment by Mike Fikes [ 06/Jul/14 8:27 PM ]

Not sure of the perf, but the attached cljs_824.patch appears to resolve the issue.

Comment by Mike Fikes [ 06/Jul/14 10:04 PM ]

Attached a simpler cljs_824-2.patch:

The JavaScript idiom to coerce to signed 32-bit is x|0. The revised patch does this, but by simply invoking the cljs core int fn (which in turn does a bit-or with 0).

Comment by David Nolen [ 16/Jul/14 4:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/5ca535759f8e2490b42dfa25df0458a6c3376d8c





[CLJS-823] Hashing regression with JavaScriptCore Created: 05/Jul/14  Updated: 06/Jul/14  Resolved: 06/Jul/14

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

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


 Description   

With https://github.com/clojure/clojurescript/commit/6ed5857aa49f28dc81390e3522e85b497be0f9de Weasel (https://github.com/tomjakubowski/weasel) functionality regressed for JavaScriptCore/Safari (but not with Firefox or Chrome), where the result appears to the end user as a Weasel hang when issuing a browser REPL command.

At its core, it appears to be related to a failure to parse / read a Weasel command of the form {:op :eval-js, :code "cljs.core.pr_s ... <omitted for brevity>"} within the browser, where the :op :eval-js keyword/value pair is not parsed, resulting in a subsequent multimethod dispatching on :op in Weasel's process-message multimethod to derail.

More detail is in https://github.com/james-henderson/simple-brepl/issues/4, but my plan is to attempt to formulate a minimal setup to reproduce the failure.



 Comments   
Comment by Mike Fikes [ 05/Jul/14 9:48 PM ]

Perhaps a minimal reproduction of the failure is achieved by invoking the cljs reader directly in JavaScript as described below:

Within the Safari JavaScript REPL, evaluating the following

> cljs.reader.read_string.call(null, "{:op :eval-js :code :x}");

results in a JavaScript object representing the map. Prior to the hashing commit referenced in this ticket's description, this map representation has 4 elements in its root arr (representing the 2 key/val pairs), where element 0 is "op", with a _hash value of 1013907795, which matches the :op keyword expression that is emitted in various places in my JavaScript output (new cljs.core.Keyword(null, "op", "op", 1013907795)).

After the hashing commit, evaluating the same at the JavaScript REPL, the resulting map representation is a bit "wonky" in Safari with respect to hash values (but is similar to that described in the previous paragraph, in Firefox). By "wonky", it's root arr has 2 elements, the 1st being null, and the 2nd containing an arr of length 4 (which appears to be the 2 key/val pairs, simply a bit lower in the trie), but where the hash is 1013904242 for both the item representing the "op" keyword and for the item representing the "code" keyword, and since this hash differs from that for the :op keyword expression (new cljs.core.Keyword(null, "op", "op", -1882987955)), the attempt to get :op from the map fails. (Note that in the Firefox debugger, the element associated with "op" has a hash of 2411979341, which actually matches -1882987955 (the first being an unsigned and the 2nd being 2's complement representation of the same 32-bit hex value).

Comment by Mike Fikes [ 05/Jul/14 11:22 PM ]

An equivalent, but perhaps simpler reduction of the bug:

(ns foo.bar
(:require [cljs.reader :as reader]))

(println "Expecting :js-eval here:" (:op (reader/read-string "{:op :js-eval :code :x}")))

With 0.0-2261 this will print

Expecting :js-eval here: nil

but with 0.0-2234 this will print

Expecting :js-eval here: :js-eval

if you run this in the shipping version of JavaScriptCore (Safari or iOS).

Note that this bug does not occur if you use a WebKit nightly. Thus, automated integration / unit tests for this against WebKit nightly fail to detect the problem.

Comment by Mike Fikes [ 06/Jul/14 8:12 AM ]

I think this is valid simpler reduction, focusing on the hash of a read keyword (these numbers reflect those 2 comments back):

(println (hash :op))
;=> -1882987955

(println (hash (let [r (reader/push-back-reader "op ")]
(reader/read-keyword r nil))))
;=> 1013904242

Comment by Mike Fikes [ 06/Jul/14 8:24 AM ]

A simpler reduction, independent of the reader code:

(hash :op)
;=> -1882987955

(hash (keyword "op"))
;=> 1013904242

Comment by Mike Fikes [ 06/Jul/14 8:58 AM ]

Perhaps it is related to hash cacheing rather than hash computation:

(hash (keyword "op"))
;=> 1013904242

(hash (keyword "code"))
;=> 1013904242

(hash (keyword "abc"))
;=> 1013904242

Comment by David Nolen [ 06/Jul/14 9:51 AM ]

All released versions of Safari 7 have a broken implementation of Math.imul, fall back to non-native imul implemented now in master https://github.com/clojure/clojurescript/commit/e92e8064813ed9a74c6dcf5bfd3adf5b85df1aea





[CLJS-822] "TypeError: invalid 'in' operand a" Created: 05/Jul/14  Updated: 29/Jul/14  Resolved: 29/Jul/14

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

Type: Defect Priority: Major
Reporter: Zack Piper Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: clojure.browser.repl, javascript
Environment:

Source: http://github.com/zackp30/crateincinerator
OS: Linux openSUSE 13.1, Bottle
Browser: Firefox 25.0



 Description   

When using `clojure.browser.repl` in a ClojureScript project, the evaluation fails due to the error `TypeError: invalid 'in' operand a @ http://127.0.0.1:3000/js/site.js:6559`
Source of line:
`if (d in a && !/^https?:\/\//.test(a[d])) {`
^ fails here, definitely syntax error.

Sorry if I have put this in completely the wrong place...

Thanks!



 Comments   
Comment by David Nolen [ 16/Jul/14 4:33 PM ]

This ticket need more information - complete simple steps to reproduce with no external dependencies beyond ClojureScript, thanks.

Comment by Zack Piper [ 28/Jul/14 8:06 AM ]

Wow, sorry for this, but it now works. I couldn't reproduce the error in this ticket, however, I got one ([14:05:57.342] TypeError: parentElm is null @ http://localhost:3000/js/main.js:33501), indicating that it was initializing before the document was ready (I blame JavaScript), so I put it at the bottom of the file, and bam, fixed.
Again, sorry.
Thanks!





[CLJS-821] specify/specify! fails for Object methods Created: 04/Jul/14  Updated: 04/Jul/14  Resolved: 04/Jul/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: Declined Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 04/Jul/14 1:59 PM ]

not a issue





[CLJS-820] MetaFn invoke without arguments is missing Created: 01/Jul/14  Updated: 01/Jul/14  Resolved: 01/Jul/14

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

Type: Defect Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript from github aa9e5fe (post r2234)


Attachments: Text File 0001-CLJS-820-Missing-invoke-without-arguments-in-MetaFn.patch    

 Description   

MetaFn lacks definition of invoke without arguments. Missing invoke breaks asynchronous tests in cemerick.cljs.test.
Fix is trivial, patch attached.



 Comments   
Comment by David Nolen [ 01/Jul/14 9:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/08832549ce2ed386300fa637d7f88a75d57cc344





[CLJS-819] cljs.reader cannot handle character classes beginning with slashes in regex literals Created: 20/Jun/14  Updated: 01/Jul/14  Resolved: 01/Jul/14

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

Type: Defect Priority: Critical
Reporter: Ziyang Hu Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug, cljs, reader

Attachments: Text File cljs-819.patch    

 Description   

cljs.user> (cljs.reader/read-string "#\"\\s\"")
Compilation error: Error: Unexpected unicode escape \s



 Comments   
Comment by Ziyang Hu [ 20/Jun/14 10:03 AM ]

This in particular means that (cljs.reader/read-string (str [#"\s"])) won't work

Comment by Francis Avila [ 25/Jun/14 11:46 AM ]

Patch and test.

Comment by David Nolen [ 01/Jul/14 9:25 PM ]

fixed https://github.com/clojure/clojurescript/commit/32259c5ff3f86ea086ae3949403df80c2f518c7e





[CLJS-818] Externs don't get loaded when running under immutant as cljs.js-deps/find-js-classpath fails Created: 18/Jun/14  Updated: 01/Jul/14

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

Type: Defect Priority: Major
Reporter: James Cash Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Java 1.8.0_05, Clojure 1.6.0, clojurescript 0.0-2234, immutant 1.1.1


Attachments: Text File 818.patch     PNG File Screen Shot 2014-06-18 at 20.14.59 .PNG    

 Description   

When compiling clojurescript that relies on library-provided externs (e.g. Om needing React.js externs), the clojurescript is compiled without errors, but the generated javascript fails to work, due to the externs not being loaded. Externs don't get loaded, as cljs.js-deps/find-js-classpath doesn't find the javascript externs file. This occurs because it uses cljs.js-deps/all-classpath-urls, which filters out the immutant classloader, since org.immutant.core.ImmutantClassLoader is not an instance of java.net.URLClassLoader (and hence lacks a .getURLs method anyway).



 Comments   
Comment by Toby Crawley [ 19/Jun/14 9:23 AM ]

Chas: Is there a reason not to depend on dynapath here? This exact case is kinda why it exists

Comment by David Nolen [ 19/Jun/14 10:47 AM ]

Patch welcome for this.

Comment by James Cash [ 19/Jun/14 2:12 PM ]

Simply replacing cljs.js-deps/all-classpath-urls with dynapath.util/all-classpath-urls worked for me. I don't know if there are policies around adding dependencies to cljs, but the following patch is working for me. Would it be preferable to re-implement the functionality instead?

Comment by David Nolen [ 19/Jun/14 2:19 PM ]

We are not going to take on a dependency for this. The code should be copied over, thanks.

Comment by James Cash [ 19/Jun/14 3:46 PM ]

Due to the way dynapath works, I don't think a straightforward copying of the code will work, since it relies on a protocol. Backing up a step though, would it be reasonable for externs to be loaded via io/resource, in the same way that the :preamble is?

Comment by Toby Crawley [ 19/Jun/14 3:54 PM ]

Unfortunately, the code can't be copied over. Dynapath works by providing a protocol that providers/users of funky classloaders can implement, allowing libraries that use dynapath to access the dynamic features of those classloaders without having to care about the loader's concrete type. Dynapath itself provides implementations for j.n.URLClassLoader and c.l.DynamicClassloader by default, so libraries don't have to do anything special to access the dynamic features of those classes.

java.classpath also provides a similar mechanism that the Immutant classloader implements as well. If you are more open to dependencies that are under org.clojure, using that will work as well. Ideally, I'd like to see java.classpath subsume dynapath.

Comment by James Cash [ 19/Jun/14 4:23 PM ]

Made a new patch that sidesteps the all-classpath-urls issue by just using io/resource instead of iterating over all urls

Comment by David Nolen [ 01/Jul/14 9:26 PM ]

Can people chime in whether the patch works for them, thanks.





[CLJS-817] Warning on use of undeclared var when creating recursive definition Created: 17/Jun/14  Updated: 01/Jul/14  Resolved: 01/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs_817.patch    

 Description   

If you create a definition that recursively refers to itself, a warning is issued. This is simply a warning; the generated code still works. The same code works without such a warning in Clojure.

Here is an example:

(ns foo.bar)

(def fib-seq (lazy-cat [0 1] (map + (rest fib-seq) fib-seq)))

This generates the following warning:

Compiling "js/main.js" from ["src/cljs" "src/clj"]...
WARNING: Use of undeclared Var foo.bar/fib-seq at line 3 src/cljs/foo/bar.cljs
WARNING: Use of undeclared Var foo.bar/fib-seq at line 3 src/cljs/foo/bar.cljs
Successfully compiled "js/main.js" in 5.819 seconds.



 Comments   
Comment by Mike Fikes [ 17/Jun/14 7:09 PM ]

An easy workaround is to forward declare. For the example given in the description,

(declare fib-seq)

suffices.

Comment by David Nolen [ 19/Jun/14 10:49 AM ]

Would be happy to take a patch that propagates the def'ed var so that the initializer expression has it during analysis.

Comment by Mike Fikes [ 19/Jun/14 3:58 PM ]

Patch attached.

Note that I'm a new contributor (I signed the online CLA), but I have no experience with the ClojureScript compiler codebase. In other words, increased review for this patch is appropriate IMHO.

Comment by David Nolen [ 19/Jun/14 4:45 PM ]

The patch initially looks OK will give a closer look tomorrow. Thanks!

Comment by Mike Fikes [ 20/Jun/14 9:54 AM ]

Thanks David. Let me know if the patch could use further work. Glad to revise as necessary.

Comment by David Nolen [ 01/Jul/14 9:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/054b0142e303586761d3da01849a2e24896ad7dd





[CLJS-816] clojure.set/rename-keys accidentally deletes keys when there's a collision. Created: 13/Jun/14  Updated: 24/Jun/14  Resolved: 24/Jun/14

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

Type: Defect Priority: Minor
Reporter: Brian Kim Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-816.patch    

 Description   

Same as the error on the clojure side (http://dev.clojure.org/jira/browse/CLJ-981). rename-keys will drop keys when there's a mutual overwrite.



 Comments   
Comment by Brian Kim [ 13/Jun/14 11:26 PM ]

Probably messed this up. Sorry!

Comment by David Nolen [ 16/Jun/14 11:05 AM ]

Can we please include a test with the patch? Thanks.

Comment by Brian Kim [ 23/Jun/14 9:07 PM ]

with test.

Comment by David Nolen [ 24/Jun/14 11:26 AM ]

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





[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-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-813] Warn about reserved JS keyword usage in namespace names Created: 11/Jun/14  Updated: 16/Jul/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 namespace is identified as foo.long.core it will get munged into foo.long$.core. This is unexpected and a source of confusion when goog.require("foo.long.core") fails.



 Comments   
Comment by Max Veytsman [ 15/Jul/14 4:50 PM ]

I'm starting to take a look at this.

Would it be most appropriate to add this check into compiler.cljs where the munging actually happens, or into analyzer.cljs where most of the warnings of this type live?

Comment by Mike Fikes [ 15/Jul/14 5:34 PM ]

If a solution is identified that eliminates “overly aggressive” munging for certain cases, then CLJS-689 could benefit.

Comment by Max Veytsman [ 16/Jul/14 2:44 PM ]

Currently, when munging "foo.bar.baz", we map over ["foo", "bar", "baz"] and check if each is a reserved word (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L94-L95)

According to my understanding of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Reserved_word_usage and https://es5.github.io/#x7.6 reserved words are only disallowed in Identifiers. MemberExpressions and CallExpressions (the things with "."s in them) do not ban reserved words except in the first Identifier.

For our purposes, it could be enough to check if the entire token and the first period-seperated element is reserved. I.e. long becomes long$, long.night.ahead becomes long$.night.ahead, but foo.long.core remains foo.long.core.

Mike, this unfortunately won't affect CLJS-689

Does that sound like a good approach?





[CLJS-812] Recurring from a case statement emits invalid JavaScript Created: 09/Jun/14  Updated: 13/Jun/14  Resolved: 13/Jun/14

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

Type: Defect Priority: Critical
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug, compiler


 Description   

In 0.2227, compiling the following form produces syntactically invalid JavaScript:

(defn reproduce
  [value]
  (case value
    :a (recur :b)
    :b 0))

Yields:

bug_repro_test.reproduce = (function reproduce(value) {
    while (true) {
        var G__5832 = (((value instanceof cljs.core.Keyword)) ? value.fqn : null);
        var caseval__5833;
        switch (G__5832) {
            case "b":
                caseval__5833 = 0
                break;
            case "a":
                caseval__5833 = {
                    var G__5834 = cljs.core.constant$keyword$67;
                    value = G__5834;
                    continue;
                }

                break;
            default:
                caseval__5833 = (function () {
                    throw (new Error(("No matching clause: " + cljs.core.str.cljs$core$IFn$_invoke$arity$1(value))))
                })()
        }
        return caseval__5833;
        break;
    }
});

When evaluated in any JavaScript environment (including the Google Closure compiler) environment, this yields a syntax error in this code:

caseval__5833 = {
                    var G__5834 = cljs.core.constant$keyword$67;
                    value = G__5834;
                    continue;
                }


 Comments   
Comment by David Nolen [ 10/Jun/14 7:56 AM ]

Good catch. I suspect that throw may be similarly problematic.

Comment by David Nolen [ 13/Jun/14 3:59 PM ]

fixed in master https://github.com/clojure/clojurescript/commit/cc11c7996ba8522d6767fb45df2f76e20e4c1773





[CLJS-811] cljs.js-deps/goog-resource doesn't get the correct classloader when running under immutant Created: 05/Jun/14  Updated: 06/Jun/14  Resolved: 06/Jun/14

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

Type: Defect Priority: Minor
Reporter: James Cash Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: classloader, cljsc
Environment:

Java 1.8.0_05, Clojure 1.6.0, clojurescript 0.0-2202, immutant 1.1.1



 Description   

When attempting to call cljs.closure/build when running under immutant, cljs.js-deps/goog-dependencies* fails, because the classloader goog-resources uses doesn't seem to be correct: In a repl, I can see that ClassLoader/getSystemClassLoader doesn't find goog/deps.js, but using (.getContextClassLoader (Thread/currentThread)) (as in clojure.java.io/resource) works:

user=> (enumeration-seq (.getResources (ClassLoader/getSystemClassLoader) "goog/deps.js"))
nil
user=> (enumeration-seq (.getResources (.getContextClassLoader (Thread/currentThread)) "goog/deps.js"))
(#<URL vfs:/Users/james/Work/LeanPixel/Next36/blocks/tmp_mounts/blocks.clj/google-closure-library-third-party-0.0-20140226-71326067.jar/goog/deps.js> #<URL vfs:/Users/james/Work/LeanPixel/Next36/blocks/tmp_mounts/blocks.clj/google-closure-library-0.0-20140226-71326067.jar/goog/deps.js>)



 Comments   
Comment by David Nolen [ 06/Jun/14 9:50 AM ]

fixed https://github.com/clojure/clojurescript/commit/737ccb0aab55816ce7f49dfb86130fbb34445bb8





[CLJS-810] re-matches returns [] if string is nil Created: 04/Jun/14  Updated: 02/Jul/14  Resolved: 02/Jul/14

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

Type: Defect Priority: Major
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

clojurescript 0.0-2227


Attachments: Text File cljs-810-nil.patch     Text File cljs-810.patch     Text File cljs-810-throw.patch    

 Description   

(re-matches #"no-match" "") => nil
(re-matches #"no-match" nil) => []

In clojure calling re-matches on nil throws an exception, but in clojurescript it returns [].



 Comments   
Comment by David Nolen [ 06/Jun/14 9:32 AM ]

this is a platform distinction, otherwise we need to put checks for nil and throw exceptions in several places.

Comment by Stephen Nelson [ 08/Jun/14 9:27 PM ]

I understand the need for efficiency and recognise that it is reasonable to have differences between platforms, but I don't think it's reasonable to return truthy when given null – the regex did not match, the function should return falsey.

I think the performance implication can be avoided using a case switch on the result of the existing length test:

Replace:

(if (== (count matches) 1)
(first matches)
(vec matches))

With:

(case (count matches)
0 nil
1 (first matches)
(vec matches))

Comment by Stephen Nelson [ 08/Jun/14 9:33 PM ]

Just to note, this is not a hypothetical problem, we encountered a bug in production where a 'local url' regex test passed when it was unexpectedly given nil. This bug was hard to track down because it's not obvious even from reading the implementation that the function could return truthy when the regex didn't match.

Comment by David Nolen [ 09/Jun/14 5:28 PM ]

Oh, I didn't realize that re-matches differs from re-find in this regard. Is there any reason that re-matches can't follow re-find? If it can, yes, patch welcome.

Comment by Francis Avila [ 09/Jun/14 6:06 PM ]

`re-find` and `re-matches` are both completely broken for nil because of js coersion of nil to the string "null". Your suggested fix (using a case statement) is not radical enough to solve this.

(re-find #"." nil)
; => "n"
(re-matches #"." nil)
; => nil

Note that `re-matches` itself doesn't work as you might expect: CLJS-776. You should use re-find with an explicit `^` and `$` instead.

I think both re-find and re-matches should have an explicit nil check on their string argument.

Comment by Francis Avila [ 12/Jun/14 2:51 AM ]

The fundamental issue here is that RegExp.exec(x) in javascript will x.toString() its argument before applying the regex. This leads to surprising behavior for anything which is not a string.

Options are:

  1. Call it a platform difference and leave current behavior as-is.
  2. Return nil if argument is not a string (unlike clojure, which throws).
  3. Throw if argument is not a string (like clojure and re-seq in cljs).

Attached patch takes option 2 and does nothing about re-seq (which currently throws). This is inconsistent with clojure but nil-punning is convenient. Option 3 seems equally valid, though.

Comment by David Nolen [ 12/Jun/14 10:13 AM ]

I think I prefer option 3 if it aligns us with Clojure, thanks!

Comment by Francis Avila [ 24/Jun/14 10:16 AM ]

Added patch which throws TypeError instead of nil for non-string.

Comment by David Nolen [ 01/Jul/14 9:29 PM ]

Can we rebase this patch to master? Thanks!

Comment by Francis Avila [ 01/Jul/14 11:27 PM ]

Rebased to master.

Comment by David Nolen [ 02/Jul/14 7:55 AM ]

fixed https://github.com/clojure/clojurescript/commit/774d4588581f6d29a1b6b2555171d3f3d36c2c83





[CLJS-809] tools.reader 0.8.4 causes clojurescript to stop working in mysterious ways Created: 04/Jun/14  Updated: 06/Jun/14  Resolved: 06/Jun/14

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

Type: Defect Priority: Major
Reporter: James Cash Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

OS X 10.9.3, Java 1.8, Clojure 1.6.0



 Description   

If tools.reader 0.8.4 is included in a project's dependencies, then clojurescript fails to work in a browser, with the error that cljs.core.PersistentArrayMap is undefined. I have created a sample project demonstrating this at https://github.com/jamesnvc/cljs-toolreader-debugging.

It was suggested by Nicola Momento on tools.reader that the issue is due to "tools.reader 0.8.4 introduced :file metadata, clojurescript needs to dissoc it here https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L1490 to work with this version."






[CLJS-808] Warning from `find-classpath-lib` mistakenly included in generated source Created: 02/Jun/14  Updated: 02/Jun/14

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

Type: Defect Priority: Major
Reporter: Joshua Ballanco Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojurescript 0.0-r2227
target: node


Attachments: Text File warn-with-out-str-fix.patch    
Patch: Code

 Description   

When compiling for node.js, `find-classpath-lib` generates a warning because `cljs/nodejs.js` doesn't contain a `goog.provide` declaration. However, this warning ends up being emitted into the JS files being generated (caused by a call to `with-out-str`), later causing a critical failure in the Closure compiler.

To reproduce:

  • `lein new cljs-node buggy`
  • Update clojurescript to r2227, lein-cljsbuild to 1.0.3
  • `lein cljsbuild once`

Result:

  • Multiple warnings
  • Generated "buggy.js" file contains only node.js shebang

Fix:

  • The simplest patch (attached) is to rebind `out` to `err` when emitting a warning
  • A better fix would be to write a function for error printing that wraps this idiom
  • Ideally, cljs/nodejs.js should have a `goog.provides` line added to prevent the warning in the first place


 Comments   
Comment by David Nolen [ 02/Jun/14 7:45 AM ]

A patch that adds a goog.provide to the file is preferred, thanks!





[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-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-805] add-watch returns map of watch fns instead of watched reference Created: 09/May/14  Updated: 09/May/14  Resolved: 09/May/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


 Description   
ClojureScript:cljs.user> (def a (atom nil))
#<Atom: nil>
ClojureScript:cljs.user> (add-watch a :foo nil)
{:foo nil}
ClojureScript:cljs.user> (add-watch a :bar nil)
{:bar nil, :foo nil}

In Clojure add-watch returns the watched reference.



 Comments   
Comment by David Nolen [ 09/May/14 8:27 AM ]

fixed https://github.com/clojure/clojurescript/commit/2406ba841db4776cfaa59eb37bec2e8ae543c466





[CLJS-804] Binding *print-length* breaks str Created: 07/May/14  Updated: 10/May/14  Resolved: 10/May/14

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

Type: Defect Priority: Major
Reporter: Adam Clements Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

clojurescript 0.0-2202



 Description   

(binding [*print-length* 10] (str {:foo "bar"}))

Breaks with an IMapEntry -key method unimplemented



 Comments   
Comment by David Nolen [ 10/May/14 2:40 PM ]

fixed https://github.com/clojure/clojurescript/commit/337d30cf3a98a0c28f658e230855fc2e09abdeaa





[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-802] Enable generatePseudoNames compiler option for advanced debugging Created: 30/Apr/14  Updated: 05/May/14  Resolved: 05/May/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-802-Add-pseudo-names-compiler-option.patch    

 Description   

The closure compiler option generatePseudoNames [1] can be used to generate readable names in advanced compilation.

a call 

(get {.. ..} :some-kw)

would be emitted like

$cljs$core$get$$.$cljs$core$IFn$_invoke$arity$2$($map__11005__$1$$inline_1045$$, $cljs$core$constant$0keyword$072$$);

this is a great aid when debugging issues in advanced mode, hence a clojurescript compiler option should be added.

[1] http://closure-compiler.googlecode.com/svn/trunk/javadoc/com/google/javascript/jscomp/CompilerOptions.html#generatePseudoNames



 Comments   
Comment by Herwig Hochleitner [ 30/Apr/14 8:01 AM ]

Patch adds :pseudo-names compiler option

Comment by David Nolen [ 05/May/14 5:03 PM ]

fixed https://github.com/clojure/clojurescript/commit/4381c04cce82fc3f8e8bc41e8e0370b8ef0b1065





[CLJS-801] str macro emits unoptimizable js code Created: 27/Apr/14  Updated: 11/May/14  Resolved: 11/May/14

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

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: string
Environment:

r2202


Attachments: Text File cljs-801.patch     Text File cljs-801-v2.patch     Text File cljs-801-v3.patch     Text File cljs-801-v4.patch    
Patch: Code and Test

 Description   

Clojurescript's str macro emits javascript code which is inefficient and which the closure compiler cannot optimize.

Currently it emits code like {{[cljs.core.str(arg1),cljs.core.str(arg2)].join('')}}. The problems with this:

  1. The emitted function is the arity-dispatch str wrapper instead of the 1-arg implementation of str. The closure compiler cannot eliminate the dispatch.
  2. An intermediate array is always created; the closure compiler cannot optimize it out.
  3. The closure compiler can evaluate constant string expressions (e.g. 'a'+1 to 'a1'), but cannot in this case because it cannot eliminate the str call.

The attached patch rewrites the str macro to generate js code that looks like this:

(str arg1 "constant" \space true nil 123 456.78)

(""+cljs.core.str.cljs$core$IFn$_invoke$arity$1(arg1)+"constant "+true+123+456.78)

This has a number of benefits:

  1. No short-lived array or Array.join operation.
  2. No arity dispatch is invoked. I have also observed that it can (but won't necessarily) inline the function body.
  3. The compiler can perform constant evaluation. For example, in advanced mode the compiler will emit the above example as (""+w.c(a)+"constant true123456.78") where w.c is the munged cljs.core.str.cljs$core$IFn$_invoke$arity$1


 Comments   
Comment by Francis Avila [ 28/Apr/14 12:17 AM ]

Updated patch adds booleans to the str test case, and does not eagerly stringify bools anymore. (It emits as literals and lets the closure compiler decide to stringify at compile time if it wants.)

Comment by David Nolen [ 10/May/14 2:23 PM ]

Need the patch rebased to master.

Comment by Francis Avila [ 11/May/14 1:38 AM ]

Updated patch.

Comment by David Nolen [ 11/May/14 10:52 AM ]

Sorry because of the last commit to master this ticket will not apply, please rebase again. I'll refrain from adding tests until this one gets merged in

Comment by Francis Avila [ 11/May/14 11:22 AM ]

No problem, updated patch.

Comment by David Nolen [ 11/May/14 11:27 AM ]

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





[CLJS-800] Printing PersistentQueueSeq causes StackOverflowError Created: 18/Apr/14  Updated: 18/Apr/14  Resolved: 18/Apr/14

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

Type: Defect Priority: Minor
Reporter: Chad Taylor Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

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

 Description   

A cljs.core.PersistentQueueSeq is generated correctly, but printing it causes a java.lang.StackOverflowError. This can be verified in the REPL:

(rest (conj cljs.core.PersistentQueue.EMPTY 1 2 3))  ;; prints stack trace

(map #(* % %) (rest (conj cljs.core.PersistentQueue.EMPTY 1 2 3))) ;; => (4 9)

Cause: PersistentQueueSeq does not implement IPrintWithWriter protocol

Solution: Provide an implementation of IPrintWithWriter for PersistentQueueSeq

Patch: CLJS-800.patch



 Comments   
Comment by Chad Taylor [ 18/Apr/14 1:00 AM ]

Added patch with test and code.

Comment by David Nolen [ 18/Apr/14 8:30 AM ]

fixed https://github.com/clojure/clojurescript/commit/2907190e5414fd53a0e0a07424f342360eb31ed9





[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-798] regression: upstream dependencies are no longer honored Created: 16/Apr/14  Updated: 20/May/14  Resolved: 20/May/14

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

Type: Defect Priority: Major
Reporter: Toby Crawley Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: File restore-upstream-deps.diff    

 Description   

The latest cljs no longer honors upstream deps. This is a regression introduced by https://github.com/clojure/clojurescript/commit/e16a3da. The js-dependency-index is now calculated once, before the upstream deps are loaded. One potential fix is to regenerate the index after loading the upstream deps, but I'm not familiar enough with the compiler to know if that's a bad idea. I'll attach a patch with that change for review.



 Comments   
Comment by David Nolen [ 17/Apr/14 11:03 AM ]

Out of curiosity are you relying on this functionality and in what way?

Comment by Toby Crawley [ 17/Apr/14 11:48 AM ]

I currently use a deps.cljs in https://github.com/vert-x/mod-lang-clojure - it provides a cljs library that relies on two javascript libraries, and I want users to be able to use my cljs library without having to specify its upstream js deps as configuration to their cljs build. If there is another way to achieve that, I'm open to suggestions, especially given that deps.clj is experimental.

Comment by David Nolen [ 20/May/14 7:38 PM ]

fixed https://github.com/clojure/clojurescript/commit/65f9d988f915cab0bff9c961e85495400ee1e542





[CLJS-797] Nested 'for' loops (3+ deep) in Android Chrome cause "Uncaught RangeError: Maximum call stack size exceeded" Created: 16/Apr/14  Updated: 17/Apr/14

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

Type: Defect Priority: Minor
Reporter: John M. Newman III Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Android 4.3, Chrome 34, ClojureScript 2202



 Description   
(do (println "for loop test: 2 deep")
  (for [a [[1]]]
    (for [b a]
      b)))
;; this compiles and runs fine in the browser

(do (println "for loop test: 3 deep")
  (doall
   (for [a [[[1]]]]
     (for [b a]
       (for [c b]
         c)))))
;; this fails while the page loads, with the error: Uncaught RangeError: Maximum call stack size exceeded

The above works fine in a desktop browser. For some reason the error condition only happens on the Android Chrome browser.

Let me know if any further details are required.






[CLJS-796] (get [42] nil) => 42 Created: 11/Apr/14  Updated: 14/Apr/14  Resolved: 14/Apr/14

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

Type: Defect Priority: Major
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug
Environment:

r2156



 Comments   
Comment by Francis Avila [ 12/Apr/14 9:23 PM ]

This is a duplicate of CLJS-728 and is fixed by this commit, which is included in r2197 and above.





[CLJS-795] Enhance multimethod performance Created: 11/Apr/14  Updated: 14/Apr/14  Resolved: 14/Apr/14

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

Type: Enhancement Priority: Major
Reporter: Jonas Enlund Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File CLJS-795.diff    

 Description   

Multimethods can be made more performant by implementing (the long list of) IFn methods. There is one benchmark (the last one) covering multimethods in script/benchmark and I saw a drop from ~250msecs to ~25msecs with the suggested changes.

If it seems to be too much repetition I'm sure its possible to throw some macros at the implementation to make most of the boilerplate disappear.



 Comments   
Comment by David Nolen [ 14/Apr/14 4:29 PM ]

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





[CLJS-794] RegExp flags are being dropped by `string/replace` Created: 09/Apr/14  Updated: 02/Dec/14

Status: Open
Project: