<< Back to previous view

[CLJS-866] Faulty ns macro desugaring Created: 01/Oct/14  Updated: 02/Oct/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-2356, Clojure 1.7.2-alpha2 (but I believe this has been around for some time)



 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





[CLJS-865] js-dependency-index won't work when running from an uberjar Created: 26/Sep/14  Updated: 29/Sep/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: 0
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`)





[CLJS-864] ClojureScript's 'and' does not always short-circuit Created: 24/Sep/14  Updated: 25/Sep/14

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

Type: Defect Priority: Major
Reporter: Zachary Kanfer Assignee: Unassigned
Resolution: Unresolved 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





[CLJS-863] Invalid arity error when calling multimethod with 0-arity dispath fn Created: 24/Sep/14  Updated: 24/Sep/14

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

Type: Defect Priority: Minor
Reporter: Immo Heikkinen Assignee: Unassigned
Resolution: Unresolved 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.






[CLJS-862] re-pattern is inconsistent with Clojure Created: 19/Sep/14  Updated: 19/Sep/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: 0
Labels: None

Attachments: Text File 862.patch    

 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.





[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: 16/Sep/14

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

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

Attachments: Text File 0001-change-deftype-defrecord-special-forms-to-include-th.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.






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

Status: Resolved
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: 10/Sep/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: bug, metadata
Environment:

Clojure: 1.7.0-alpha2
ClojureScript: 0.0-2322
tools.reader: 0.8.8



 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.





[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: 06/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: 1
Labels: None


 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.





[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: 31/Aug/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!





[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: 13/Aug/14

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

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






[CLJS-841] cljs.closure/build file locks Created: 13/Aug/14  Updated: 22/Aug/14

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

Type: Defect Priority: Major
Reporter: joerupen Assignee: Unassigned
Resolution: Unresolved 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.





[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: 08/Aug/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


 Description   

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






[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: 10/Sep/14  Resolved: 10/Sep/14

Status: Resolved
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: 13/Jun/14

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

Type: Defect Priority: Major
Reporter: Jamie Brandon Assignee: Unassigned
Resolution: Unresolved 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.






[CLJS-814] clojure.string/reverse breaks surrogate pairs Created: 12/Jun/14  Updated: 12/Jun/14

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

Type: Defect Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: 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.





[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/May/14

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

Type: Defect Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: 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.






[CLJS-806] support ^:const Created: 09/May/14  Updated: 09/May/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   

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






[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: 05/May/14

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

Type: Defect Priority: Major
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: 16/Apr/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.





[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: 09/Apr/14

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

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


 Description   

`clojure.string/replace` accepts either a string or pattern argument to match against.

For pattern arguments, the current implementation discards the original RegExp and creates a new one:
`(.replace s (js/RegExp. (.-source match) "g") replacement)`

This is killing any flags on the original pattern (case insensitivity, for example). As a result, things like `(str/replace "Foo" #"(?i)foo" "bar")` currently fail. The result is "Foo", it should be "bar".

Can I submit a patch that'll check for and preserve other (i/m/y) flags?

Thanks






[CLJS-793] memoize doesn't correctly cache non-truthy return values Created: 08/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: Peter Taoussanis Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-793.patch     Text File CLJS-793.patch     Text File CLJS-793.patch     Text File patch_commit_2ba4dac94918.patch    
Patch: Code

 Description   

ClojureScript's memoize fn currently uses `(get @mem args)` to check for the existence of a cache entry. This prevents falsey values from being cached correctly.

A direct copy of Clojure's `memoize` code fixes this, patch attached.

This is the first issue+patch I've submitted, so please double check for mistakes - thanks.



 Comments   
Comment by David Nolen [ 08/Apr/14 9:23 AM ]

The patch is not properly formatted - please follow these instructions http://github.com/clojure/clojurescript/wiki/Patches

Comment by Peter Taoussanis [ 08/Apr/14 9:41 AM ]

Updated patch formatting, thanks!

Comment by David Nolen [ 08/Apr/14 10:55 AM ]

Thanks, I've updated the instructions to be clearer on what the commit message should look like. We've been inconsistent in the past and I would like for people to follow a basic guideline. Thanks.

Comment by Peter Taoussanis [ 08/Apr/14 11:01 AM ]

No problem, thanks! Just updated to fit the new spec exactly.

Comment by David Nolen [ 08/Apr/14 7:37 PM ]

Looking at this more closely I would prefer that this be done with a closed over sentinel value via (js-obj), and to perform an identical? check on (get @mem args sentinel) to see if it matches the sentinel.

Comment by Peter Taoussanis [ 09/Apr/14 12:18 AM ]

Sure, updated the patch. Please note that I used the global `lookup-sentinel` instead of closing over a new one. Let me know if there was some reason you wanted a unique sentinel & I'll update!

Cheers

Comment by David Nolen [ 10/Apr/14 12:23 PM ]

Looks good thanks!

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

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





[CLJS-792] cljs.core.reducers/reduce does not support cljs.core/PersistentArrayMap Created: 07/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: Shahar Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File CLJS-792.diff    

 Description   
((require '[clojure.core.reducers :as r])
(into [] (r/map identity {}))
;; Clojure: []
;; ClojureScript: Error: No protocol method IReduce.-reduce defined for type cljs.core/PersistentArrayMap: {}


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

Thanks for the patch, have you submitted a CA? Thanks!

Comment by Jonas Enlund [ 14/May/14 6:03 AM ]

Yes, as the author of the patch I have submitted the CA.

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

fixed https://github.com/clojure/clojurescript/commit/76cf7c22cc99950029b0b023cd6996367d4a742a





[CLJS-791] Cross-target ClojureScript to the DartVM. Created: 06/Apr/14  Updated: 08/Apr/14  Resolved: 08/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Yesudeep Mangalapilly Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cross-target, dart, enhancement, vm
Environment:

Any.



 Description   

ClojureScript is a better language than many others, I figure.
The DartVM is picking up speed. Can CLJS be retargetted to generate
Dart code as well?



 Comments   
Comment by David Nolen [ 08/Apr/14 9:22 AM ]

Queries like this are best directed toward the mailing list before opening tickets. Thanks.





[CLJS-790] Advanced compilation broken with latest closure libra Created: 04/Apr/14  Updated: 03/Jul/14  Resolved: 03/Jul/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None
Environment:

cljs >= 2197


Attachments: Text File cljs-790.patch     Text File cljs-790-updated.patch    

 Description   

CLJS releases >= 2197 are depending on the most recent packaged closure lib [org.clojure/google-closure-library "0.0-20140226-71326067"].

This is bad,



 Comments   
Comment by Francis Avila [ 04/Apr/14 3:46 PM ]

ARG, submitted too soon by accident.

Rundown is this:

  1. CLJS >= 2197 depends on the latest packaged closure lib (March 17th release, "0.0-20140226-71326067").
  2. This closure lib version is incompatible with the closure compiler closurescript depends on at least because of changes to how goog.base works. See commit 5bdb54d221dbf7c6177ba5ba6901c012981501ec on the closure compiler library (and many more after this one with a similar commit message).
  3. When you advanced-compile a cljs project, goog classes which internally use the goog.base(this, 'superMethod') form will all be munged incorrectly and throw exceptions. Minimal test case: (ns myproj (:require goog.net.XhrIo)) (goog.net.XhrIo/send "http://example.org" #(js/console.log %)). This will fail at a "disposeInternal" call. (Notice that the output js still has the "disposeInternal" string!)
  4. Oddly, the compiler does not emit any warnings about this!
  5. Additionally, the clojurescript project's POM and project.clj specify different versions of the closure library starting at https://github.com/clojure/clojurescript/commit/523a0c5b18138c9a4d23c5104a77b65488bc28c3 The POM specifies the new lib, but the project.clj the older one.

A workaround is to declare an explicit dependency in your project to [org.clojure/google-closure-library "0.0-20130212-95c19e7f0f5f"]. If you do this everything will start working again.

Basically, cljs can't use the new closure lib until its closure compiler has an updated release. (There is some hope that this may be easier to do in the future: https://groups.google.com/d/msg/closure-compiler-discuss/tKZQ1eLUixA/3urgnli84SYJ)

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

Has this been resolved by a new closure compiler release yet? Thanks!

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

Donno, will give v20140407 release a try tonight.

Comment by Francis Avila [ 13/May/14 8:13 AM ]

Seems to work. Ran the test suite, benchmark, and some other projects I had which were failing before. Patch attached with updated dependencies.

Comment by Stephen Nelson [ 17/Jun/14 10:21 PM ]

This bug (or a similar one) is still affecting clojurescript 0.0-2234. Another example:

(ns myproj
(:require goog.net.XhrManager))
(.send (goog.net.XhrManager.) "xhr-request" "myproj.js" "GET" nil nil nil #(.log js/console %))

The workaround suggested above (using the old closure library) works, but versions of clojurescript newer than 2197 cannot be used in production without the workaround. If this bug is going to remain open in new releases, please consider documenting the workaround in the release notes.

Comment by David Nolen [ 18/Jun/14 10:42 AM ]

Can we get a new patch with a more recent release? http://search.maven.org/#artifactdetails%7Ccom.google.javascript%7Cclosure-compiler-parent%7Cv20140508%7Cpom

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

Bumped version. Note that this patch is unnecessary in the 1.6.0 branch, as its versions are up to date and its pom and project.clj agree. This commit is likely conflict when 1.6.0 is merged into master (the master side should be ignored if that happens).

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

this should be resolved in master

Comment by Francis Avila [ 03/Jul/14 1:55 PM ]

It is. You can close.

Comment by David Nolen [ 03/Jul/14 2:10 PM ]

fixed in master with 1.6.0 work merge





[CLJS-789] Advanced compilation broken with latest closure libra Created: 04/Apr/14  Updated: 22/Apr/14  Resolved: 22/Apr/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

cljs >= 2197



 Description   

CLJS releases >= 2197 are depending on the most recent packaged closure lib [org.clojure/google-closure-library "0.0-20140226-71326067"].

This is bad,



 Comments   
Comment by Francis Avila [ 04/Apr/14 3:50 PM ]

I have no idea how this was submitted both too soon and twice! I'm very sorry for the mess. Please mark duplicate of CLJS-790 and close.





[CLJS-788] spurious namespace warnings about goog namespaces in 2197 Created: 31/Mar/14  Updated: 01/Apr/14  Resolved: 01/Apr/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


 Comments   
Comment by David Nolen [ 01/Apr/14 1:35 PM ]

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





[CLJS-787] cljs.reader does not read blank string as nil Created: 20/Mar/14  Updated: 08/May/14  Resolved: 08/May/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: File CLJS-787.diff    

 Comments   
Comment by David Nolen [ 20/Apr/14 6:23 PM ]

Several tests fail when I apply this patch.

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

fixed https://github.com/clojure/clojurescript/commit/279157ac526f7aa0b01b95091821491f574024eb





[CLJS-786] cljs.core/into doesn't preserve metadata Created: 17/Mar/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: Minor
Reporter: Kevin Marolt Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

cljs.core/into doesn't preserve metadata:

(meta (into ^{:foo :bar} [] [1 2 3])) ;; => nil


 Comments   
Comment by Chas Emerick [ 19/Mar/14 9:15 AM ]

Confirmed. Both TransientVector and TransientHashMap need to have meta fields added that are passed on when instances are made persistent!.

Comment by Kevin Marolt [ 19/Mar/14 9:33 AM ]

Not sure what's the preferred way to handle this. The transients in Clojure don't support metadata either. Instead, Clojure simply uses

(defn into
  "Returns a new coll consisting of to-coll with all of the items of
   from-coll conjoined."
  {:added "1.0"
   :static true}
  [to from]
  (if (instance? clojure.lang.IEditableCollection to)
    (with-meta (persistent! (reduce conj! (transient to) from)) (meta to))
    (reduce conj to from)))

See also [#CLJ-916] into loses metadata.

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

with transducers we've since adopted the Clojure approach





[CLJS-785] :refer-macros in conjunction with :refer not working Created: 17/Mar/14  Updated: 17/Mar/14

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

Type: Defect Priority: Minor
Reporter: Kevin Marolt Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

The :refer-macros directive isn't working when used in conjunction with the :refer directive. Compiling a ns-form like

(ns foo
  (:require
    [bar :refer [baz] :refer-macros [quux]]))

produces the compiler error

Each of :as and :refer options may only be specified once in :require / :require-macros; offending spec: (bar :refer [baz] :refer [quux])

The problem seems to be with analzyer.cljs/desugar-ns-specs. Invoking

(desugar-ns-specs '((:require [bar :refer [baz] :refer-macros [quux]])))

returns

'((:require-macros (bar :refer [baz] :refer [quux])) (:require (bar :refer [baz])))

instead of

'((:require-macros (bar :refer [quux])) (:require (bar :refer [baz])))

Furthermore, there seems to be a typo in the local remove-sugar function on line 1094 [1]: It should probably be

(let [[l r] (split-with ...)] ...) ;; no '&' before 'r'

instead of

(let [[l & r] (split-with ...)] ...)

Otherwise, something like

(desugar-ns-specs '((:require [bar :include-macros true :as b])))

becomes

((:require-macros (bar :as b)) (:require (bar)))

instead of

((:require-macros (bar :as b)) (:require (bar :as b)))

[1] https://github.com/clojure/clojurescript/blob/78d20eebbbad17d476fdce04f2afd7489a507df7/src/clj/cljs/analyzer.clj#L1094






[CLJS-784] PersistHashMap's -conj implementation recurses infinitely if element to be conjed is not a vector. Created: 15/Mar/14  Updated: 08/May/14  Resolved: 08/May/14

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

Type: Defect Priority: Minor
Reporter: Alex Coventry Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: errormsgs
Environment:

Happens on cljsfiddle, among other environments.


Attachments: Text File 0001-CLJS-784-make-conj-on-maps-behave-as-it-does-in-Cloj.patch     Text File 0002-CLJS-784-Fix-Map.-conj-for-map-entry-seqs-that-don-t.patch     Text File 0003-CLJS-784-use-reduce-in-conj-on-maps.patch    

 Description   

In commit b8681e8 the implementation is

ICollection
  (-conj [coll entry]
    (if (vector? entry)
      (-assoc coll (-nth entry 0) (-nth entry 1))
      (reduce -conj coll entry)))

Thus, e.g., (-conj {} "foo") results in an infinite recursion, and a stack overflow. This causes things like (merge {} "foo") to fail for the same reason.

Not sure what the purpose of the not-vector branch could be. I can't think of a situation where it would give a useful result. Maybe it could throw a more helpful error message.



 Comments   
Comment by Michał Marczyk [ 22/Apr/14 6:13 AM ]

This actually applies to all three map types. (In fact, in (-conj {} "foo"), the map is an array map.) In Clojure, conj on a map works with a number of argument types:

1. map entries;

2. two-element vectors;

3. seqables of map entries.

The final case is, perhaps surprisingly, the oldest one. Merging maps falls under it, since for map arguments it boils down to merge minus special treatment of nil (merge uses conj to merge pairs of maps); but arbitrary seqables of map entries are supported. (NB. these must be actual map entries, not two-element vectors!) This allows one, for example, to filter a map and conj the result of that into another map.

So, we want to support the legitimate use cases while maybe complaining about code that wouldn't work in Clojure if it's not too much of a problem performance-wise. An example of a call that we'd probably like to throw: {{(conj {} (list (list [:foo 1])))}}.

The attached patch makes the -conj implementations in all the map types use an explicit loop in the non-vector branch and adds some test for the resulting behaviour.

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

fixed https://github.com/clojure/clojurescript/commit/3d4405b9b22d36e2e686a084c54ae3f6e5a6208a

Comment by Herwig Hochleitner [ 06/May/14 6:11 AM ]

With patch 0001 (3d4405b), map conj fails for seqs, that don't implement -next.

(merge {:a 1} (hash-map :b 2))
;;=> Error: No protocol method INext.-next defined for type cljs.core/NodeSeq: ([:b 2])
...
at cljs.core.PersistentArrayMap.cljs$core$ICollection$_conj$arity$2 (http://localhost:6030/:15569:42)
...
Comment by Herwig Hochleitner [ 06/May/14 6:46 AM ]

I guess implementing INext is not part of the contract for ISeqable.-seq, which means NodeSeq doesn't have to implement it, right?
In that case, the right fix is to use next instead of -next inside of Map.-conj, when dealing with a (possibly user defined) seq of MapEntries.

Attached patch 0002 uses next instead of -next and adds tests for map-entry seqs not implementing INext

Comment by Michał Marczyk [ 06/May/14 3:04 PM ]

Good catch, thanks!

Another approach would be to use reduce, hopefully benefiting from IReduce speed boosts. Of course we'd need to use a custom reduction function wrapping -conj with a vector? check. The attached patch implements this.

Comment by Michał Marczyk [ 06/May/14 3:49 PM ]

Actually, scratch the part about IReduce speed boosts – sorry for the confusion!

Having run better benchmarks with the two patches on a recent build of V8 and I have to say that there doesn't seem to be much of a difference and actually the next-based approach comes out ahead sometimes. In Clojure, a hand-rolled loop-based "map-seq-conj" loses to a hand-rolled reduce-based impl consistently, as far as I can tell, although only by ~3-5%. I've been conj-ing seqs over vectors of vectors, which should be friendly to reduce.

Comment by Herwig Hochleitner [ 08/May/14 4:10 AM ]

Despite no direkt speed boost in benchmarks, I'm fond of using reduce here. GC Pressure is hard to benchmark.

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

going to go with the next based patch.

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

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





[CLJS-783] Confusing error messages when ns compilation fails due to a missing dependency Created: 11/Mar/14  Updated: 17/Apr/14  Resolved: 17/Apr/14

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

Type: Defect Priority: Minor
Reporter: Michael Klishin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: error-reporting, errormsgs, usability


 Description   

I have a namespace proj.a which requires proj.b. proj.b, in turn, relies on core.async. I did not have
core.async listed in project.clj by accident, and the resulting error message was

goog.require could not find: proj.b.

This is not incredibly helpful. I've wasted over an hour trying to understand why one ns in my project
cannot reference another one.

Expected outcome: compilation must fail instead of swallowing exceptions. If it matters, I use lein-cljsbuild 1.0.2.



 Comments   
Comment by David Nolen [ 12/Mar/14 8:36 AM ]

And which version of ClojureScript are you using?

Comment by Michael Klishin [ 12/Mar/14 8:52 AM ]

0.0-2138

Comment by Michael Klishin [ 12/Mar/14 8:53 AM ]

I strongly disagree with the severity change. Anything that can waste beginners hours of time is not a minor priority.

Comment by David Nolen [ 12/Mar/14 10:18 AM ]

That is a fairly old release of ClojureScript, can you replicate the issue with 0.0-2173? When you change your dependency please make sure to run "lein cljsbuild clean" first.

Comment by David Nolen [ 17/Apr/14 3:53 PM ]

Closing unless I hear step on how to reproduce this in more recent ClojureScript releases. Feel free to request a re-open if you can demonstrate that this isn't resolved.





[CLJS-782] cljs.core.UUID should have a toString method Created: 10/Mar/14  Updated: 12/Mar/14  Resolved: 12/Mar/14

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

Type: Enhancement Priority: Trivial
Reporter: James Reeves Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Sometimes it's useful to turn a UUID into a string, particularly when interoperating with Javascript. Currently the toString implementation for UUIDs is quite opaque:

=> (str #uuid "f47ac10b-58cc-4372-a567-0e02b2c3d479")
"[object Object]"

Mimicking the behavior in Clojure/Java would be preferable:

=> (str #uuid "f47ac10b-58cc-4372-a567-0e02b2c3d479")
"f47ac10b-58cc-4372-a567-0e02b2c3d479"


 Comments   
Comment by David Nolen [ 12/Mar/14 8:23 AM ]

fixed https://github.com/clojure/clojurescript/commit/2870101b1a4ad4ef70ff06df3c04cda5d621b2cc





[CLJS-781] ClojureScript Browser REPL analysis regression Created: 07/Mar/14  Updated: 07/Mar/14  Resolved: 07/Mar/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 [ 07/Mar/14 3:29 PM ]

Latest 2173 has an analysis regression that results in spurious warnings about cljs.core symbols that did not exist in 2156. I strongly suspect this is related to http://dev.clojure.org/jira/browse/CLJS-615. When that patch landed I tested only the behavior of the browser REPL from within the repo and did not test the behavior when used with CLJS dependencies in a project.

Comment by David Nolen [ 07/Mar/14 3:37 PM ]

After closer inspection this does appear to be a bug in ClojureScript itself, rather lein-cljsbuild's browser REPL support.





[CLJS-780] Incorrect behaviour of apply-to for (>= argc 6) Created: 03/Mar/14  Updated: 05/Mar/14  Resolved: 05/Mar/14

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

Type: Defect Priority: Major
Reporter: Kevin Marolt Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None


 Description   

The cljs.core/apply-to function exhibits incorrect behaviour when called with an "argument count" argc (the second argument to apply-to) greater than or equal to 6.

This is because the cljs.core/gen-apply-to macro produces (essentially) to following function:

(defn apply-to
  [f argc args]
  (let [args (seq args)]
    (if (== argc 0)
      (f)
      (let [[a & args] args]
        (if (== argc 1)
          (f a)
          (...
            (if (== argc 6)
              (let [[f % args] args] ;; f is being overshadowed
                (f a b c d e f)
                (if (== argc 7)
                  (let [[g & args] args]
                    (f a b c d e f g)
                    (...)))))))))))

For (>= argc 6) the f in

(defn apply-to [f argc args] ...)

is being overshadowed by the one in

(let [[f % args] args] ...)

The obvious solution would be to output something like

(defn apply-to
  [func argc args]
  (let [args (seq args)]
    (if (== argc 0)
      (func)
      (let [[a & args] args]
        (if (== argc 1)
          (func a)
          (...
            (if (== argc 6)
              (let [[f % args] args]
                (func a b c d e f)
                (if (== argc 7)
                  (let [[g & args] args]
                    (func a b c d e f g)
                    (...)))))))))))

The following changes to gen-apply-to-helper and gen-apply-to should do the trick:

(defn gen-apply-to-helper
  ([] (gen-apply-to-helper 1))
  ([n]
     (let [prop (symbol (core/str "-cljs$core$IFn$_invoke$arity$" n))
           func (symbol (core/str "cljs$core$IFn$_invoke$arity$" n))]
       (if (core/<= n 20)
         `(let [~(cs (core/dec n)) (-first ~'args)
                ~'args (-rest ~'args)]
            (if (core/== ~'argc ~n)
              (if (. ~'func ~prop)
                (. ~'func (~func ~@(take n cs)))
                (~'func ~@(take n cs)))
              ~(gen-apply-to-helper (core/inc n))))
         `(throw (js/Error. "Only up to 20 arguments supported on ifntions"))))))
         
(defmacro gen-apply-to []
  `(do
     (set! ~'*unchecked-if* true)
     (defn ~'apply-to [~'func ~'argc ~'args]
       (let [~'args (seq ~'args)]
         (if (zero? ~'argc)
           (~'func)
           ~(gen-apply-to-helper))))
     (set! ~'*unchecked-if* false)))


 Comments   
Comment by David Nolen [ 04/Mar/14 11:33 AM ]

apply-to is an internal helper do you have an example where this is a problem in practice? Thanks.

Comment by Kevin Marolt [ 04/Mar/14 5:26 PM ]

Sure, try this with :optimizations :none (I haven't tested it with other optimization settings):

(def a (atom {:foo (with-meta [] {:bar '(1 2 3)})}))
(swap! a update-in [:foo] vary-meta update-in [:bar] vector)
(prn (= @a {:foo (with-meta [] {:bar [1 2 3]})})) ;; prints "false"
(prn (= @a [{:foo (with-meta [] {:bar '(1 2 3)})}
            [:foo]
            vary-meta
            update-in
            [:bar
            vector])) ;; prints "true"

The goal was to vectorize the list '(1 2 3), but swap! is applying update-in to the arguments

{:foo (with-meta [] {:bar '(1 2 3)})} ;; a
[:foo]                                ;; b
vary-meta                             ;; c
update-in                             ;; d
[:bar]                                ;; e
vector                                ;; f

and thus, because vector is in sixth position (and therefore the f-argument), what's actually happening is that vector is incorrectly called with those same arguments instead.

Comment by David Nolen [ 05/Mar/14 8:51 AM ]

fixed https://github.com/clojure/clojurescript/commit/13cacc68b4c9816bb09ce048ca3eb6dcf44e3144





[CLJS-779] Permit omitting hashbang with nodes target. Created: 03/Mar/14  Updated: 04/Mar/14  Resolved: 04/Mar/14

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

Type: Enhancement Priority: Major
Reporter: Michael Glaesemann Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-779.patch    

 Description   

In some environments (such as parse.com's Code Cloud CLJS-722), the node.js hashbang should be omitted. Currently if the target is "nodejs", a hashbang will always be applied at the top of the file.



 Comments   
Comment by Michael Glaesemann [ 03/Mar/14 5:48 PM ]

Following brief discussion on CLJS-771 and <https://groups.google.com/d/msg/clojurescript/CuInr2L5yFo/562AIcRsrksJ>, I've worked up a patch that omits the hashbang if :hashbang false is added as a compiler option.

Another option would be to go back to the original behavior that if the :preamble option is provided, it assumes the necessary hashbang is provided in the preamble. I find this a bit unintuitive (which is why I provided a patch to allow preambles with the node.js target anyway), and think that this is a better, more explicit solution.

Comment by David Nolen [ 04/Mar/14 11:44 AM ]

fixed https://github.com/clojure/clojurescript/commit/00a9b7bde0c8823175560b453a00e7be09ddd250





[CLJS-778] Unable to call `set` on `RSeq`s Created: 03/Mar/14  Updated: 04/Mar/14  Resolved: 04/Mar/14

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

Type: Defect Priority: Minor
Reporter: Omri Bernstein Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript release 1798+
(Probably not relevant: Leiningen 2.3.4, Java 1.7.0_51, Ubuntu 12.04, Toshiba Satallite L755)



 Description   

Error when I run, at a REPL:

user> (set (reverse [0]))
"Error evaluating:" (set (reverse [0])) :as "cljs.core.set.call(null,cljs.core.reverse.call(null,new cljs.core.PersistentVector(null, 1, 5, cljs.core.PersistentVector.EMPTY_NODE, [0], null)))"
org.mozilla.javascript.JavaScriptException: Error: No protocol method INext.-next defined for type cljs.core/RSeq: (0) (cljs/core.cljs#266)
at cljs/core.cljs:266 (anonymous)
at cljs/core.cljs:260 (_next)
at cljs/core.cljs:6368 (set)
at <cljs repl>:1 (anonymous)
at <cljs repl>:1
nil

(Specifically, this is in a SublimeREPL Rhino REPL.)

The problem seems to happen for any `cljs.core/RSeq` types. I've tried loading different ClojureScript release versions. The problem seems to exist for releases 1798 and higher but not for 1586 and lower. If someone could explain how to load non-released ClojureScript commits, I would be happy to narrow the window even further.



 Comments   
Comment by David Nolen [ 04/Mar/14 11:41 AM ]

fixed https://github.com/clojure/clojurescript/commit/87deb8b080d0c930fe92a7a58c51f22a559033e5





[CLJS-777] multimethods should be ifn? Created: 01/Mar/14  Updated: 01/Mar/14  Resolved: 01/Mar/14

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

Type: Defect Priority: Major
Reporter: Jacob Maine Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

In regular Clojure, this passes

(defmulti mm :type)
(assert (ifn? mm))

But it fails in ClojureScript.

Note that (fn? mm) is false for Clojure, so this can't be fixed by adding the Fn marker protocol to MultiFn.



 Comments   
Comment by David Nolen [ 01/Mar/14 3:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/525154f2a4874cf3b88ac3d5755794de425a94cb





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

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

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


 Description   

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

Example in Clojure:

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

Compare Clojurescript:

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

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

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

Questions:

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


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

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

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

Essential points:

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

Thoughts?





[CLJS-775] cljs.reader parses radix form of int literals (e.g. 2r101) incorrectly Created: 27/Feb/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: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: reader

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

 Description   
ClojureScript:cljs.user> (cljs.reader/read-string "2r10")
"Error evaluating:" (cljs.reader/read-string "2r10") :as "cljs.reader.read_string.call(null,\"2r10\")"
org.mozilla.javascript.JavaScriptException: Error: Invalid number format [2r10] (file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js#107)
	at file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js:107 (anonymous)
	at file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js:112 (anonymous)
	at file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js:374 (read_number)
	at file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js:650 (read)
	at file:/Users/favila/wcs/clojurescript/.repl/cljs/reader.js:677 (read_string)
	at <cljs repl>:1 (anonymous)
	at <cljs repl>:1


 Comments   
Comment by Francis Avila [ 28/Feb/14 1:42 AM ]

Turns out other integer literals were broken besides radix due to a re-match problem (CLJS-776). Patch includes fix and tests for all the different integer literal forms.

Floats, ratios, and symbols/keywords might also have parsed incorrectly in certain cases, but I did not produce failing tests to confirm.

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

Can we get a new patch rebased on master? Thanks!

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

Rebased patch.

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

fixed https://github.com/clojure/clojurescript/commit/56ea020fd9b15df220ba247f73b68873f041d8ef





[CLJS-774] cljs.reader should js js/parseInt with radix specified Created: 27/Feb/14  Updated: 12/Mar/14  Resolved: 12/Mar/14

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

Type: Defect Priority: Major
Reporter: Tatu Tarvainen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-774.patch    

 Description   

In cljs.reader, the parse-int calls js/parseInt without a radix.

The radix is implementation defined if not specified.
This causes problems for example in older Android browsers which parse "08" as 0 instead of 8.

This makes reading timestamps fail. For example:
Trying to read: #inst "2013-07-08T21:00:00.000-00:00"
causes:
Error: timestamp day field must be in range 1..last day in month Failed: 1<=0<=31

in js console:
parseInt("08") => 0
parseInt("08", 10) => 8



 Comments   
Comment by Francis Avila [ 27/Feb/14 10:38 PM ]

Discovered integer-with-radix parsing problem but split it into CLJS-775 since it's not a simple parseInt issue.

This patch should fix ratio and inst parsing, but I don't have access to a browser which infers octals in parseInt.

Existing tests are sufficient to catch this problem for #inst.

There are no ratio tests for reader at all currently, but since ratios are shaky ground in cljs anyway I didn't add any.

Comment by Dave Della Costa [ 12/Mar/14 1:14 AM ]

Just chiming in to mention that I have experienced this issue in Internet Explorer 8 and was going to submit a patch myself. Would be interested in getting this into the next version as for now we have a customized reader that we use to get past this.

Comment by David Nolen [ 12/Mar/14 8:26 AM ]

fixed https://github.com/clojure/clojurescript/commit/004224511b4351acbfe051c7dea913fb0c183c04





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

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

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

r2173



 Description   

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

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



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

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





[CLJS-772] Enhance Node.js Support Created: 24/Feb/14  Updated: 24/Feb/14  Resolved: 24/Feb/14

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

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


 Description   

Google Closure now comes with a Node.js bootstrap script that makes require and provide work https://code.google.com/p/closure-library/wiki/NodeJS. This enhancement requires a newer version of Closure Library.



 Comments   
Comment by David Nolen [ 24/Feb/14 7:48 AM ]

fixed https://github.com/clojure/clojurescript/commit/0c7b31ada01237de33cef77b817ccef3f2b3576d





[CLJS-771] Allow nodejs target preambles Created: 21/Feb/14  Updated: 26/Feb/14  Resolved: 21/Feb/14

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

Type: Enhancement Priority: Major
Reporter: Michael Glaesemann Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-771.2.patch     Text File CLJS-771.patch    

 Description   

Currently targeting nodejs precludes the use of a preamble. There are cases where you might want to add arbitrary code (besides the node hashbang) at the top of the compiled file. (see brief discussion at https://groups.google.com/forum/#!topic/clojurescript/CuInr2L5yFo)



 Comments   
Comment by David Nolen [ 21/Feb/14 1:16 PM ]

Thanks but it appears your patch is dirty - it includes compiler version information. Will apply if we get a cleaned up version. Thanks!

Comment by Michael Glaesemann [ 21/Feb/14 1:23 PM ]

Updated (git -a is not always helpful).

Comment by David Nolen [ 21/Feb/14 1:40 PM ]

fixed http://github.com/clojure/clojurescript/commit/4b7cdd4fb3cfae1c2e325a78eae54de0cb164d78

Comment by Travis Vachon [ 26/Feb/14 12:59 PM ]

Arg, sorry I missed this - the problem is that not all node-like environments need a hashbang. As this code used to be it was up to the creator of the preamble to include the hashbang if they wanted one - the idea was that the hashbang was just the default preamble in a node environment.

I've replied to the list addressing the original motivation for this patch, but I'd very much like to see it reconsidered (except for the test, which is great! though of course I'd want it updated for the behavior I think is correct ;-D )

happy to submit a followup patch if needed

Comment by Michael Glaesemann [ 26/Feb/14 1:29 PM ]

That'd be an option, though it's also possible to set the value of the hashbang. Does :hashbang "" do what you want?

Comment by Michael Glaesemann [ 26/Feb/14 1:31 PM ]

Scratch that, it doesn't, does it. The code prepends #! to the path to node.





[CLJS-770] Include :js-dependency-index in result of `(env/default-compiler-env)` Created: 18/Feb/14  Updated: 23/Feb/14  Resolved: 23/Feb/14

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

Type: Defect Priority: Minor
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: File CLJS-770.diff    
Patch: Code

 Description   

It looks like the change from CLJS-615 requires the addition of something like this prior to using any of the dependency machinery:

(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but cljs.env/default-compiler-env is there for exactly this sort of thing: the environment that fn returns should have :js-dependency-index added already. This would necessitate pulling all of the JS dependency resolution machinery up into cljs.env (or perhaps another namespace, cljs.js-deps?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of cljs.closure and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?



 Comments   
Comment by David Nolen [ 18/Feb/14 8:11 AM ]

I'm ok with pulling this stuff into it's own namespace.

Comment by Chas Emerick [ 23/Feb/14 9:35 AM ]

Attached patch refactors goog-style JS dependency stuff into cljs.js-deps, and changes cljs.env/default-compiler-env to populate :js-dependency-index based on the provided set of options.

I didn't move the externs stuff, seemed like it should stay in cljs.closure.

Note that this doesn't change the fact that CLJS compiler API consumers will need to change: cljs.env/default-compiler-env now takes a map argument of options, which needs to be provided if such consumers need to have their e.g. :libs, :foreign-libs options picked up. The advantage is default-compiler-env again does the job it's intended to do. The refactoring / slimming of cljs.closure is a nice side effect, tho.

Comment by David Nolen [ 23/Feb/14 4:24 PM ]

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





[CLJS-769] clojure.core/unsigned-bit-shift-right not excluded in cljs.core Created: 17/Feb/14  Updated: 01/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: None

Attachments: File CLJS-769.diff    
Patch: Code

 Description   

EOM



 Comments   
Comment by Kyle VanderBeek [ 17/May/14 8:34 PM ]

This defect does cause a warning with lein-cljsbuild 1.0.3 under Clojure 1.6.0 and ClojureScript 0.0-2202.

WARNING: unsigned-bit-shift-right already refers to: #'clojure.core/unsigned-bit-shift-right in namespace: cljs.core, being replaced by: #'cljs.core/unsigned-bit-shift-right
Comment by Francis Avila [ 01/Jul/14 10:57 PM ]

This is fixed in r2261 (the clojure 1.6 release).





[CLJS-768] (persistent! (assoc! (transient [0]) nil 1)) => [1] Created: 17/Feb/14  Updated: 23/Feb/14  Resolved: 23/Feb/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: None
Environment:

r2156


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

 Description   

assoc!-ing a non-numeric index into a transient vector is the same as assoc!-ing index 0. This should throw an exception instead. Patch and tests attached.



 Comments   
Comment by David Nolen [ 20/Feb/14 11:41 AM ]

This patch no does not cleanly apply on master. Can we get a new patch? Thanks.

Comment by Francis Avila [ 21/Feb/14 9:31 AM ]

Rebased patch.

Comment by David Nolen [ 23/Feb/14 4:30 PM ]

fixed https://github.com/clojure/clojurescript/commit/8419d8aeb6d2e975029fc693f96137775d2442c6





[CLJS-767] vector and subvector implement assoc-n with assoc; (assoc [0] nil 1) -> [1] Created: 17/Feb/14  Updated: 20/Feb/14  Resolved: 20/Feb/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: None
Environment:

r2156


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

 Description   

PersistentVector and Subvec implement their IVector protocol (-assoc-n) with IAssociative (-assoc); it should be the other way around:

  1. Because IVector has an additional numeric index contract that IAssociative does not. The neglected number check causes (assoc [0] nil 1) to return [1] instead of throwing an exception.
  2. Because Java Clojure does not do this (APersistentVector.assoc does a numeric check then calls PersistentVector.assocN).

See comments in CLJS-728 for more detailed rationales.

Patch attached which reverses this, and also fixes the error when a non-number is used as a key/index to assoc on PersistentVector and Subvec. Note: (assoc! (transient [0]) nil) is still broken.



 Comments   
Comment by David Nolen [ 20/Feb/14 11:40 AM ]

fixed, https://github.com/clojure/clojurescript/commit/a7f7ea3b5be1dd450d5503766024cbde9c731147





[CLJS-766] Fix broken NodeJS samples Created: 16/Feb/14  Updated: 20/Feb/14  Resolved: 20/Feb/14

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

Type: Defect Priority: Minor
Reporter: Jeff Dik Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Broken by CLJS-722



 Comments   
Comment by David Nolen [ 20/Feb/14 11:47 AM ]

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





[CLJS-765] Subvec should implement IReversible Created: 16/Feb/14  Updated: 16/Feb/14  Resolved: 16/Feb/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-765-implement-IReversible-for-Subvec.patch    

 Description   

Reported by Sunil Nandihalli in this thread:

https://groups.google.com/d/msg/clojure/1XkYDB-FDMA/AAWHMukms8MJ

rseq doesn't work on Subvec, because no implementation of IReversible is provided.

Patch forthcoming.



 Comments   
Comment by Michał Marczyk [ 16/Feb/14 1:54 PM ]

Oops, didn't include tests in the last patch. Same fix + tests this time.

Comment by David Nolen [ 16/Feb/14 2:46 PM ]

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





[CLJS-764] repl/evaluate-form should pass along file-information Created: 16/Feb/14  Updated: 20/Feb/14  Resolved: 20/Feb/14

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

Type: Defect Priority: Major
Reporter: Gary Trakhman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

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

 Description   

I was getting missing file info for core defs in a repl context, where normally (a full compile) would show the correct information.

This is required for repl-level file-metadata, which cider needs for jump-to-definition.
https://github.com/clojure-emacs/cider/issues/462

With the current patch, cider can jump through cljs files and jar resources correctly.



 Comments   
Comment by Gary Trakhman [ 16/Feb/14 9:44 AM ]

fix and repl-based test

Comment by Gary Trakhman [ 16/Feb/14 9:00 PM ]

In piggieback, load-stream seems to miss file/line for transitive requires, but it looks fine when I try to replicate it in a cljs test. Will work on it.

Comment by Gary Trakhman [ 19/Feb/14 12:40 PM ]

Further testing shows that file+line is indeed making it through, I think the current patch is enough.

Comment by David Nolen [ 20/Feb/14 11:43 AM ]

Because of whitespace changes it's impossible to see what changed. Can you explain the changes? Thanks!

Comment by Gary Trakhman [ 20/Feb/14 12:09 PM ]

The relevant line is '(binding [ana/*cljs-file* filename]'.

It sets the var so code down the line can pick it up.

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

fixed https://github.com/clojure/clojurescript/commit/953819ff7e4250e593489445ae650856cd0ce1b7





[CLJS-763] Docstring does not mention argument names in description. Created: 13/Feb/14  Updated: 14/Feb/14  Resolved: 14/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Ryan Mulligan Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

http://clojuredocs.org/clojure_core/clojure.core/rem

The arguments to rem are named num and div and the docstring refers to numerator and denominator.



 Comments   
Comment by Ryan Mulligan [ 14/Feb/14 9:30 AM ]

Sorry, I meant to report this against the Clojure project, not the Clojurescript one. I recommend closing this! Thanks.





[CLJS-762] "goog.require could not find: cljs_gwt.css" since compiler generates wrong code Created: 06/Feb/14  Updated: 08/Feb/14  Resolved: 08/Feb/14

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

Type: Defect Priority: Major
Reporter: Sergey Stupin Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

[org.clojure/clojure "1.5.1"]
[org.clojure/clojurescript "0.0-2156"]

OSX 10.9.1
Chrome Version 32.0.1700.102



 Description   

The project for which the problem occurred is here https://github.com/hsestupin/cljs-gwt.
Steps to reproduce the problem:
1) from one console: lein ring server
2) from other console: lein cljsbuild clean & lein cljsbuild auto dev
3) http://localhost:3000/ in Google Chrome with dev-tools console. There will be an error: "goog.require could not find: cljs_gwt.css"

It was happened because of cljs/cljs_gwt/core.cljs is beginning with lines

(ns cljs-gwt.core
  (:require [cljs-gwt.css :as css]))

And then ClojureScript compiler emits the following from previous 2 lines resources/public/js/cljs_gwt.js

goog.require("cljs_gwt.css");
goog.require("cljs_gwt.css");

instead of

goog.provide("cljs_gwt.css");
goog.require("cljs_gwt.css");

therefore an error occurred. That's it.



 Comments   
Comment by Francis Avila [ 06/Feb/14 4:15 PM ]

There is no goog.provide("cljs_gwt.css") emitted because there is no file that provides that namespace. Your css.clj file is misnamed--it should be css.cljs, otherwise clojurescript won't see it. (Also it has unbalanced parens and an illegal set!, so it won't compile anyway.)

There are two minor issues that you did hit upon, though:

  • You should have gotten a compiler warning for the missing namespace. I'm not sure why you didn't.
  • The cljs compiler may emit the same goog.require multiple times. This doesn't harm anything and the google closure compiler will remove them all.
Comment by Sergey Stupin [ 06/Feb/14 5:00 PM ]

Thanks for explanation, it helps a lot.





[CLJS-761] no method may be called on number literals Created: 02/Feb/14  Updated: 20/Feb/14  Resolved: 20/Feb/14

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

Type: Defect Priority: Minor
Reporter: Ryan Mulligan Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

http://himera.herokuapp.com/index.html
Chromium 32.0.1700.77 (244343)
Linux hostname 3.12.7-2-ARCH #1 SMP PREEMPT Sun Jan 12 13:09:09 CET 2014 x86_64 GNU/Linux



 Description   

Himera REPL v0.1.5
cljs.user> (.toString 1)
Compilation error: SyntaxError: Unexpected token ILLEGAL

The problem is that it compiles down to

1.toString()

and the parser interprets "1." as a literal float, followed by junk. This can be fixed with parenthesis around the literal:

(1).toString()



 Comments   
Comment by Ryan Mulligan [ 02/Feb/14 1:11 PM ]

At dc4ba2e this happens at the REPL started by following the instructions at https://github.com/clojure/clojurescript/wiki/Quick-Start

ClojureScript:cljs.user> (.toString 1)
"Error evaluating:" (.toString 1) :as "1.toString()"
org.mozilla.javascript.EvaluatorException: missing ; before statement (<cl
js repl>#1)

Comment by Francis Avila [ 03/Feb/14 10:36 AM ]

Duplicate of CLJS-715





[CLJS-760] Add an IAtom protocol Created: 01/Feb/14  Updated: 17/Feb/14  Resolved: 17/Feb/14

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

Type: Enhancement Priority: Minor
Reporter: Jamie Brandon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Add an IAtom protocol with a -reset! method and a fast path for Atom in cljs.core/reset!.

See jsperf here - http://jsperf.com/iatom-adv

Latest chrome and firefox versions suffer ~20-30% slowdown. Older firefox versions suffer up to 60-70%.



 Comments   
Comment by David Nolen [ 02/Feb/14 11:40 AM ]

This is not a properly formatted patch - http://github.com/clojure/clojurescript/wiki/Patches.

Comment by Jamie Brandon [ 02/Feb/14 11:51 AM ]

Fixed

Comment by David Nolen [ 02/Feb/14 6:33 PM ]

fixed https://github.com/clojure/clojurescript/commit/33692b79a114faf4bedc6d9ab38d25ce6ea4b295

Comment by Jozef Wagner [ 03/Feb/14 2:40 AM ]

What is the rationale behind this? Do you plan to have multiple Atom types?

Comment by David Nolen [ 03/Feb/14 8:13 AM ]

There's no good reason to lock down atom behavior to the one provided by ClojureScript.

Comment by Jozef Wagner [ 03/Feb/14 10:25 AM ]

I agree that such behavior should be polymorphic, I was just surprised that it is a pressing issue in a single threaded CLJS. Clojure itself does not provide such abstraction. Anyway, if you want to get it right, I suggest also to include swap! and compare-and-set! in the protocol (in order to not lock down the sharing and change policy), and to name it something like IMutableReference, as these methods are generic enough to be used outside atom, (as atom promises additional features besides those of a simple mutable reference). See https://gist.github.com/wagjo/8786305 for inspiration.

Comment by David Nolen [ 03/Feb/14 11:21 AM ]

I question the utility of compare-and-set! given most JS hosts are single threaded (but who knows? things are changing quickly in the JS world). It's something to consider in the future. IMutableReference is just more to type and I'm not really convinced it offers anything semantically over IAtom. Atoms do support more functionality but this is covered in other protocols.

Comment by David Nolen [ 03/Feb/14 11:21 AM ]

We need to include -swap! in the IAtom protocol.

Comment by Jozef Wagner [ 03/Feb/14 12:02 PM ]

The usefulness of the atom and protocols behind it should 'transcendent' the notion of single/multiple threads. Prime example is the Avout and its zk-atom and zk-ref. If e.g. implementing Avout in CLJS is a reasonable and useful thing to do, we should prepare for it by designing protocols to handle such use cases.

Comment by David Nolen [ 03/Feb/14 12:30 PM ]

Avout is an third party library which has no relation to the direction of ClojureScript. This ticket is a conservative change that opens up the door for extending atom-like capabilities to user types, that's it. Further enhancements may or may not come in the future.

Comment by David Nolen [ 17/Feb/14 1:49 PM ]

fixed https://github.com/clojure/clojurescript/commit/3e6564a72dc5e175fc65c9767364d05af34e4968





[CLJS-759] Exceptions are reported at weird locations in the presence of bad quasiquoting Created: 31/Jan/14  Updated: 31/Jan/14

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

Type: Defect Priority: Minor
Reporter: Jamie Brandon Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2138"]



 Description   

(defn foo [x]
`(f ~@(for [i (range 10)] ~i)))

(foo 1)

This code has an extra unquote next to the i causing:

TypeError: Cannot call method 'call' of undefined
at aurora.compiler.foo.cljs.core.with_meta.call.cljs.core.seq.call.cljs.core.concat.call.iter_7602auto_ (/home/jamie/aurora/src/aurora/compiler.cljs[eval362]:229:100)

Compare this to:

(defn foo [x]
(bar x))

(foo 1)

Which causes:

TypeError: Cannot call method 'call' of undefined
at foo (/home/jamie/aurora/src/aurora/compiler.cljs[eval376]:212:67)

In the first case, the symbol is mangled and the line number and column are incorrect.






[CLJS-758] Browser REPL doesn't properly catch errors Created: 31/Jan/14  Updated: 20/Feb/14  Resolved: 20/Feb/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   

Brandon Bloom has a patch here http://gist.github.com/brandonbloom/8744556/raw/2efb894b11bb25e9724e3514c7bccc872e30b28d/open+0001-Use-catch-default-in-browser-repl-and-reflect.patch



 Comments   
Comment by David Nolen [ 20/Feb/14 11:45 AM ]

fixed https://github.com/clojure/clojurescript/commit/203d853f1e9c5673cfd987feb7cd07a4762e1479





[CLJS-757] Redundant bounds checking in indexed types Created: 29/Jan/14  Updated: 24/Feb/14  Resolved: 24/Feb/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: None
Environment:

r2156


Attachments: Text File cljs-757.patch     Text File cljs-757.patch    

 Description   

PersistentVector and TransientVector contain code paths where a bounds check is performed more than once unnecessarily.



 Comments   
Comment by Francis Avila [ 29/Jan/14 11:44 PM ]

Patch gives a minor across-the-board speedup on vector operations.

I also discovered and fixed an off-by-one check in PersistentVector's -seq, which causes an unnecessary ChunkedSeq to be created when vector length is exactly 32.

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

Can we get a rebased version of this patch? Thanks.

Comment by Francis Avila [ 24/Feb/14 12:22 AM ]

Rebased cljs-757 with a change to unchecked-array-for-longvec (now first-array-for-longvec) and extra comments documenting invariants.

Comment by David Nolen [ 24/Feb/14 5:59 AM ]

fixed https://github.com/clojure/clojurescript/commit/0f943b616e36c65e0a8921fb838b71368fe9674c





[CLJS-756] undeclared ns warning needs to check for existing required namespaces Created: 29/Jan/14  Updated: 20/Feb/14  Resolved: 20/Feb/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   

with-out-str currently produces a warning even though goog.string is loaded by core.clj.



 Comments   
Comment by David Nolen [ 29/Jan/14 11:46 AM ]

This means we need to always know the entire set of namespaces that are currently required. This will eliminate a long outstanding bug with respect to resolving symbols generated by macros that are in namespaces not explicitly required by users as they are implicit when loading the main library namespace. This is of course known for ClojureScript libraries, but we need special casing for Google Closure libs, Closure compatible libs, and foreign libs that the user provides a namespace for.

Comment by David Nolen [ 14/Feb/14 9:04 PM ]

See http://dev.clojure.org/jira/browse/CLJS-615

Comment by David Nolen [ 20/Feb/14 11:46 AM ]

Resolved by CLJS-615





[CLJS-755] Expose Clojure's hash calculation helper API Created: 29/Jan/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: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

To aid in collection portability between Clojure implementations.






[CLJS-754] Assess Murmur3 for Collections Created: 29/Jan/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: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

We should assess the performance implications of adopting Murmur3 for hashing - http://github.com/clojure/clojure/commit/dff9600387b962f16fc78e6477e10e34651fd366



 Comments   
Comment by Francis Avila [ 19/Feb/14 5:19 AM ]

I have a (lightly tested, probably untrustworthy) gist of clj.lang.murmur3 ported to javascript. The two other murmur3 js libs I saw both had flaws which I attempt to address.

Found a jsperf comparing murmur3 libs and added a goog.string.hashCode case as a baseline. This jsperf test isn't very good because:

  1. it only tests a tiny string
  2. the murmur libraries are both flawed:
    1. one of them assumes strings are 8-bit only (consumes 4 chars at a time for hashing)
    2. the other one treats strings as utf-16, but does not account for integer overflows.

However, the results are still informative--string hashing with murmur3 in js will probably be half as fast as goog.string.hashCode, except on firefox where it is about the same.

Not nearly enough data for a true assessment, but I hope it helps.

Comment by Francis Avila [ 19/Feb/14 10:20 AM ]

It occurs to me that the slowdown is probably entirely due to integer multiplication, and that spidermonkey's jit is detecting and optimizing that pattern of code to Math.imul while the others are not. Math.imul is still experimental but supported on many browsers already--it might be better to polyfill it instead of unconditionally inlining the bit-shifting. Will try later.

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

There's very little need to write this logic in JavaScript - it should just be done in ClojureScript. I think modern JS engines can probably handle Murmur3, here are some perf numbers for simulating multiplication of 2 unsigned 32bit ints that I found on StackOverflow http://jsperf.com/bit-multiply

Comment by Francis Avila [ 19/Feb/14 3:14 PM ]

I agree re: implementing in clojurescript. It looked hairier when I started.

You're also right about the bit multiply. I extended that case with larger input values and a comparison against Math.imul and it seems to be compiling to the same code. Maybe inlining the bit-multiply is confusing the jits, or maybe it's something else entirely. I'll give it another shot when I get a chance.

Any advice on setting up tests for this that would work in the clojurescript project's environment? Probably what we want is a generative test which ensures the hash value in cljs is the same as in clj?

Comment by David Nolen [ 19/Feb/14 4:34 PM ]

Generative tests that guarantee the hashes are the same in both CLJS & CLJ is a nice to have and not really a priority. Murmur3 is pretty straightforward looking, I'm sure we can get it right

Comment by David Nolen [ 20/Feb/14 9:57 AM ]

One of the V8 engineer did a more accurate version of the benchmark. It looks like Math.imul is the way to go and we should shim in something for older browsers.

http://jsperf.com/bit-multiply/5

Comment by Francis Avila [ 22/Feb/14 2:20 AM ]

I thought those results looked too good. 32-bit maths in js is always tricky, hence desire for tests, especially if one of the use cases is comparing compile-time and run-time hashes.

Cleaned up js implementation of murmur3 just to get something up quickly to assess murmur3 performance. It uses Math.imul or a shim for integer multiplication. Then created a pair of jsperfs:

In both tests, setup code is the same: pretty-printed closure-advanced-compiled code from my js-murmur3 implementation, and copy-pasted code from cljs's current number and string hashing.

Results are...interesting. So interesting that I am suspicious that something is wrong with my benchmarks or code. Perhaps you can have Mr. Egorov take a look? In summary:

  • murmur3 int hash is about an 8% performance regression in firefox and safari.
  • murmur3 string hash is more than double the speed of goog.string.hashCode on ff and safari for small strings, and even better for large ones.
  • Except in chrome, where murmur3 is abysmal--about an order of magnitude regression on both ints and strings!

All these browsers have a native Math.imul, and Chrome's imul is definitely working. There must be something else making chrome's jit cranky.

I did not expect murmur3 to perform so well or so badly!

Comment by Francis Avila [ 26/Feb/14 3:37 AM ]

Maintaining a fork with murmur3 hashing (see murmur3 branch). So far numbers, strings, and collections (ordered and unordered) hash identically to clojure 1.6-beta1, but symbols and keywords do not. I suspect integer-math differences in cljs's hash-combine vs clojure.lang.Util/hashCombine, but I have not isolated the problem yet.

Comment by Francis Avila [ 26/Feb/14 3:06 PM ]

Nevermind, cljs and clj reverse the order of ns+name hashing and I just didn't notice.

This branch appears to produce hashes identical to clojure 1.6 for the following types:

  • strings
  • keywords and symbols
  • vectors, lists, persistentqueue, seqs
  • maps, sets
  • integers representable with doubles in js (i.e., about 53 bits of signed integer)

Types which do not hash the same (likely incomplete list):

  • uuids
  • instances
  • floating-point types. Clojure 1.6 still uses Java's native hashCode for these, and checking for a non-int is a bit involved in js, so I just hash all js numbers as if they were longs. This might be a bad idea if we have a collection of "real" doubles.
Comment by David Nolen [ 08/Jun/14 12:15 PM ]

Murmur3 is now implemented in the 1.6.0 branch http://github.com/clojure/clojurescript/tree/1.6.0

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

fixed in master





[CLJS-753] Too many open files exception Created: 28/Jan/14  Updated: 28/Jan/14  Resolved: 28/Jan/14

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

Type: Defect Priority: Blocker
Reporter: Roman Scherer Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: File too-many-open-files.diff    
Patch: Code

 Description   

This patch uses `io/reader` in conjunction with `with-open` to prevent
"too many open files" errors. This happens in a browser REPL when
evaluating a file that references a couple of other files multiple
times.

From the Google Group discussion at: https://groups.google.com/forum/#!topic/clojurescript/r2iGPh2Lv0U

----------------------------

Hello Clojure Scripters,

with my current REPL setup I get some really annoying "Too many
files open" errors. I'm not sure if it's a problem with
ClojureScript itself, Austin the browser REPL, nREPL or my own
miss-configuration of something.

When I connect to the browser REPL via AUSTIN and eval a whole
ClojureScript file the first time a lot of Ajax requests are sent
over the wire and my main namespace is getting compiled and
shipped to the browser. So far so good, my Java process is at
around 18676 open files. I don't care yet.

Compiling the same file again and again increases the open files

not sure if this is a problem with my setup, but I

18676, 19266, 22750, 21352, 33097, 62913, 64398, 64398, 64398,
64398, 64398 up to 171977, where some ulimit is reached and I get
an exception like this:

java.io.FileNotFoundException: .repl/5614/request/routes.js (Too many open files)

and my ClojureScript hangs up and I have to do a
cider-restart. Ok maybe I shouldn't eval whole files too often
over the XHR connection, but this seems not right.

I used the command "lsof -n | grep java | wc -l" to watch the
above numbers while evaluating the file again and again.

Does someone had a similar problem, knows how to solve that, or
has any ideas how to track this one down?

Thanks for your help, Roman.



 Comments   
Comment by Roman Scherer [ 28/Jan/14 5:56 PM ]

Hi David, can you review this. Thanks.

Comment by David Nolen [ 28/Jan/14 6:19 PM ]

Looks good to me will apply soon and test.

Comment by David Nolen [ 28/Jan/14 10:51 PM ]

fixed, https://github.com/clojure/clojurescript/commit/905c64445faa1c60e21b66fb226982759b0d4001





[CLJS-752] Implement specify! to enable extending instances that don't implement ICloneable Created: 28/Jan/14  Updated: 28/Jan/14  Resolved: 28/Jan/14

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

Type: Enhancement Priority: Minor
Reporter: Darrick Wiebe Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Implementing specify! would enable instances of built-in objects (DOM objects, etc) to be extended with protocols without polluting the environment.



 Comments   
Comment by David Nolen [ 28/Jan/14 10:59 PM ]

fixed, https://github.com/clojure/clojurescript/commit/6e5896cb6cd3549d069473e86e424104c107223d





[CLJS-751] requiring clojure.core.reducers throws error Created: 15/Jan/14  Updated: 16/Jan/14  Resolved: 16/Jan/14

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

Type: Defect Priority: Critical
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: reducers
Environment:

r2138


Attachments: Text File cljs-751.patch    

 Description   

Requiring the reducers library in any file throws a TypeError at runtime. The generated javascript in reducers.js is this:

cljs.core.IPersistentVector.prototype.clojure$core$reducers$CollFold$ = true;

Error messages is "Uncaught TypeError: Cannot read property 'prototype' of undefined reducers.js:733
(anonymous function)". This stops all execution of JS and basically makes it impossible to use any

IPersistentVector in reducers.cljs should probably be PersistentVector. The specify machinery in extend-type probably exposed this bug.



 Comments   
Comment by Francis Avila [ 15/Jan/14 4:54 PM ]

Note that because of advanced-compilation dead-code elimination, you won't catch this if you advance-compile. Run script/test with whitespace optimizations to see the reducer tests fail.

Comment by Francis Avila [ 15/Jan/14 4:57 PM ]

Patch fixes typo and causes existing reducer tests to pass (tests run with whitespace optimization). Doesn't make sense to write any more tests, although the test runner should probably not use advanced compilation as the default....

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

fixed, https://github.com/clojure/clojurescript/commit/6e10f3d2ca99c58c441de1a1031be2649dae4072





[CLJS-750] js->clj not turning keys into keywords Created: 14/Jan/14  Updated: 16/Jan/14  Resolved: 16/Jan/14

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

Type: Defect Priority: Major
Reporter: Matthew Phillips Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

I created a fiddle for this: http://cljsfiddle.net/fiddle/matthewp.myfiddle

Click run and then inspect the console. You'll notice that the keys are just strings, not clojure keywords as expected.



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

The fiddle is incorrect.

(js->clj x :keywordize-keys true)

Is what you want.





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

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

Type: Defect Priority: Minor
Reporter: Osbert Feng Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File change-browser-repl-compile-directory.patch    
Patch: Code

 Description   

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

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

See attached patch.

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

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

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






[CLJS-748] Dump analyzer state during a compilation. Created: 13/Jan/14  Updated: 16/Jan/14

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

Type: Enhancement Priority: Minor
Reporter: Gary Trakhman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler

Attachments: Text File CLJS_748.patch    
Patch: Code

 Description   

CLJS doesn't have the luxury of a fully-reified runtime environment, therefore effective tooling needs a view of the internal compiler state. This issue addresses this need by enabling a dump of the compiler state based on additional cljs compiler options.

The compiler state is a map contained within an atom, either passed in as the 'compiler-env' arg to cljs.closure/build or contained globally within cljs.env/compiler.

The prototype is implemented as such:

:dump-analysis-to
either a string or :print, when :print, output will be written to out, when a string, the argument will be passed to clojure.java.io/writer.

:dump-analysis-full
checked for truthiness, default is false
When this is switched on, the full structure of the compiler state will be printed, which is impractically large for most use cases. In normal operation, :env keys and extended :method-params will be removed from the output, making the analysis represent simply the globally reachable environment. Additionally, only the contents under :cljs.analyzer/namespaces and :cljs.analyzer/constants will be printed.



 Comments   
Comment by Gary Trakhman [ 13/Jan/14 6:46 PM ]

Added implementing patch

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

I question the utility of :dump-analysis-full for now. Lets remove it.

Comment by Gary Trakhman [ 16/Jan/14 5:32 PM ]

I was thinking it might be good along with :print followed by a pipe, but... I don't have an explicit use-case. I'll create a new patch.





[CLJS-747] Numerical inconsistency between Clojure and ClojureScript (running in Rhino) Created: 11/Jan/14  Updated: 16/Jan/14  Resolved: 16/Jan/14

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

Type: Defect Priority: Minor
Reporter: Jim Pivarski Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: integer-overflow
Environment:

Clojure (1.5.1) on JVM vs. ClojureScript (cloned from GitHub today) in Rhino on JVM



 Description   

In Clojure, the maximum long value throws an ArithmeticException when incremented with + and rolls over to a BigInt when incremented with +'

user=> (+ 9223372036854775807 1)
ArithmeticException integer overflow  clojure.lang.Numbers.throwIntOverflow (Numbers.java:1388)
user=> (+' 9223372036854775807 1)
9223372036854775808N

But ClojureScript rolls over to BigInt with + and does not have a +' function

ClojureScript:cljs.user> (+ 9223372036854775807 1)
9223372036854776000N
ClojureScript:cljs.user> (+' 9223372036854775807 1)
WARNING: Use of undeclared Var cljs.user/+' at line 1 
"Error evaluating:" (+' 9223372036854775807 1) :as "cljs.user._PLUS__SINGLEQUOTE_.call(null,9223372036854775807,1)"
org.mozilla.javascript.EcmaError: TypeError: Cannot call method "call" of undefined (<cljs repl>#1)
        at <cljs repl>:1 (anonymous)
        at <cljs repl>:1

In fact, the inexactness of 9223372036854776000N suggests the reason: it's actually a double, presented as though it were a BigInt. (Perhaps that's a limitation of JavaScript? If so, at least it should be presented as a double, as very large numbers are: e.g. (* 9223372036854775807 9223372036854775807) --> 8.507059173023462E37.) From http://stackoverflow.com/questions/8664093/not-getting-integer-overflow-in-clojure , it is my understanding that this behavior was defined in Clojure version 1.3--- is ClojureScript a few versions behind in that it attempts roll-over and does not have a +' function?



 Comments   
Comment by Jim Pivarski [ 11/Jan/14 3:53 AM ]

That is, ClojureScript doesn't implement http://dev.clojure.org/display/design/Documentation+for+1.3+Numerics

Comment by Jozef Wagner [ 11/Jan/14 5:54 AM ]

If I understand your description correctly, the issue is that Clojurescript does not have BigInts and precise/unchecked variants of arithmetic functions. Please read this bigint discussion and design page for some background information about this.

Comment by Jim Pivarski [ 11/Jan/14 1:43 PM ]

You're right--- I misunderstood ClojureScript's interoperability policy. I'm considering Clojure/ClojureCLR/ClojureScript/ClojureC/Clojure-Py for cross-platform numeric processing, so I've started testing them for numerical compatibility at the edge cases. At first, this looked like a serious bug (no exception, inexact result) until I figured out that it's just double precision. For my purposes, I could say that the largest integer with cross-platform support is 9223372036854775000, and above that, Clojure* may throw an exception, roll over to BigInt, or inexactly represent it as a double, depending on platform.

I'd like to convert this into a minor feature request for BigInt and +' functions or close this ticket and open a new one, but it doesn't look like I have permission to change its resolution state.

I researched the problem before submitting this ticket, but I didn't find the references that you sent (very helpful, thanks!). Should I continue to report edge cases of incompatibility between Clojure and ClojureScript? For my own purposes, I need to know the ranges within which to expect compatibility, so I'm building up a test suite with an emphasis on numerical processing. (And I'm only checking Clojure, ClojureScript, and Clojure-py for now.) If it's helpful, I'll submit bug reports here; if it's annoying, I won't.

Comment by Jozef Wagner [ 12/Jan/14 4:12 AM ]

Numbers are one of few things which are not consistent in Clojure across hosts. This is a design decision in order to provide fast numerics for hosts which have one. A host independent numeric API would preclude many optimizations. Another outcome of this design decision is that the number types are not user extensible. Even basic arithmetic functions may behave differently in edge cases in different hosts. ClojureScript may get BigInt if someone finds time to implement them, but I doubt they will behave exactly the same as in JVM. You'd better accept that numbers will always be host dependent and code according to that.

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

This will require a considerable amount of design work. Until that happens there's not much point opening tickets.





[CLJS-746] clojure.string/replace pattern/function of match API difference with clojure version Created: 10/Jan/14  Updated: 10/Jan/14

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

Type: Defect Priority: Minor
Reporter: Curtis Gagliardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2138"]



 Description   

When calling clojure.core/replace with a pattern and a function, the Clojurescript version delegates to Javascript's s.replace method, which calls that function with a variable number of arguments, depending on how many match groups are in your pattern. The Clojure version always calls it with a single argument, which may be a vector if you have match groups in your pattern.

I'm not sure if this was intentional. If it wasn't, I think this difference could be fixed through some use of re-find, which appears to return the same string or vector that you'd get in Clojure. If this is intentional for performance reasons, perhaps the doc string should be updated to note this, as there's no warning that the function is being called with too many arguments.



 Comments   
Comment by Curtis Gagliardi [ 10/Jan/14 1:32 AM ]

Afraid I don't see how to edit, but I wanted to include a simple example:

CLJS:
(clojure.string/replace "hello world" #"(hello) world" (fn [m] (.log js/console (str "Match: " m)) m))

will log: "Match: hello world"

CLJ
user=> (clojure.string/replace "hello world" #"(hello) world" (fn [m] (println (str "Match: " m) m)))
Match: ["hello world" "hello"] [hello world hello]

NullPointerException java.util.regex.Matcher.quoteReplacement (Matcher.java:655)





[CLJS-745] Support keywords and namespaced keywords in destructuring Created: 06/Jan/14  Updated: 23/Feb/14  Resolved: 23/Feb/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   

http://dev.clojure.org/jira/browse/CLJ-1318, slated for 1.6



 Comments   
Comment by David Nolen [ 23/Feb/14 4:52 PM ]

fixed https://github.com/clojure/clojurescript/commit/95d60acf610b255dbb4079ccf60739c81a611922





[CLJS-744] ISequential types should implement JS indexOf, lastIndexOf Created: 05/Jan/14  Updated: 05/Jan/14

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

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





[CLJS-743] Allow language_in and language_out options to be passed to the Google Closure compiler Created: 04/Jan/14  Updated: 15/Sep/14  Resolved: 23/Feb/14

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

Type: Enhancement Priority: Major
Reporter: Ivan Willig Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-743.patch    

 Description   

While I was messing around with ClojureScript and nodejs, I
noticed that the Google Closure compiler complains when JavaScript
reserved words are used. Newer versions of JavaScript engines allow
reserved words (like "static", "class") to be used as object
properties. The Closure Compiler knows this and does not complain when
you set the language_in option to the correct version of JavaScript.



 Comments   
Comment by David Nolen [ 07/Jan/14 7:07 AM ]

Excellent thanks.

Comment by David Nolen [ 09/Jan/14 5:17 PM ]

Ivan have you signed and sent in your CA?

Comment by Chas Emerick [ 20/Feb/14 9:23 AM ]

@iwillig Looking forward to seeing this, def send in that CA.

Comment by Ivan Willig [ 20/Feb/14 10:06 PM ]

I apologize for letting this slip, work got busy. CA is in the mail.

Comment by David Nolen [ 23/Feb/14 4:33 PM ]

fixed, https://github.com/clojure/clojurescript/commit/b33bde39fe838d367939b55b9dadb0480ea00a7c

Comment by Richard Hull [ 14/Sep/14 3:22 PM ]

Could the ECMASCRIPT6 and ECMASCRIPT6_STRICT enums be added please?
http://javadoc.closure-compiler.googlecode.com/git/com/google/javascript/jscomp/CompilerOptions.LanguageMode.html

Not really a major problem, but when I set :language-in to ECMASCRIPT5 without setting :language-out, I get the following:

com.google.javascript.jscomp.CompilerOptionsValidator$InvalidOptionsException: 
Can only convert code from ES6 to ES3. Cannot convert from ECMASCRIPT5 to ECMASCRIPT3.
Comment by David Nolen [ 15/Sep/14 12:17 PM ]

Richard please open a new ticket for this.





[CLJS-742] Compilation with :output-file option set fails Created: 03/Jan/14  Updated: 03/Jan/14

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

Type: Defect Priority: Trivial
Reporter: Timothy Pratley Assignee: Timothy Pratley
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJS-742.diff    

 Description   

./bin/cljsc hello.cljs '{:output-file "foo.js"}'

Expected:
a file called foo.js

Actual:
Compiler throws an internal error

Notes:
The correct option is :output-to
:output-file might be removable (it is broken, nobody is using it?)
Further analysis required to determine if it is useful to keep.



 Comments   
Comment by Timothy Pratley [ 03/Jan/14 12:14 PM ]

Fixes handling of compiling one IJavaScript or many





[CLJS-741] seq error message readability is not optimal Created: 02/Jan/14  Updated: 02/Jan/14

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

Type: Enhancement Priority: Trivial
Reporter: Julien Eluard Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File cljs_741.patch    

 Description   

When calling seq on an invalid type an errr with following message is thrown: :keywordis not ISeqable.

Adding a space between the argument and the message improves the readability.



 Comments   
Comment by David Nolen [ 02/Jan/14 12:58 PM ]

Excellent thanks!





[CLJS-740] undeclared-ns warnings are broken Created: 02/Jan/14  Updated: 17/Jan/14  Resolved: 17/Jan/14

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

Type: Defect Priority: Minor
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: Compiler, bug, patch,

Attachments: File fix-undeclared-ns-warnings.diff    
Patch: Code

 Description   

In the recent versions of cljs, the undeclared-ns warnings have stopped working. This patch seems to be the culprit.



 Comments   
Comment by David Nolen [ 02/Jan/14 12:58 PM ]

Great thanks!

Comment by David Nolen [ 07/Jan/14 10:32 AM ]

CLJS-615

Comment by David Nolen [ 17/Jan/14 9:26 AM ]

fixed, https://github.com/clojure/clojurescript/commit/b8cf302b1500e88e0602e72fa6aec6f7328a1a00





[CLJS-739] fn/recur bug Created: 01/Jan/14  Updated: 23/Feb/14  Resolved: 23/Feb/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   
(defn create-functions [arr names]
  (let [name (first names)]
    (if name
      (recur (conj arr (fn [] (println name)))
             (rest names))
      arr)))
 
(doseq [fn (create-functions [] [:a :b :c :d])] (fn))

Generates the following incorrect code, the local name is not closed over:

cljs_play.let_bug.core.create_functions = function create_functions(arr, names) {
  while (true) {
    var name = cljs.core.first.call(null, names);
    if (cljs.core.truth_(name)) {
      var G__4744 = cljs.core.conj.call(null, arr, function(arr, names) {
        return function() {
          return cljs.core.println.call(null, name);
        };
      }(arr, names));
      var G__4745 = cljs.core.rest.call(null, names);
      arr = G__4744;
      names = G__4745;
      continue;
    } else {
      return arr;
    }
    break;
  }
};


 Comments   
Comment by David Nolen [ 01/Jan/14 9:43 PM ]

This is because we don't do two passes to determine if a function uses recur. loop/recur works because we know up front.

Comment by David Nolen [ 23/Feb/14 6:04 PM ]

I actually can't reproduce this in master - I've added tests to confirm https://github.com/clojure/clojurescript/commit/aaae2688aa2646bb252a9a4dd321e9fa8dbedb6a





[CLJS-738] port clojure.data Created: 31/Dec/13  Updated: 01/Jan/14  Resolved: 01/Jan/14

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

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


 Comments   
Comment by Francis Avila [ 31/Dec/13 9:46 PM ]

Isn't this already done? CLJS-378 data.cljs

Comment by David Nolen [ 01/Jan/14 9:55 AM ]

Oh wow, was looking for the wrong file thanks!

Comment by David Nolen [ 01/Jan/14 9:55 AM ]

Already done.





[CLJS-737] tool.reader can't handle white-space Created: 30/Dec/13  Updated: 09/Jan/14  Resolved: 09/Jan/14

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

Type: Defect Priority: Minor
Reporter: Andrew Zures Assignee: Unassigned