<< Back to previous view

[CLJS-1439] Add type annotations to goog-define defined vars Created: 01/Sep/15  Updated: 01/Sep/15

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

Type: Enhancement Priority: Major
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently it's still required to annotate goog-define constants with ^boolean to allow the Closure compiler to safely remove dead branches. The macro could emit the var with ^boolean metadata to make this unnecessary.

In general it would be nice to have similar annotations for the already defined constants like goog.DEBUG although I'm not sure how/if that's possible.






[CLJS-365] apply needs to put all args after the 20th into an array seq Created: 29/Aug/12  Updated: 29/Aug/12

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   

This ticket is related to CLJS-359






[CLJS-364] compiler needs to put all args of an invocation after 20 into an array-seq Created: 29/Aug/12  Updated: 29/Aug/12

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   

This ticket is related to CLJS-359






[CLJS-1466] Absolute paths in :output-dir break Node.js shim for :none Created: 11/Oct/15  Updated: 29/Jan/16

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

Type: Defect Priority: Major
Reporter: Sebastian Bensusan Assignee: Sebastian Bensusan
Resolution: Unresolved Votes: 0
Labels: bug

Attachments: Text File cljs_1466.patch    
Patch: Code

 Description   

When compiling a trivial example with the following script:

(require 'cljs.build.api)

(cljs.build.api/build "src"
  {:main 'hello.core
   :output-to "main.js"
   :output-dir "/home/carlos/Playground/node-abs/out"
   :target :nodejs})

It generates code that tries to resolve the following path:

/home/carlos/Playground/node-abs/home/carlos/Playground/node-abs/out/goog/bootstrap/nodejs.js

We should check if the provided path for :output-dir is absolute before resolving it in the Node.js :none shim. The shim has a related ticket in CLJS-1444.

Even if it's uncommon for users to have absolute paths, tooling might need to.



 Comments   
Comment by Sebastian Bensusan [ 11/Oct/15 4:28 PM ]

The attach patch cljs_1466.patch solves the issue by using path.resolve which takes into account relative vs absolute paths when joining paths. Successfully tested in the example repo with both relative and absolute :output-dir

Comment by Martin Klepsch [ 14/Oct/15 3:57 AM ]

Looking at the patch it seems that it might break current behaviour in some cases? Have you thought about that?

CLJS-1444 [1] would probably also break the shim in some way so would be good to get these in together and be very clear about what will break etc. As long as we come up with a robust and predictable impl this is something worth breaking imo.

[1] http://dev.clojure.org/jira/browse/CLJS-1444 for

Comment by David Nolen [ 14/Oct/15 8:05 AM ]

Yes would like to get feedback from people already heavily invested in ClojureScript + Node.js before moving forward on this.

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

Martin Klepsch: I did think about breakage but I couldn't find any cases. Do you have an example one? In the example repo I've put together some tests (by running ./script/test.sh) but it boils down to path.join(path.resolve("."),paths) being equivalent to path.resolve(paths) for all relative paths, since the "Resolve to absolute" method is the same for both (process.cwd() inside of path.resolve). When considering absolute paths, only the new version does the right thing.

On the other hand, those tests also reveal that the proposed patch doesn't cover CLJS-1446 as I originally thought since

node main.js

succeeds while:

cd ..
node node-abs/main.js

fails.





[CLJS-1558] Code allowed to re-define referred var Created: 31/Jan/16  Updated: 31/Jan/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If you refer a var from another namespace, then you can def a new value for that var, and the def will mutate the other namespace, and other things will go wrong as illustrated in the example below.

FWIW, Clojure disallows this, and refuses to allow you to evaluate a def involving a referred var, and emits an error diagnostic like:

CompilerException java.lang.IllegalStateException: foo already refers to: #'some.name.space/foo in namespace: user, compiling:(NO_SOURCE_PATH:2:1)

Here is a complete example illustrating the issues:

Given:

(ns foo.core)

(defn square [x]
  (* x x))

then do this in a REPL:

cljs.user=> (require '[foo.core :refer [square]])
nil
cljs.user=> (var square)
#'foo.core/square
cljs.user=> (square 3)
9
cljs.user=> (ns-interns 'cljs.user)
{}
cljs.user=> (defn square [x] (+ x x))
WARNING: square already refers to: foo.core/square being replaced by: cljs.user/square at line 1 <cljs repl>
#'foo.core/square
cljs.user=> (square 3)
6
cljs.user=> (var square)
#'foo.core/square
cljs.user=> (in-ns 'foo.core)
nil
foo.core=> (square 3)
6
foo.core=> (in-ns 'cljs.user)
nil
cljs.user=> (ns-interns 'cljs.user)
{square #'cljs.user/square}
cljs.user=> (cljs.user/square 3)
TypeError: Cannot read property 'call' of undefined
    at repl:1:105
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:54:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:221:14)
    at Socket.<anonymous> ([stdin]:40:25)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
cljs.user=> #'cljs.user/square
#'cljs.user/square
cljs.user=> @#'cljs.user/square
nil





[CLJS-1479] Race condition in browser REPL Created: 03/Nov/15  Updated: 12/Feb/16

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

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File heavy-load.sh     File race-condition.clj     File race-condition.jstack    

 Description   

Evaluation in browser REPL occasionally hangs. It seems that repl environment and browser sometimes miss each other and their "randezvous" fails. Browser is waiting for POST reply and repl is trying to send a command, but they do not meet each other.

I found the issue when we switched our tests from nodejs to browser environment. Luckily I was able to find very small example which hangs during execution. It seems that (simulated) heavy load increases the chance of "hanging".

Minimal setup:

(ns race.condition
  (:require [cljs.repl.browser :as browser]
            [cljs.repl :as repl]
            [cljs.env :as env]
            [cljs.build.api :as api]))


(api/build '[(ns race.repl
               (:require [clojure.browser.repl]))
             (clojure.browser.repl/connect "http://localhost:9000/repl")]
           {:output-to  "target/cljs-race/main.js"
            :output-dir "target/cljs-race"
            :main       'race.repl})

(spit "target/cljs-race/index.html"
      (str "<html>" "<body>"
           "<script type=\"text/javascript\" src=\"main.js\">"
           "</script>" "</body>" "</html>"))

Now start the environment:

(def env (browser/repl-env :static-dir ["target/cljs-race" "."] :port 9000 :src nil))

(env/with-compiler-env (env/default-compiler-env)
  (repl/-setup env {}))

cross your fingers and start this endless loop:

(loop [i 0]
  (println (java.util.Date.) i)
  (dotimes [j 100]
    (let [result (repl/-evaluate env "<exec>" "1"  "true")]
      (when-not (= :success (:status result))
        (println i j result))))
  (recur (inc i)))

To simulate heavy load run heavy-load.sh from attachment.

After some iterations (eg 55 big loop i) execution stops. If you investigate stacks (see race-condition.jstack), you can see in one thread:

at clojure.core$promise$reify__6779.deref(core.clj:6816)
	at clojure.core$deref.invoke(core.clj:2206)
	at cljs.repl.browser$send_for_eval.invoke(browser.clj:65)
	at cljs.repl.browser$browser_eval.invoke(browser.clj:193)
	at cljs.repl.browser.BrowserEnv._evaluate(browser.clj:262)

The code is waiting for a promise with a connection (which already did arive).

My guess is suspicious code in cljs.repl.server functions connection and set-connection. Both functions access an atom in non-standard way. They deref a valua and make a swap! in two steps.

Can somebody with better understanding of REPL internals investigate? Thank you.



 Comments   
Comment by David Nolen [ 12/Feb/16 2:57 PM ]

A patch is welcome for this one.





[CLJS-1691] spec internal compiler APIs Created: 21/Jun/16  Updated: 21/Jun/16

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

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





[CLJS-525] Allow hashtable lookup used for numbers and strings to be extended to other built-in types Created: 17/Jun/13  Updated: 17/Jun/13

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

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

...which would enable safe extension of key cljs protocols to types without modifying their prototypes, e.g. CLJS-523.



 Comments   
Comment by David Nolen [ 17/Jun/13 2:56 PM ]

Date is the only JS native case that I'm aware of that we don't handle. One tricky bit is that goog.typeOf won't give us the information we need, but I think instanceof should cover us here?

Comment by Fogus [ 17/Jun/13 3:05 PM ]

instanceof or the ever-gruesome toString.call(aDate) == '[object Date]' will work.





[CLJS-1753] cljs.pprint does not print default tagged literals/elements Created: 16/Aug/16  Updated: 16/Aug/16

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

Type: Defect Priority: Major
Reporter: Miroslav Kubicek Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Win 7 64bit



 Description   

Hi there!
I am having troubles making the cljs pretty print (cljs.pprint/pprint) behave the same way as the regular cljs or clj print function when it goes down to tagged elements that are by default used to denote custom records.

See example below - pr, pr-str, print and print-str functions all use the default approach toward creating (edn-like) tagged element for MyRecord and all produce the same result:
#cljs.user.MyRecord{:value "a"}

On the other hand pprint just ignores the record's tag and simply just traverses/prints it as a map:
{:value "a"}

Is there some setting and/or parameter in cljs.pprint namespace I am missing? I looked briefly at the code, but it seems it uses print-str by default - so maybe it just traverses the graph depth-first and does not check for every node's type? At this stage this seems like a bug to me as the expected behavior of the pprint function is that it would behave the same way print and other core functions do.

THIS WORKS:

cljs.user=> (defrecord MyRecord [value])
cljs.user/MyRecord
cljs.user=> (pr (MyRecord. "a"))
#cljs.user.MyRecord{:value "a"}
nil
cljs.user=> (pr-str (MyRecord. "a"))
"#cljs.user.MyRecord{:value \"a\"}"
cljs.user=> (print (MyRecord. "a"))
#cljs.user.MyRecord{:value a}
nil
cljs.user=> (print-str (MyRecord. "a"))
"#cljs.user.MyRecord{:value a}"

BUT THIS DOESN'T:

cljs.user=> (cljs.pprint/pprint (MyRecord. "a"))
{:value "a"}
("{:value \"a\"}\n")
cljs.user=> (with-out-str (cljs.pprint/pprint (MyRecord. "a")))
"{:value \"a\"}\n"

According to github the head revision of the cljs.pprint namespace has not changed since 1.7.28 so I'd assume all versions up to the current one are affected.

Thanks for help!






[CLJS-1690] spec the ClojureScript AST Created: 21/Jun/16  Updated: 21/Jun/16

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

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





[CLJS-1777] `module` undefined when using `:module-type :commonjs` Created: 14/Sep/16  Updated: 14/Sep/16

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

Type: Enhancement Priority: Major
Reporter: Arne Brasseur Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The Google Closure Compiler support for CommonJS modules rewrites `exports` and `module.exports`, but not `module`. Many libraries try to detect the module type (CommonJS) by checking the type of `module`, e.g. this is taken from D3.

```
typeof exports === 'object' && typeof module !== 'undefined'
```

This becomes

```
goog.provide('module$resource$d3')
typeof module$resource$d3 === 'object' && typeof module !== 'undefined'
```

Because `module` is undefined this fails, and nothing gets exported.

This seems like something Google Closure should address.

Alternatives would include injecting some code that defines `module` (`var module={}`) or munging `typeof module` to `"object"`.



 Comments   
Comment by Arne Brasseur [ 14/Sep/16 6:41 AM ]

To test

curl https://d3js.org/d3.v4.js > d3.js

Compiler options

:foreign-libs [{:file "d3.js"
:provides ["d3"]
:module-type :commonjs}]

Code

(:require '[d3])
(d3/select "#app")

Comment by Arne Brasseur [ 14/Sep/16 8:32 AM ]

Seems this exact case was already documented on Maria Geller's blog: http://mneise.github.io/posts/2015-07-08-week-6.html

Comment by Arne Brasseur [ 14/Sep/16 9:04 AM ]

Did some more digging, the issue is that thanks to http://dev.clojure.org/jira/browse/CLJS-1312 Closure Compiler tries to deal with UMD syntax, but there's no single definition of what UMD really looks like. Two popular tools (rollup and webpack) generate code that is not correctly recognized. This is what rollup generates

(function (global, factory) {
  typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
  typeof define === 'function' && define.amd ? define(['exports'], factory) :
  (factory((global.d3 = global.d3 || {})));
}(this, (function (exports) { 'use strict';

This is what webpack generates

(function webpackUniversalModuleDefinition(root, factory) {
	if(typeof exports === 'object' && typeof module === 'object')
		module.exports = factory(require("react"), require("react-dom"));
	else if(typeof define === 'function' && define.amd)
		define(["react", "react-dom"], factory);
	else if(typeof exports === 'object')
		exports["ReactDataGrid"] = factory(require("react"), require("react-dom"));
	else
		root["ReactDataGrid"] = factory(root["React"], root["ReactDOM"]);
})(this, function(__WEBPACK_EXTERNAL_MODULE_1__, __WEBPACK_EXTERNAL_MODULE

This will require changes to ProcessCommonJSModulesTest, similar to https://github.com/google/closure-compiler/commit/aa0a99cf380b05b2185156735d023b6fa78ec4ac





[CLJS-1706] cljs.reader support for namespaced map literal Created: 15/Jul/16  Updated: 11/Aug/16

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Clojure 1.9 extends support for namespaced maps and cljs.reader needs to be able to read them.

#:example{:key "value"} currently results in a Could not find tag parser for :example in ... error.



 Comments   
Comment by Thomas Heller [ 17/Jul/16 3:20 AM ]

I wanted to start implementing this and started looking at the "spec" [1]. It mentions support for #:: and #::alias, but since cljs.reader does not have access to aliases (or the current ns) I do not know how to proceed. Should it just throw then encountering #:: since it isn't really all that useful in a EDN context?

http://dev.clojure.org/jira/browse/CLJ-1910

Comment by David Nolen [ 10/Aug/16 1:56 PM ]

What does the Clojure EDN reader do here?

Comment by Linus Ericsson [ 11/Aug/16 2:26 PM ]

The clojure.edn-reader seems to catch that it doesn't have a default namespace, nor any aliases.

user> (clojure.edn/read-string "#:a {:b 1}")
{:a/b 1}

user> (clojure.edn/read-string "#::a {:b 1}")
RuntimeException Namespaced map must specify a valid namespace: :a  clojure.lang.EdnReader$NamespaceMapReader.invoke (EdnReader.java:494)

user> (clojure.edn/read-string "#:: {:b 1}")
RuntimeException Invalid token: :  clojure.lang.Util.runtimeException (Util.java:221)

This seems to be sane defaults also for cljs.reader.

Also see the commit adding namespaces maps in clojure





[CLJS-1801] Foreign libs that are processed as modules should not have their exports invoked with fn optimizations Created: 30/Sep/16  Updated: 30/Sep/16

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

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





[CLJS-1813] bring clojure.core.specs from clojure Created: 06/Oct/16  Updated: 06/Oct/16

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

Type: Enhancement Priority: Major
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec


 Description   

In clojure there is clojure.core.specs namespace.
It has not yet be ported in clojurescript.






[CLJS-1823] read-string on map with multiple keys should throw error Created: 15/Oct/16  Updated: 17/Oct/16

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

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File cljs-1823-alt.patch     Text File CLJS-1823.patch    

 Description   

On an array-map sized string `(read-string "{:a 1 :b 2 :a 1}")` returns a map with 2 :a keys.
On a hash-map sized string with duplicates no error is thrown.

Both of these cases deviate from Clojure's behaviour which is to throw a duplicate key exception.



 Comments   
Comment by Rohit Aggarwal [ 16/Oct/16 5:22 AM ]

I am attaching a patch which makes the behaviour of reading a PersistentArrayMap same as PersistentHashMap. Now the duplicates are removed from PersistentArrayMap.

Both however don't throw an error.

Comment by Thomas Mulvaney [ 17/Oct/16 5:03 AM ]

I have attached an alternative patch which throws on duplicate when reading and handles other duplicate cases as per clojure. The extra cases which were not handled correctly previously have been added as tests.





[CLJS-1829] 3-arity get does not return not-found on negative key values for arrays and strings Created: 21/Oct/16  Updated: 21/Oct/16

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

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Examples of failing cases:

Calling `(get "asd" -1 :not-found)` returns nil rather than :not-found.
Calling `(get #js [1 2 3] -1 :not-found)` returns nil rather than :not-found.



 Comments   
Comment by António Nuno Monteiro [ 21/Oct/16 5:10 AM ]

Good catch. Probably related to https://github.com/clojure/clojurescript/commit/9cb8da1d82078cfe21c7f732d94115867f455a0a

Mind if I work on this?

Comment by Thomas Mulvaney [ 21/Oct/16 5:49 AM ]

The attached patch also catches the case where a key is nil. Also using charAt rather than aget for strings.

Comment by Thomas Mulvaney [ 21/Oct/16 5:50 AM ]

Sorry António, I just saw your comment.





[CLJS-1786] Add knob for controlling printing of namespaced maps Created: 21/Sep/16  Updated: 23/Oct/16

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Lauri Oherd
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1786.patch    

 Description   

See these Clojure commits:

https://github.com/clojure/clojure/commit/d57b5559829be8e8b3dcb28a20876b32615af0cb
https://github.com/clojure/clojure/commit/b49c1984a1527d17951fbb23ddf9406805a1343f
https://github.com/clojure/clojure/commit/05a8e8b323042fa043355b716facaed6003af324



 Comments   
Comment by Lauri Oherd [ 28/Sep/16 4:30 PM ]

Added print-namespace-maps flag for controlling printing of namespaced maps.
Unlike in Clojure repl the default value is false in Clojurescript repl.
Please comment if this needs to be fixed.

Comment by David Nolen [ 28/Sep/16 4:44 PM ]

Why should the default be different from Clojure?

Comment by Lauri Oherd [ 06/Oct/16 10:59 AM ]

I couldn't figure out initially how to set the print-namespace-maps value to true only in repl environment.
This is resolved in an updated patch. Please check if the implementation is acceptable - in file src/main/clojure/cljs/repl.cljc line 558 the following statement is executed: (set! cljs.core.print-namespace-maps true)

Comment by António Nuno Monteiro [ 06/Oct/16 11:05 AM ]

I don't think that's the desired approach for `print-namespace-maps` to work in every REPL. The wrap-fn is not used for every REPL, as custom REPLs may specify their own.

Why don't you bind the dynamic variable where all others are being bound?

Comment by Lauri Oherd [ 06/Oct/16 1:20 PM ]

Thank you for reviewing the previous patch and suggesting a better approach, António! Updated the patch with suggested solution.





[CLJS-1833] Consider moving int behavior to unchecked-int + clearer docstrings for limitations of other coercions (long etc) Created: 23/Oct/16  Updated: 23/Oct/16

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

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





[CLJS-1800] Defer to tools.reader for cljs.reader functionality Created: 30/Sep/16  Updated: 26/Oct/16

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Aleš Roubíček
Resolution: Unresolved Votes: 0
Labels: None


 Description   

We already have a dependency on tools.reader and maintaining our own EDN reader is just an unnecessary burden.



 Comments   
Comment by Aleš Roubíček [ 02/Oct/16 1:02 AM ]

I'm fiddling with this and found two differences in reading (according to cljs.reader-tests):

  1. cljs.tools.reader reads the @ macro as 'clojure.core/deref instead of 'deref
  2. cljs.tools.reader does not read small maps as PersistentArrayMap

Shoud this be solved by patching cljs.tools.reader or by changing cljs.reader-tests?

Comment by David Nolen [ 04/Oct/16 1:35 PM ]

Questions should be asked about 2). 1) seems fine.

Comment by Rohit Aggarwal [ 05/Oct/16 4:11 AM ]

I think fixing cljs.tools.reader/read-map to return a PersistentArrayMap or a PersistentHashMap is a relatively easy fix and should be raised in TRDR bug tracker.

Comment by Aleš Roubíček [ 26/Oct/16 7:04 AM ]

I did the patch for cljs.tools.reader http://dev.clojure.org/jira/browse/TRDR-40





[CLJS-1410] Support source maps in deps.cljs Created: 09/Aug/15  Updated: 12/Feb/16

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

Type: Enhancement Priority: Major
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

There should be support to package source maps with a foreign-lib using deps.cljs



 Comments   
Comment by David Nolen [ 12/Feb/16 3:00 PM ]

Patch welcome for this one!





[CLJS-1836] nth doesn't throw for IndexedSeqs Created: 25/Oct/16  Updated: 25/Oct/16

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

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Examples:

(nth (seq (array 0 1 2 3)) 4) => nil (expected js/Error)
(nth (seq (array 0 1 2 3)) -1) => nil (expected js/Error)
(nth (seq [0 1 2 3]) 4 :not-found) => nil (expected :not-found)
(nth (seq [0 1 2 3]) -1 :not-found) => nil (expected :not-found)

This only affects sequences of javascript arrays, strings and small (<= 32 elements) PersistentVectors.






[CLJS-1702] Warning or error when using private vars Created: 07/Jul/16  Updated: 07/Jul/16

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

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


 Description   

Currently no warning or error of any kind is given. Throwing an error and forcing users to go through vars is somewhat less attractive since vars dump information like file, line, etc. A warning would be a simple way to flag users that they are treading into dangerous territory. Downstream tooling error handling can make it a hard error if they like.






[CLJS-1501] Add :parallel-build support to REPLs Created: 05/Dec/15  Updated: 08/Nov/16

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

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


 Description   

The :parallel-build option does not currently work in REPLs due to the implementation of cljs.repl/load-namespace






[CLJS-1461] Convert analyzer to conform to tools.analyzer's spec Created: 28/Sep/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Ambrose Bonnaire-Sergeant
Resolution: Unresolved Votes: 5
Labels: None


 Description   

tools.analyzer has a handy common AST format for map-based analysis results. We should use this format in the ClojureScript analyzer so tooling that already accepts this format can work seamlessly with ClojureScript.






[CLJS-1447] IFn implementors have a broken call implementation, all args after 20th argument should be collected into a seq Created: 11/Sep/15  Updated: 08/Nov/16

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

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


 Description   

Regular fns (which are just JavaScript fns) have no such limit. For IFn implementors we should not allow arities above 21 args, and we should transform the 21st arity into a var args signature.



 Comments   
Comment by François De Serres [ 18/Jun/16 9:13 AM ]

we should transform the 21st arity into a var args signature

Unless misunderstanding, can't do that. Var args sigs aren't allowed in protocols.

we should not allow arities above 21 args

Emitting an analyzer warning is what you want?

Comment by Antonin Hildebrand [ 05/Jul/16 6:07 PM ]

I believe I hit this problem in my code using core.async[1].

If it is not possible to implement ATM, I would kindly ask for a compiler warning at least. This thing manifested as a infinite recursive loop ending up in a cryptic stack overflow.

[1] https://github.com/binaryage/dirac/commit/cce56470975a287c0164e6f79cd525d6ed27a543





[CLJS-1446] autodoc + gh-pages for cljs.*.api namespaces Created: 11/Sep/15  Updated: 08/Nov/16

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

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


 Comments   
Comment by W. David Jarvis [ 11/Sep/15 6:07 PM ]

I just tried to get this working - unfortunately, autodoc doesn't currently have support for ClojureScript. An issue is currently open on the GH project here but it doesn't look like it's seen any movement in nearly two years.

Comment by Tom Faulhaber [ 13/Sep/15 2:26 PM ]

I would love to see this work as well and, as the author of autodoc, am happy to help move it forward. I've added some commentary to the issue in autodoc about how to do this. If it's going to happen soon, though, I will need some help from the ClojureScript community as outlined over there.

Comment by David Nolen [ 14/Sep/15 10:42 AM ]

This ticket is about generating docs for Clojure code. Getting autodoc to work for ClojureScript files is worth pursuing but unrelated to this ticket.

Comment by Sebastian Bensusan [ 11/Oct/15 5:54 PM ]

I took at stab at this and only got it running using autodoc-0.9.0-standalone.jar from the command line. My results are not useful at all but those issues should be sorted out in autodoc.

David, do you have a preference in how the docs and artifacts needed should be managed? Should it be a lein plugin or can it be a script that assumes that the correct jars have been installed?

Comment by Tom Faulhaber [ 12/Oct/15 12:37 AM ]

Oh, I did misunderstand this and then didn't see David Nolen's follow-up until now. Let me take a look at whether I can make this happen pretty easily. I wouldn't think it would be too difficult. (Famous last words!)

Comment by Tom Faulhaber [ 02/Jul/16 2:14 AM ]

I have just closed the blocking issue in autodoc Issue 21, andSebastian Bensusan has successfully built a version of doc for the src/main/clojure/... stuff.

The next step is to flesh out what we want to push to http://clojure.github.io/clojurescript. I don't think that this is too hard. Then we can integrate it with the autodoc robot and get automatic updates.





[CLJS-1443] ES6 Module Processing at individual :foreign-lib spec Created: 09/Sep/15  Updated: 08/Nov/16

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

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


 Description   

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






[CLJS-1402] Source Mapping Closure Error Logger Created: 08/Aug/15  Updated: 08/Nov/16

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

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


 Description   

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






[CLJS-1373] Generalize CLJS-1324, check invokes of all IFn implementors Created: 28/Jul/15  Updated: 08/Nov/16

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

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


 Description   

We currently track all IFn implementors but in order to do arity checking of statically analyzeable invokes of keywords, vector, etc. we need to do a bit more. extend-type should update the type in the compiler state with :method-params :max-fixed-arity and :variadic. Then we can just reuse the existing checks in cljs.analyzer/parse-invoke.






[CLJS-1328] Support defrecord reader tags Created: 04/Jul/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reader, readertags


 Description   

Currently, defrecord instances print similar to how they do in clojure

> (pr-str (garden.units/px 5))
#garden.types.CSSUnit{:unit :px, :magnitude 5}

This representation cannot be read by the compiler, nor at runtime by cljs.reader/read-string

> #garden.types.CSSUnit{:unit :px, :magnitude 5}
clojure.lang.ExceptionInfo: garden.types.CSSUnit {:type :reader-exception, :line 1, :column 22, :file "NO_SOURCE_FILE"}
...
> (cljs.reader/read-string "#garden.types.CSSUnit{:unit :px, :magnitude 5}")
#<Error: Could not find tag parser for garden.types.CSSUnit in ("inst" "uuid" "queue" "js")>
...

Analysis

The two requirements - using record literals in cljs source code and supporting runtime reading - can be addressed by using the analyzer to find defrecords and registering them with the two respective reader libraries.

Record literals

Since clojurescript reads and compiles a file at a time, clojure's behavior for literals is hard to exactly mimic. That is, to be able to use the literal in the same file where the record is defined.
A reasonable compromise might be to update the record tag table after each file has been analyzed. Thus the literal form of a record could be used only in requiring files.

EDIT: Record literals can also go into the constant pool

cljs.reader

To play well with minification, the ^:export annotation could be reused on defrecords, to publish the corresponding reader tag to cljs.reader.

Related Tickets



 Comments   
Comment by David Nolen [ 08/Jul/15 12:00 PM ]

It's preferred that we avoid exporting. Instead we can adopt the same approach as the constant literal optimization for keywords under advanced optimizations. We can make a lookup table (which won't pollute the global namespace like exporting does) which maps a string to its type.

I'm all for this enhancement.





[CLJS-1300] REPLs do no write out updated deps.js when compiling files Created: 05/Jun/15  Updated: 08/Nov/16

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

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

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

 Description   

For example a user may edit a file including a new dependency. This will work at the REPL but if a browser refresh is made the emitted goog.require will fail due to the initial deps.js file being stale.



 Comments   
Comment by ewen grosjean [ 05/Dec/15 4:15 PM ]

load-file is broken into 4 sub-functions:
repl-compile-cljs: compile the cljs file beeing loaded
repl-cljs-on-disk: ensures all dependencies are on disk
refresh-cljs-deps: refreshes the cljs_deps.js file
repl-eval-compiled: eval the compiled file

Comment by David Nolen [ 05/Dec/15 9:02 PM ]

Thanks will review.

Comment by Mike Fikes [ 31/Jan/16 3:25 PM ]

cljs-1300.patch no longer applies on master





[CLJS-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 08/Nov/16

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

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


 Description   

Instead of forcing users to refresh browser and lose application state, the browser REPL should poll once a second to connect if connection is unreachable for some reason.



 Comments   
Comment by David Nolen [ 21/Mar/15 8:56 PM ]

This is firmly a major nice-to-have, but not a blocker.





[CLJS-1141] memoization of js-dependency-index and get-upstream-deps needs knobs Created: 18/Mar/15  Updated: 08/Nov/16

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

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

Attachments: Text File CLJS_1141.patch     Text File CLJS-1141-with-js-dep-caching-latest.patch    

 Description   

knobs should be exposed for more dynamic compilation environments like Figwheel which may desire to add dependencies to the classpath on the fly.



 Comments   
Comment by Bruce Hauman [ 21/Mar/15 3:51 PM ]

A patch that caches upstream dependencies in the compiler env.

Comment by Bruce Hauman [ 21/Mar/15 3:59 PM ]

Actually I'm going to submit another patch that includes the memoize calls in js-deps.

Comment by Bruce Hauman [ 28/Mar/15 12:50 PM ]

New patch that moves cljs.js-deps memoization to current env/compiler as well as get-upstream-deps.

Unfortunately there is a circular dep between cljs.env and cljs.js-deps, if we want to cache in env/compiler. I overcame this with a resolve.

Compile performance is either completely unchanged or slightly improved based on several test runs.

Comment by Bruce Hauman [ 28/Mar/15 2:22 PM ]

Hold off on this. Its not behaving as expected. Doesn't seem to be caching in certain situations.

Comment by David Nolen [ 28/Mar/15 2:26 PM ]

Thanks for the update. This will definitely not land until after the pending REPL/piggieback release anyhow.

Comment by Bruce Hauman [ 28/Mar/15 2:44 PM ]

Yeah there is an obvious bug and a subtle one. Hopefully will finish it up soonish.

Comment by Bruce Hauman [ 28/Mar/15 3:43 PM ]

Alright, this latest patch works. There was a subtle memoizing nil value bug.





[CLJS-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 08/Nov/16

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

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: bug
Environment:

Originally found with [org.clojure/clojurescript "0.0-2496"]
Still reproducible with the latest cljsc (b5e9a5116259fc9f201bee4b9c6564f35306f9a5)



 Description   

Here is a minimal test case that produces the invalid Javascript:

(defn f []
  (let [a 0]
    ^{"meta" "data"}
    (fn [] true)))

The compiled Javascript includes the invalid token sequence "return return". (Per Chrome: Uncaught SyntaxError: Unexpected token return)

The problem does not occur if the metadata applies to a map literal instead of a function literal.
The problem only occurs when the function and metadata are inside of a let.



 Comments   
Comment by Bobby Eickhoff [ 07/Jan/15 9:45 PM ]

I forgot to try with-meta. Using with-meta does not produce this syntax error, so it's only a problem with the reader macro for metadata.

Comment by David Nolen [ 08/Jan/15 7:41 AM ]

Any quick thoughts about this one Nicola? Quite possibly a compiler issue on the CLJS side.

Comment by Nicola Mometto [ 08/Jan/15 8:07 AM ]

David, I understand why this happens but I don't know enough about how cljs's js emission to propose a fix.
The issue is that with this commit: https://github.com/clojure/clojurescript/commit/d54defd32d6c5ffcf6b0698072184fe8ccecc93a the following scenario is possible:

{:op :meta
 :env {:context :return}
 :expr {:op :fn
        :env {:context :expr}
        :methods [{:op :fn-method 
                   :env {:context :return} ..}]
        ..}
 ..}

i.e. analyze-wrap-meta changes the context of the :fn node to :expr but keeps the context of the :fn-methods to :return.

This causes both
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L575-L576
and
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L488 (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L233)

to be true and emit a "return".

Comment by David Nolen [ 06/May/15 7:15 PM ]

Hrm, it appears analyze-wrap-meta may need to defer to a helper to change the :context of the given AST node.

Comment by Herwig Hochleitner [ 11/Dec/15 10:52 AM ]

I just randomly ran into this, when upgrading an old project. There is also a duplicate already: http://dev.clojure.org/jira/browse/CLJS-1482

Comment by Jonathan Chu [ 28/Jan/16 6:19 PM ]

This issue occurs for me even without a let.

(fn []
  ^{"meta" "data"}
  (fn [] true))

gives me

#object[SyntaxError SyntaxError: Unexpected token return]




[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 08/Nov/16

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

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

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    

 Description   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.



 Comments   
Comment by Samuel Miller [ 10/Aug/15 10:06 PM ]

Took some time to look at this issue. Originally thought "Do what loop/recur does" but that does not take into account multi-arity. It seems like maybe the best option is to somehow use the second pass of the analyze(analyze-fn-methods-pass2). The entire information about the function is present and the warning section of the code gets triggered but because of no-warn is ignored. Any other ideas for a solution to this?

Comment by Samuel Miller [ 14/Nov/15 7:47 PM ]

So I am looking for feed back on this patch and I will try to explain the reasoning for each section.

The issue is that a function only knows about it's arity after it has been parsed once.
So we need to check arity issues on the second pass

First off, added two new variables.
-activate-second-pass-warnings:Boolean Basically if you want to have second-pass warnings turned on
-second-pass-cljs-warnings:Set Right now we only have :fn-arity but I figure might as well make it generic.

So first up if the modifications to the analyze-fn-methods-pass2 function.
Instead of using no-warn marco here we have some new functionality.
The goal is to turn everything off except the second-pass warnings

So if activate-second-pass-warnings is false just use no-warn else it will use the new section of code.

The default-warning-handler was also modified. After checking if a warning is on, it checks if the warning is a second-pass warning and
if that warning can now be activated. If activate-second-pass-warnings is false AND a warning is still on that implies it is a second pass warning
in the second pass so we activate it.

Also I tried to keep all modifications in cljs.analyzer.

Originally I had the cljs-warnings :fn-arity to false and it would only be turned on in the second pass.
However the repl section just sets everything to true (and turns off select parts like ns errors).
So I decided to not touch those sections and instead keep how other files interface with the analyzer the same.

Comment by Samuel Miller [ 16/Nov/15 10:58 PM ]

Just realized that I have the patch marked as .md instead of .patch





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 08/Nov/16

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

Type: Defect Priority: Major
Reporter: Kevin Marolt Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When a this-as expression gets put in a "scoping function", e.g. in a let-binding, the value bound via this-as refers to the scoping function, and not to the outer scope.

Example:

(def foo
  (js-obj
    "bar" "baz"
    "getBarRight" (fn [] (this-as self (.-bar self)))
    "getBarWrong" (fn []
                    (let [bar (this-as self (.-bar self))]
                      bar))))
     
(.log js/console (.getBarRight foo)) ;; => "baz"
(.log js/console (.getBarWrong foo)) ;; => undefined

Whereas foo.getBarRight expands to something like

function() {
  var self = this; // this refers to foo
  return self.bar; // returns "bar"
}

foo.getBarWrong on the other hand expands to

function() {
  var bar = function() {
    var self = this; // this refers to enclosing function
    return self.bar; // returns undefined
  }();
  return bar; // returns undefined
}





[CLJS-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 08/Nov/16

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

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


 Description   

We need to recur on the first segment passing an new additional argument to resolve-var indicating that we should not try to resolve in the current namespace and instead warn.






[CLJS-1693] rel-output-path produces wrong path for :lib files Created: 25/Jun/16  Updated: 25/Jun/16

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

Type: Defect Priority: Major
Reporter: Ruslan Prokopchuk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Bug presents in any environment, but break things only on Windows.



 Description   

Building with a cljsjs package where cljsjs package includes :libs in deps.cljs you find the "out" directory includes a subdirectory "file:".

An example from my specific case:
"out/file:/home/vagrant/.m2/repository/cljsjs/openlayers/3.3.0-0/openlayers-3.3.0-0.jar!/cljsjs/development/openlayers/ol/array.js"

':' is not permitted on Windows filesystem, which leads to compilation error.

It happens because rel-output-path here https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1515 detects closure-lib first (before taking in account that it is in jar) and passes it to lib-rel-path which acts as if file is located in project directory.

Another issue, but deeply related, is that on Windows, lib-rel-path breaks build even for :lib files located in project directory. It happens because of naive path prefix removal https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1500 because it tries to remove a subpath with native path separators, but path comes here in url-like form with forward slashes. Compiler then reports that path is not relative and aborts compilation.






[CLJS-1806] build api fails to generate inline code for :target :nodejs Created: 01/Oct/16  Updated: 30/Oct/16

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

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

when inline code is provided as vector to the composition of `cljs.build.api/build` and `cljs.build.api/inputs` methods under `:target :nodejs` the provided inline code is not output.

;; this outputs code
(cljs.build.api/build (cljs.build.api/inputs '[(ns hello.core) (+ 1 2)]) {}) 

;; this does not output inline code
(cljs.build.api/build (cljs.build.api/inputs '[(ns hello.core) (+ 1 2)]) {:target :nodejs}) 


;; When you don't use cljs.build.api/inputs everything works correctly
(cljs.build.api/build '[(ns hello.core) (+ 1 2)] {}) ;; this outputs code

(cljs.build.api/build '[(ns hello.core) (+ 1 2)] {:target :nodejs}) ;; this outputs code


 Comments   
Comment by Bruce Hauman [ 30/Oct/16 11:31 AM ]

From @ykomatsu on Github:

add-preloads seems to remove cljs/nodejs.cljs.

https://github.com/clojure/clojurescript/blob/ab7a4911f1fd3a81210b1a9f2d84857748f8268b/src/main/clojure/cljs/closure.clj#L897

This patch will fix this problem but I am not sure if this is correct solution.

https://github.com/ykomatsu/clojurescript/commit/fc986467e66e6a628dc8f0e8a2ef2b30f715fd23





[CLJS-1497] `find` on an associative collection does not return collection key Created: 30/Nov/15  Updated: 18/Nov/16

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

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

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

 Description   

Instead find returns the passed in key. This means metadata on the key will appear to be lost. Related to CLJS-1496.



 Comments   
Comment by Rohit Aggarwal [ 15/Jun/16 3:39 PM ]

Proposed Plan

IAssociative protocol has a function called -entry-at which has been commented out. This function needs to be implemented which will return the necessary data structure, similar to the way it has been done in Clojure.

An example of its implementation for PersistentArrayMap is:

(-entry-at
 [coll k]
 (let [idx (array-map-index-of coll k)]
   (when-not (neg? idx)
     [(aget arr idx) (aget arr (inc idx))])))

We will need to implement this for all the collections which implement that protocol.

A failing test case:

(deftest test-find-meta-cljs-1497
  (let [k        [1 2 3]
        m        {:my "meta"}
        v        1
        xs       {(with-meta k m) v}
        [k' v']  (find xs k)]
    (is (= k k'))
    (is (= v v'))
    (is (= m (meta k')))))
Comment by António Nuno Monteiro [ 13/Nov/16 10:23 AM ]

Attached patch with proposed fix and tests.

Comment by Alex Miller [ 18/Nov/16 7:50 PM ]

dnolen's comment was lost here in a system migration, but he said: "The proposed patch is a breaking change for people who implement custom collections. We need a new protocol `IEntryAt` or something like this."





[CLJS-1074] Externs inference Created: 02/Mar/15  Updated: 01/Mar/16

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

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


 Description   

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

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



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

Is this still being worked on?

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

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

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

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

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

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

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

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

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

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

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

Thanks David. Will have a closer look asap.





[CLJS-1297] defrecord does not emit IKVReduce protocol Created: 03/Jun/15  Updated: 08/Nov/16

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

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1297-19-July-2015.patch    

 Description   

Records are maps and in Clojure they support reduce-kv (IKVReduce protocol).
This is not true in ClojureScript:

(defrecord Foobar [x y])
 (reduce-kv assoc {} (Foobar. 1 2))

Fails wit Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]



 Comments   
Comment by David Nolen [ 03/Jun/15 7:25 PM ]

Just seems like an oversight. Patch welcome, this one is a relatively easy one.

Comment by Daniel Skarda [ 04/Jun/15 2:53 AM ]

OK

I checked Clojure implementation. Records do not implement any reduce protocol on their own. For IKVReduce records use default implementation using reduce and destructuring. Is this approach OK?

Recently Alex Miller implemented many optimizations of reduce protocols in Clojure. Eg range returns an object which implements IReduce protocol so reduce (and transducers in general) can take advantage of it. Any plans for such optimizations in ClojureScript?

;;clojure/src/clj/clojure/core.clj:6523
;;slow path default
clojure.lang.IPersistentMap
(kv-reduce 
  [amap f init]
  (reduce (fn [ret [k v]] (f ret k v)) init amap))
Comment by David Nolen [ 04/Jun/15 9:05 AM ]

Going with the Clojure implementation is fine. Yes all of the optimizations in 1.7.0 are on the table for ClojureScript but these are separate issues from this one.

Comment by Samuel Miller [ 16/Jul/15 10:39 PM ]

Mind if I take this as my first cljs bug? Poking around quickly I think I know what needs to happen.

Comment by David Nolen [ 17/Jul/15 5:21 AM ]

Sure! Have you submitted your CA yet?

Comment by Samuel Miller [ 17/Jul/15 7:13 PM ]

Yes, I did yesterday.

Comment by Samuel Miller [ 20/Jul/15 9:52 PM ]

Here is a potential patch. I implemented a basic IKVreduce based on Daniel Skarda's comment. Note: I am a little fuzzy on macros still so please look over what I have. There is probably a better way. Also added a test for reduce-kv on records.

I ran the test on Linux on V8 and SpiderMonkey. I plan to get JSC and Nashorn working and tested this week but if someone wants to test them out before that would be great.

Comment by Sebastian Bensusan [ 23/Jul/15 6:45 PM ]

Experience report:

I just tested the patch in the Node Repl and it seems to work:

cljs.user=> (defrecord A [a b])
cljs.user/A
cljs.user=> (reduce-kv (fn [m k v] (assoc m k (inc v))) {} (A. 1 2))
{:a 2, :b 3}

and the provided tests passed in Spidermonkey, V8, and Nashorn (I don't have JSC installed).

For completeness: before applying the patch the same code fails with:

Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]
Comment by David Nolen [ 10/Aug/15 10:22 PM ]

Is this the same approach taken by Clojure?

Comment by Samuel Miller [ 10/Aug/15 10:36 PM ]

You can see the relevant current Clojure code here...
https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L6526
I think it is the same. I literally just tried to translate it over into CLJS. I might of understood something wrong though.

Comment by David Nolen [ 11/Aug/15 6:10 AM ]

Yes that's the slow path. Please use the implementation used by defrecord instead. If defrecord doesn't have one then this patch is OK.

Comment by Samuel Miller [ 11/Aug/15 8:48 PM ]

As far as I can tell there is no implementation on defrecord itself however there are separate implementations on the the java classes PersistentVector, PersistentArrayMap, PersistentTreeMap, and PersistenHashMap in pure java. I am not sure if you would want to do something similar for Clojurescript.

I can also spend some time trying to make a more performant version.

Comment by António Nuno Monteiro [ 27/Jul/16 7:38 AM ]

Confirmed that Clojure uses the slow path via the IPersistentMap implementation in defrecord
https://github.com/clojure/clojure/blob/d920ad/src/clj/clojure/core.clj#L6712

Patch still applies and can also confirm it works for me.





[CLJS-1701] cljs.spec impact on :advanced builds Created: 07/Jul/16  Updated: 07/Jul/16

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

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


 Description   

Investigate the impact of cljs.spec on :advanced builds.

Currently all specs are kept in the (private) cljs.spec/registry-ref atom. This atom is not understood by the Closure Compiler and cannot be eliminated as dead code. So even if specs are not used in "production" they still bloat the generated JS size. Some specs may be used at runtime and cannot not be removed, the gen parts however are probably never required in :advanced builds and should be omitted somehow.

In a test build (with 1.9.93) this adds 11kb (102kb vs 91kb) as soon as cljs.spec is :require'd somewhere and goes up with each defined spec.






[CLJS-246] Use protocol mask test in protocol fns Created: 09/May/12  Updated: 29/Jul/13

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Michał Marczyk
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-246-have-protocol-methods-check-bitmasks-for-fa.patch    

 Description   

This is a performance win on many browsers.

http://jsperf.com/direct-vs-chain/8



 Comments   
Comment by Michał Marczyk [ 10/May/12 1:11 PM ]

See CLJS-247 for comments relevant to this patch.

Comment by David Nolen [ 10/May/12 3:30 PM ]

Not seeing much of a perf benefit from this, though Michal reports differently. More investigation is needed.

Comment by David Nolen [ 17/Jun/12 12:14 PM ]

patch no longer applies. I wonder if I see bad behavior because I was testing with node or it was prior to the fixes around avoiding deoptimization.

Comment by Michał Marczyk [ 17/Jun/12 9:28 PM ]

I'll bring it up to date with the recent changes, thanks for the prod!





[CLJS-338] Incorrect implementation of IReduce by ArrayChunk Created: 22/Jul/12  Updated: 29/Jul/13

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

Type: Defect Priority: Minor
Reporter: Anton Frolov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   
(reduce + (array-chunk (array 1 2 3 4) 1 3)) => 2 (instead of 5)
(reduce + 0 (array-chunk (array 1 2 3 4) 1 3)) => 3 (instead of 5)
(reduce + (array-chunk (array 1 2 3 4) 1 1)) => 2 (instead of 0)

In src/cljs/cljs/core.cljs, line #1817:

(deftype ArrayChunk [arr off end]
  ;; ...
  IReduce
  (-reduce [coll f]
    (ci-reduce coll f (aget arr off) (inc off))) ;; should be (if (< off end) (ci-reduce coll f (aget arr off) 1) 0)
  (-reduce [coll f start]
    (ci-reduce coll f start off))) ;; should be (ci-reduce coll f start 0)


 Comments   
Comment by David Nolen [ 14/Aug/12 6:29 AM ]

Thanks for the report. ArrayChunk is an implementation detail - do these conditions actually arise?





[CLJS-650] Optimize all protocols Created: 01/Nov/13  Updated: 01/Nov/13

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

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


 Description   

We should be optimizing all protocols insteads of just putting the core protocols on the fast path. In the current design we put the protocol mask on instance - this wastes a considerable amount of space, instead we should be putting it on the prototype. This benchmark appears to show no performance hit for this approach jsperf.com/prototype-bit-mask.

In order to have fewer tests satisfies? and protocol fns should generate different code for the different compilation modes - in anything but advanced we should just use the boolean property on the prototype, in advanced we should use the bit mask approach.






[CLJS-720] #queue literal behavior is incorrect Created: 07/Dec/13  Updated: 07/Dec/13

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


 Description   

In order for queue to work we need to adopt an approach similar to the one for #js data literals - i.e. needs special casing in the analyzer since queues are not "atomic" values.






[CLJS-150] Regular expressions don't support Javascript mode flags Created: 16/Feb/12  Updated: 12/Mar/14  Due: 24/Feb/12

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

Type: Defect Priority: Minor
Reporter: Bobby Calderwood Assignee: Bobby Calderwood
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently, the compiler and cljs.core allow for Java mode flags. Javascript doesn't support many of these, and supports one flag not supported by Java - 'g'.

ClojureScript regular expressions should support only Javascript regex mode flags: 'i', 'm', and 'g'. This applies to Regex literals in the compiler as well as (re-pattern).

This is a defect in the implementation of CLJS-116.



 Comments   
Comment by David Nolen [ 16/Feb/12 3:33 PM ]

The defect existed prior to CLJS-116. The problem is that we're using the Clojure reader and g is not a valid flag for a Java RegexPattern.

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

This ticket should be rejected. A regular expression created with the global flag is stateful (i.e., the lastIndex property is checked and used by the exec and test methods.) On sufficiently old browsers (pre js 1.5), this makes the RegExp object itself stateful, i.e., not instances, but the RegExp constructor is mutated!

Using a regex with the global flag set will already ruin the results of re-seq, re-find, etc. I could see re-seq using a clone of the input regex with the global flag set as an optimization to avoid string slicing, but we certainly shouldn't provide a public interface to create them.

See also CLJS-776





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

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

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

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

 Description   

1. This currently doesn't work:

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

I tried running the following code:

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

Some of the last results I got were:

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

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

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

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

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

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

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

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

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

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

So here are the results.

For vectors:

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

Patch 1: 205872 msecs
Patch 2: 210756 msecs

For arrays:

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

Patch 1: 123567 msecs
Patch 2: 119704 msecs

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

This makes sense.

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

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

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

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

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

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

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

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

Yeah, benchmarking with Rhino isn't informative.

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

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

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

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

Firefox:

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

Chrome:

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

Safari:

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

I'm not sure what to make of this.

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

I have attached a new version of the first patch.

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

This patch also fixes r/fold for arrays.

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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






[CLJS-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-1067] Shared AOT cache for dependencies in JARs Created: 26/Feb/15  Updated: 27/Feb/15

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


 Description   

3rd party library code in JARs shouldn't be recompiled across dev and prod configurations. There should be a shared AOT cache for all builds within a project for all non-project local source.






[CLJS-1123] this-as unexpectedly binds js/window when used within function with post-condition Created: 15/Mar/15  Updated: 16/Mar/15

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

Type: Defect Priority: Minor
Reporter: J. S. Choi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Adding a post-condition to any function that uses cljs.core/this-as will unexpectedly cause this-as's "this" symbol to be bound to the root object (e.g., js/window) instead.

(defn f-no-post-condition [argument]
  (this-as this
    (js/console.log argument this)))

(defn f-with-post-condition [argument]
  {:post [true]}
  (this-as this
    (js/console.log argument this)))

(def test-object
  #js {:methodNoPostcondition f-no-post-condition
       :methodWithPostcondition f-with-post-condition})

(f-with-post-condition "A") ; Correctly prints js/window
(.methodNoPostcondition test-object "B") ; Correctly prints test-object
(.methodWithPostcondition test-object "C") ; Incorrectly prints js/window


 Comments   
Comment by David Nolen [ 16/Mar/15 6:17 AM ]

This is almost certainly a different manifestation of CLJS-719.

Comment by Thomas Heller [ 16/Mar/15 6:21 AM ]

Just looked at the generated javascript. As David mentioned the problem is the extra function generated to get the result for the :post condition.

dummy.f_no_post_condition = (function f_no_post_condition(argument){
var this$ = this;
var G__82157 = argument;
var G__82158 = this$;
return console.log(G__82157,G__82158);
});
dummy.f_with_post_condition = (function f_with_post_condition(argument){
var _PERCENT_ = (function (){var this$ = this;
var G__82161 = argument;
var G__82162 = this$;
return console.log(G__82161,G__82162);
})();


return _PERCENT_;
});
dummy.test_object = {"methodWithPostcondition": dummy.f_with_post_condition, "methodNoPostcondition": dummy.f_no_post_condition};
dummy.f_with_post_condition("A");
dummy.test_object.methodNoPostcondition("B");
dummy.test_object.methodWithPostcondition("C");




[CLJS-1151] Noisy errors when referring to symbol in undefined ns Created: 19/Mar/15  Updated: 22/Mar/15

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

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

Quick Start / OS X / Chrome



 Description   

Run through the Quick Start to the point where you have the browser REPL running.

If you refer to a symbol in an undefined ns you will get really noisy errors:

ClojureScript:cljs.user> foo.bar/a
WARNING: Use of undeclared Var foo.bar/a at line 1 <cljs repl>
ReferenceError: foo is not defined
ReferenceError: foo is not defined
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:1:89)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:9:3)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:14:4)
    at http://localhost:9000/out/clojure/browser/repl.js:42:267
    at clojure$browser$repl$evaluate_javascript (http://localhost:9000/out/clojure/browser/repl.js:45:4)
    at Object.callback (http://localhost:9000/out/clojure/browser/repl.js:242:169)
    at goog.messaging.AbstractChannel.deliver (http://localhost:9000/out/goog/messaging/abstractchannel.js:142:13)
    at goog.net.xpc.CrossPageChannel.xpcDeliver (http://localhost:9000/out/goog/net/xpc/crosspagechannel.js:733:12)
    at Function.goog.net.xpc.NativeMessagingTransport.messageReceived_ (http://localhost:9000/out/goog/net/xpc/nativemessagingtransport.js:321:13)
    at Object.goog.events.fireListener (http://localhost:9000/out/goog/events/events.js:741:21)

Now, define the namespace, but don't actually define the symbol. For example:

(ns foo.bar)

(def z 3)

Now, require the namespace and do the same. The noise will go away.

ClojureScript:cljs.user> (require 'foo.bar)
nil
ClojureScript:cljs.user> foo.bar/a
WARNING: Use of undeclared Var foo.bar/a at line 1 <cljs repl>
nil

Additional info: Now if you attempt to refer to other unknown symbols, you will get different kinds of "noise" depending on whether the ns simply starts with foo:

ClojureScript:cljs.user> foo.baz/a
WARNING: No such namespace: foo.baz, could not locate foo/baz.cljs at line 1 <cljs repl>
WARNING: Use of undeclared Var foo.baz/a at line 1 <cljs repl>
TypeError: Cannot read property 'a' of undefined
TypeError: Cannot read property 'a' of undefined
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:1:96)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:9:3)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:14:4)
    at http://localhost:9000/out/clojure/browser/repl.js:42:267
    at clojure$browser$repl$evaluate_javascript (http://localhost:9000/out/clojure/browser/repl.js:45:4)
    at Object.callback (http://localhost:9000/out/clojure/browser/repl.js:242:169)
    at goog.messaging.AbstractChannel.deliver (http://localhost:9000/out/goog/messaging/abstractchannel.js:142:13)
    at goog.net.xpc.CrossPageChannel.xpcDeliver (http://localhost:9000/out/goog/net/xpc/crosspagechannel.js:733:12)
    at Function.goog.net.xpc.NativeMessagingTransport.messageReceived_ (http://localhost:9000/out/goog/net/xpc/nativemessagingtransport.js:321:13)
    at Object.goog.events.fireListener (http://localhost:9000/out/goog/events/events.js:741:21)
ClojureScript:cljs.user> goo.baz/a
WARNING: No such namespace: goo.baz, could not locate goo/baz.cljs at line 1 <cljs repl>
WARNING: Use of undeclared Var goo.baz/a at line 1 <cljs repl>
ReferenceError: goo is not defined
ReferenceError: goo is not defined
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:1:89)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:9:3)
    at eval (eval at <anonymous> (http://localhost:9000/out/clojure/browser/repl.js:42:272), <anonymous>:14:4)
    at http://localhost:9000/out/clojure/browser/repl.js:42:267
    at clojure$browser$repl$evaluate_javascript (http://localhost:9000/out/clojure/browser/repl.js:45:4)
    at Object.callback (http://localhost:9000/out/clojure/browser/repl.js:242:169)
    at goog.messaging.AbstractChannel.deliver (http://localhost:9000/out/goog/messaging/abstractchannel.js:142:13)
    at goog.net.xpc.CrossPageChannel.xpcDeliver (http://localhost:9000/out/goog/net/xpc/crosspagechannel.js:733:12)
    at Function.goog.net.xpc.NativeMessagingTransport.messageReceived_ (http://localhost:9000/out/goog/net/xpc/nativemessagingtransport.js:321:13)
    at Object.goog.events.fireListener (http://localhost:9000/out/goog/events/events.js:741:21)


 Comments   
Comment by Mike Fikes [ 19/Mar/15 3:47 PM ]

Clarification on the description: When defining the foo.bar ns, I actually created a source file src/foo/bar.cljs containing the ns declaration and the def z.

Comment by David Nolen [ 22/Mar/15 9:49 AM ]

I'm not sure what we can do about this or that suppressing the errors from the JavaScript evaluation environment is a good idea. Property access on something that doesn't exist is what causes the JS environment to emit the error. In the later cases the property access is on something that does exist but doesn't have the property so no error.





[CLJS-1211] Automatically requiring :main namespace under :none fails in IE9 Created: 17/Apr/15  Updated: 17/Apr/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3058, 0.0-3115, 0.0-3196, 0.0-3117, 0.0-3119, 0.0-3123, 0.0-3126
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Immo Heikkinen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Internet Explorer 9



 Description   

Automatic goog/base.js inclusion using :none & :main doesn't work in Internet Explorer 9. The following error message is printed to the console:

ClojureScript could not load :main, did you forget to specify :asset-path?

There seems to be a timing issue after writing script tags to the HTML document. Inline JavaScript for requiring the main namespace gets executed while goog is still undefined.

I played a bit with this but couldn't get it working in IE9. I tried moving the require statement to a separate JS file and adding a script tag to load that file, then goog was no longer undefined but I still got an error message:

SCRIPT5022: Undefined nameToPath for goog.string
base.js, line 753 character 9

The feature seems to work fine in other browsers (also IE10). Probably not worth fixing but at least the limitation is documented now in case someone else wonders the why it doesn't work in IE9.






[CLJS-890] Incorrect behaviour of (str obj) when obj has valueOf method Created: 24/Nov/14  Updated: 25/Apr/15

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

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

Attachments: Text File cljs-890.patch     File cljs-core-str-perf.diff    

 Description   

Example

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

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

Example in js:

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

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



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

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

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

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

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

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

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

Thanks for the link. I see what you mean.

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

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

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

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

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

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

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

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

reverted CLJS-801 in master

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

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

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

Options as I see it are:

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

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

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

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

Details:

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

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

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

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

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

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

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

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

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

To be clear, there are two issues here:

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

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

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

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

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

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

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

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

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

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

Yes, that works. Cool, thanks!

Comment by Thomas Heller [ 01/Jan/15 7:12 AM ]

Sorry for re-opening.

I was doing some profiling of my code and noticed a warning in the profiling output about cljs.core/str.

Chromes complains with: "Not optimized. Bad value context for arguments value", looking further at the implementation of goog.string.buildString

goog.string.buildString = function(var_args) {
  return Array.prototype.join.call(arguments, '');
};

Given that we don't ever call it with more than one argument it is probably not best implementation choice.

Maybe skip the call and inline it ala

(defn str
  "With no args, returns the empty string. With one arg x, returns
  x.toString().  (str nil) returns the empty string. With more than
  one arg, returns the concatenation of the str values of the args."
  ([] "")
  ([x] (if (nil? x)
         ""
         (.join #js [x] "")))
  ([x & ys]
    (loop [sb (StringBuffer. (str x)) more ys]
      (if more
        (recur (. sb  (append (str (first more)))) (next more))
        (.toString sb)))))

I didn't follow this issue but why are we not using .toString? The buildString/array approach seems kind of hackish?

I'm not too sure about the overall impact but since cljs.core/str showed up pretty high in my profile I think this should be investigated further.

Comment by Thomas Heller [ 01/Jan/15 7:28 AM ]

Before:

;;; str
[], (str "1"), 1000000 runs, 254 msecs
[], (str 1), 1000000 runs, 266 msecs
[], (str nil), 1000000 runs, 80 msecs
[], (str "1" "2" "3"), 1000000 runs, 753 msecs

After:

;;; str
[], (str "1"), 1000000 runs, 82 msecs
[], (str 1), 1000000 runs, 86 msecs
[], (str nil), 1000000 runs, 79 msecs
[], (str "1" "2" "3"), 1000000 runs, 242 msecs

But I only tested V8, probably needs some verification.

Comment by Thomas Heller [ 01/Jan/15 7:39 AM ]
(defn str
  "With no args, returns the empty string. With one arg x, returns
  x.toString().  (str nil) returns the empty string. With more than
  one arg, returns the concatenation of the str values of the args."
  ([] "")
  ([x1]
     (.join #js [x1] ""))
  ([x1 x2]
     (.join #js [x1 x2] ""))
  ([x1 x2 x3]
     (.join #js [x1 x2 x3] ""))
  ([x1 x2 x3 x4]
     (.join #js [x1 x2 x3 x4] ""))
  ...)

Does perform even better.

;;; str
[], (str "1"), 1000000 runs, 40 msecs
[], (str 1), 1000000 runs, 43 msecs
[], (str nil), 1000000 runs, 96 msecs
[], (str "1" "2" "3"), 1000000 runs, 117 msecs

How many args should it inline?

Comment by David Nolen [ 01/Jan/15 12:43 PM ]

I'd be OK with up to 4 then variadic.

Comment by Thomas Heller [ 01/Jan/15 5:05 PM ]

There is some weird interaction between the code generated by the cljs.core/str macro and function.

The macro generates

(str "hello" 1 "world" :yo nil)

yields

[cljs.core.str("hello"),cljs.core.str((1)),cljs.core.str("world"),cljs.core.str(new cljs.core.Keyword(null,"yo","yo",1207083126)),cljs.core.str(null)].join('');

Given that str with 1 arg will basically unroll to

[["hello"].join(""), ...]

I think it might be safe to completely remove the macro since cljs.core/str would then do the same and the JIT is probably smart enough to figure this out (or even Closure when compiling).

Comment by Francis Avila [ 02/Jan/15 11:14 AM ]

Chromes complains with: "Not optimized. Bad value context for arguments value", looking further at the implementation of goog.string.buildString

Chrome complains about the variadic function dispatch code in the same way, see CLJS-916 plus patch.

I think it might be safe to completely remove the macro since cljs.core/str would then do the same and the JIT is probably smart enough to figure this out (or even Closure when compiling).

The Closure compiler is not smart enough to remove the intermediate array, which is why I filed CLJS-801 (which this ticket rolled back). I don't think JITs can do it either.

I am beginning to wonder if we should ignore the Safari 6.0.5 problem in CLJS-847 that started all this string mess. To recap:

  1. CLJS-801 is accepted, which removes [123, x].join('') in the str macro case in favor of ''+123+(cljs.core/str$arity$1 x) style code, which the closure compiler can precompute. At this time, the one-arg cljs.core/str function (not macro) calls toString on its argument.
  2. CLJS-847 is filed. On Safari 6.0.5 at higher JIT levels, calling toString on some things (possibly only unboxed numbers? definitely not strings) throws a TypeError. This is unquestionably a bug in Safari. David fixes by making one-arg cljs.core/str function call js-str instead of toString. js-str uses string-concat ''+x.
  3. However, this breaks for objects that define valueOf (issue in current ticket), because in js ''+x is the same as ''+x.valueOf().toString() not ''+x.toString().
  4. David considers using String() and variations but rejects because of performance hit.
  5. David rolls back CLJS-801 from the string-concat back to the array-join style to fix.
  6. Nikita and I point out that rolling back CLJS-801 only fixes the str macro, not the string function, which still uses js-str and hence string-concat.
  7. David fixes the str function to use goog.string.buildString, which has the behavior of array.join. Behavior is now correct even on Safari 6.0.5.
  8. Thomas points out that buildString uses arguments in a way unoptimizable by v8, and now the str function (not macro) has a performance regression. He suggests using [].join() directly.

So, there's a lot of back and forth on this issue, and it's all because of a bug in Safari 6.0.5 which no one has been able to reproduce first-hand because Safari 6.0.5 is old and rare. For some perspective, Safari 6.0.x was only available on Lion and Mountain Lion between July 25,2012 and June 11,2013. Before July 25,2012 Lion used Safari 5.1.x and there was no Mountain Lion. On June 11, 2013, both Lion and Mountain Lion switched to Safari 6.1.x which does not suffer from the toString TypeError bug (I checked--I have an iMac with Lion on it). The only machines on Safari 6.0.5 are (Mountain) Lion machines which used software updates until late 2012-early 2013 and then stopped. I can't imagine this is a large number of people.

It is theoretically possible for me to run Safari 6.0.x on my Lion machine to actually test this, but I can't find a way to downgrade from 6.1.x.

I think the options are:

  1. Use array.join() for all stringification and take the performance hit (which we should quantify). Include a comment that this is only for Safari 6.0.x (only confirmed second-hand on 6.0.4 and 6.0.5) for future generations, who are going to think this is weird.
  2. Use CLJS-801 and toString (status quo before CLJS-847), and ignore this problem for Safari 6.0.x.
  3. Use CLJS-801, but add a number? check (with comment) to cljs.core/str$arity$1 for Safari 6.0.5. The number case should use js-str, and the rest toString. I think this will work, but again we have no way to test--we really need to get our hands on a Safari 6.0.x browser.

Of course we should benchmark these approaches but my hunch is that 2 is faster than 3 is faster than 1.

Comment by David Nolen [ 02/Jan/15 11:16 AM ]

We are not going to ignore Safari 6.0.X. Any decisions made about this ticket will include supporting it.

Comment by Francis Avila [ 10/Jan/15 4:12 AM ]

Update on some research I am doing into this.

I created a jsperf of alternative str implementations that I am trying out. Right now I've only looked at one-arg str. I discovered a few things:

  • {{''+[x]}} is a faster alternative to [x].join('').
  • Advanced compilation can compute {{''+[x]}} at compile time if x is a bool, str, undefined, null, or number, even through function calls! I.e. str_test.str_arr(123) compiles to "123" without macro magic.
  • However, using an intermediate array (even if a preallocated singleton) is still slower than the old (if (nil? x) "" (.toString x))
  • Using a switch statement is as least as fast as the str-tostr baseline, usually faster.
  • I am 99% sure all these implementations (except str-tostr, the baseline, which definitely fails) work on the dreaded Safari 6.0.x. If anyone has this version, point it at the jsperf link above and run the tests. I think Browserstack has this version of Safari.

I'm still investigating the variadic case (str x y z a b c). It might be better to use reduce instead of Stringbuffer+seq. (Stringbuffer just does ''+x now instead of an array-join.)

Comment by Thomas Heller [ 10/Jan/15 6:37 AM ]

Sorry, got side-tracked for a bit.

@Francis: Thanks for the recap.

Don't have Safari 6 available either, but seems wrong that we all have to suffer because is minor percentage still has this (667 users of 190k+ on my site). Don't have a solution cause I can't test whether it works, we might try String.concat.

"".concat(obj); // "42"
"".concat(obj, ""); // "hello"
String.prototype.concat(obj, "") // "hello"
String.prototype.concat("", obj) // "hello"

But no idea if String.concat works, also it behaves odd with regards to valueOf.

http://jsperf.com/js-string-concat-variants

Perf is also inconclusive since Firefox appears to be cheating.

Comment by Francis Avila [ 10/Jan/15 2:04 PM ]

Tested that jsperf with Safari 6.0.5 using Browserstack, results are there.

Note I could not reproduce CLJS-847 because str-tostr does not fail as expected. I will try harder now that I have a browser to test.

Comment by Francis Avila [ 10/Jan/15 6:55 PM ]

Still cannot reproduce CLJS-847.

This script includes my attempt at a minimum reproducible case. My theory was that certain types at higher jit levels would fail. I could not get any case to fail. I also tried flapping back and forth between types and using one type at a time, but still no failures.

In this thread I found this "minimal" script which the OP said he could get to fail reliably. I could not get it to fail. However the original post was from feb 15, 2013, which means the Safari he was using would have to be 6.0.2 or lower.

Hypotheses:

  1. This error does not affect 6.0.5 but maybe 6.0.4 or lower.
  2. BrowserStack's system somehow mitigates the bug, meaning we need a "real" Lion Safari 6.0.x to test.
  3. These tests only fail under the correct phase of the moon.

So I can code up a patch for str using the str-switch implementation (which is at least a bit faster on some browsers), but I have no idea if it may fail on Safari 6.0.5. I only know that it works so far. CLJS-801 should also be safe to reapply because the root cause of all issues is the implementation 1-arity of the cljs.core/str function.

I have also asked for Kevin's help back in CLJS-847. (Kevin was the original reporter of the Safari 6.0.x issue.)

Comment by Francis Avila [ 19/Jan/15 12:51 AM ]

Made a jsperf of variadic cases. Chrome seems to really prefer IReduce to seq+stringbuilder for vectors (other collections not tested), but there is no difference or a small slowdown on other browsers. Not sure if it's worth it.

Also updated arity-one cases with a str using switch and never using toString. Nearly 50% slower than using switch or toString on Chrome, smaller on Safari.

In terms of safety str-switch-notostr does not use toString at all so is probably safer. I think str-switch will likely work too, though, and is significantly faster. However I haven't been able to get any TypeErrors in Safari 6.0.5 so it's anyone's guess.

I suggest something like this as a new str (which doesn't use reduce, but could):

(defn str
 ([x]
  (case (js* "typeof ~{}" x)
   "string" x
   "object" (if (identical? nil x) "" (.toString x))
   ("boolean" "number") (js-str x)
   "undefined" ""
   (js-str #js [x])))                                       ;; insurance against Safari 6.0.x TypeError bug.
 ([a b] (js* "~{}+~{}" (str a) (str b)))
 ([a b c] (js* "~{}+~{}+~{}" (str a) (str b) (str c)))
 ([a b c d] (js* "~{}+~{}+~{}+~{}" (str a) (str b) (str c) (str d)))
 ([a b c d & more]
  (loop [s (str a b c d) [e f g h & r] more]
   (let [s' (js* "~{}+~{}+~{}+~{}+~{}" s e f g h)]
    (if (nil? r)
     s'
     (recur s' r))))))
Comment by Francis Avila [ 19/Jan/15 11:24 PM ]

First cut of a possible patch that resolves this while not breaking CLJS-847. Should wait for confirmation that this does not break Safari 6.0.x.

Comment by Francis Avila [ 19/Jan/15 11:34 PM ]

Oops forgot tests.

Comment by Francis Avila [ 03/Feb/15 10:24 AM ]

Update in CLJS-847: original reporter was not able to reproduce his original bug report in Safari 6.0.x running in BrowserStack. This may be because of BrowserStack, but it's the best we have.

Given how hard this bug is to reproduce, how few people it affects, and how significant the performance regression is, I still think we should go back to the simple (if (nil? x) "" (.toString x)) implementation. However, you could also try the patch on this ticket (using a typeof switch), which at least (handwaving) might fix this bug in Safari 6.0.x and is a little faster than a simple .toString in Chrome and not much slower elsewhere. (The reason I think it might avoid this bug in Safari is that it avoids calling .toString on non-Objects.)

Comment by Antonin Hildebrand [ 25/Apr/15 11:08 PM ]

I wonder if you considered swapping str function at runtime during CLJS init phase.

Implement str function using plain .toString() call (original solution). And at startup check for Safari 6.0.x presence and optionally swap str for implementation wrapping .toString() call in a try-catch block silencing TypeError exceptions by falling back to Safari 6.0.x friendly .toString() alternative.

We would get correct semantics in all cases. And price would be just slower printing execution on Safari 6.0.x not on all systems.





[CLJS-1091] Compose JavaScript dependency indexes Created: 07/Mar/15  Updated: 29/Apr/15

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


 Description   

Currently hard coded to Google Closure deps.js and the one produced for a build. Users should be able to supply JS dependency indexes that can get merged in.






[CLJS-1245] Implement bound-fn Created: 03/May/15  Updated: 05/May/15

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

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


 Description   

See discussion on http://dev.clojure.org/jira/browse/CLJS-210






[CLJS-1266] Node: Rename .cljs to .cljc -> old filenames in stacktrace Created: 12/May/15  Updated: 12/May/15

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

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


 Description   

Using QuickStart, set up Node REPL.

Manually add a foo/bar.cljs to filesystem with

(ns foo.bar)

(defn throw-ex [] (ffirst 1))

(defn call-me [] (throw-ex))

Check that it works:

cljs.user=> (require 'foo.bar)
nil
cljs.user=> (foo.bar/call-me)
repl:13
throw e__4210__auto__;
      ^
Error: 1 is not ISeqable
    at Object.cljs$core$seq [as seq] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:956:20)
    at Object.cljs$core$first [as first] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:965:16)
    at cljs$core$ffirst (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:1398:11)
    at foo$bar$throw_ex (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljs:3:20)
    at foo$bar$call_me (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljs:5:19)
    at repl:1:105
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)
    at Domain.<anonymous> ([stdin]:41:34)

Then manually move bar.cljs to bar.cljc and add a new symbol so it looks like:

(ns foo.bar)

(defn throw-ex [] (ffirst 1))

(defn call-me [] (throw-ex))

(defn call-again [] (call-me))

Then reload the ns and use the new symbol:

cljs.user=> (require 'foo.bar :reload)
nil
cljs.user=> (foo.bar/call-again)
repl:13
throw e__4210__auto__;
      ^
Error: 1 is not ISeqable
    at Object.cljs$core$seq [as seq] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:956:20)
    at Object.cljs$core$first [as first] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:965:16)
    at cljs$core$ffirst (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:1398:11)
    at foo$bar$throw_ex (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljs:3:20)
    at foo$bar$call_me (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljs:5:19)
    at foo$bar$call_again (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljs:5:19)
    at repl:1:108
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)

This illustrates the defect. call_again and the other symbols are shown as being in the old filename.

Stop the REPL and restart it to see correct behavior:

cljs.user=> :cljs/quit
orion:hello_world-node mfikes$ rlwrap java -cp cljs.jar:src clojure.main node_repl.clj
Reading analysis cache for jar:file:/Users/mfikes/Desktop/hello_world-node/cljs.jar!/cljs/core.cljs
Compiling src/foo/bar.cljc
ClojureScript Node.js REPL server listening on 49397
Watch compilation log available at: out/watch.log
To quit, type: :cljs/quit
cljs.user=> (require 'foo.bar)
nil
cljs.user=> (foo.bar/call-again)
repl:13
throw e__4210__auto__;
      ^
Error: 1 is not ISeqable
    at Object.cljs$core$seq [as seq] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:956:20)
    at Object.cljs$core$first [as first] (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:965:16)
    at cljs$core$ffirst (/Users/mfikes/Desktop/hello_world-node/out/cljs/core.cljs:1398:11)
    at foo$bar$throw_ex (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljc:3:20)
    at foo$bar$call_me (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljc:5:19)
    at foo$bar$call_again (/Users/mfikes/Desktop/hello_world-node/out/foo/bar.cljc:7:22)
    at repl:1:108
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)


 Comments   
Comment by Mike Fikes [ 12/May/15 2:04 PM ]

FWIW as a comparison, the same use case works properly with Clojure 1.7.0-beta2.





[CLJS-1320] clojure.string/split adds separator matches & failed matches (nil) when the separator is a regex with alternation Created: 26/Jun/15  Updated: 27/Jun/15

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

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


 Description   

I want to split a string on "; ", and optionally discard a final ";". So, I tried:

(clojure.string/split "ab; ab;" #"(; )|(;$)")

In Clojure, this does what I want:

["ab" "ab"]

In ClojureScript, I get:

["ab" "; " nil "ab" nil ";"]

I'm not sure to what extent this is a platform distinction and to what extent it's a bug. Returning nils and seperators from clojure.string/split's output seems like it's against string.split's contract?






[CLJS-1278] Asserts still fail while :require-ing .js file (either in :libs or in :source-paths) (same as CLJS-1196) Created: 20/May/15  Updated: 14/Jul/15

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

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

Attachments: Text File cljs_1278.patch    

 Description   

Following on CLJS-1196, I can't get it to work.

In version 0.0-3264 lein-cljsbuild crashed on weird eception `Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil"` but the current version 0.0-3269 gives the same failed assertion as previously.

I've put up a sample project to illustrate the issue.

Steps to reproduce:

`git clone https://github.com/tillda/stackone`
`cd stackone`
`git checkout 537e5c69b844bc53c159e85cafc24310543cc918`
`lein clean && lein cljsbuild once temp`

Expected behaviour: cljs compiled successfully with src/vendor/client/closure.js and env/stackone/helpersjs.js being included.

Actual behaviour:

```
Compiling "resources/public/lein-cljsbuild-temp/dev-mode-deps.js" failed.
Exception in thread "main" java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x)), compiling/private/var/folders/ym/l2qxd7l97kzfzftrdpqsclm40000gn/T/form-init3642888309490821030.clj:1:125)
at clojure.lang.Compiler.load(Compiler.java:7249)
at clojure.lang.Compiler.loadFile(Compiler.java:7175)
at clojure.main$load_script.invoke(main.clj:275)
at clojure.main$init_opt.invoke(main.clj:280)
at clojure.main$initialize.invoke(main.clj:308)
at clojure.main$null_opt.invoke(main.clj:343)
at clojure.main$main.doInvoke(main.clj:421)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:383)
at clojure.lang.AFn.applyToHelper(AFn.java:156)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x))
at cljs.util$ext.invoke(util.cljc:115)
at cljs.closure$source_on_disk.invoke(closure.clj:1206)
at cljs.closure$output_unoptimized$fn__3708.invoke(closure.clj:1235)
at clojure.core$map$fn__4551.invoke(core.clj:2622)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$filter$fn__4578.invoke(core.clj:2677)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$map$fn__4551.invoke(core.clj:2614)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:674)
at clojure.core$next__4110.invoke(core.clj:64)
at clojure.core$str$fn__4186.invoke(core.clj:528)
at clojure.core$str.doInvoke(core.clj:526)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:628)
at cljs.closure$deps_file.invoke(closure.clj:1040)
at cljs.closure$output_deps_file.invoke(closure.clj:1060)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1243)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:630)
at cljs.closure$build.invoke(closure.clj:1514)
at cljs.closure$build.invoke(closure.clj:1426)
at cljsbuild.compiler$compile_cljs$fn__3884.invoke(compiler.clj:81)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:80)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:187)
at user$eval4018$iter_40544058$fn4059$fn_4077.invoke(form-init3642888309490821030.clj:1)
at user$eval4018$iter_40544058$fn_4059.invoke(form-init3642888309490821030.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$dorun.invoke(core.clj:3007)
at clojure.core$doall.invoke(core.clj:3023)
at user$eval4018.invoke(form-init3642888309490821030.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6792)
at clojure.lang.Compiler.eval(Compiler.java:6782)
at clojure.lang.Compiler.load(Compiler.java:7237)
... 11 more
Subprocess failed
```



 Comments   
Comment by David Nolen [ 20/May/15 10:21 AM ]

This issue is in danger of being closed. Please supply minimal steps to reproduce that do not involve anything other than the ClojureScript compiler. We no longer have time to wade through the indirection introduced by cljsbuild or any other downstream tooling. Thanks.

Comment by Michal Till [ 20/May/15 11:14 AM ]

@David Nolen: I have created a failing minimal testcase based on the Quick Start document. Here it is: https://github.com/tillda/cljs-testcase/

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

Michal the failing example is not correct. You are not supplying any :libs option.

Comment by Michal Till [ 20/May/15 11:45 AM ]

Ah! Thank you very much! This additional issue was therefore my error. Now it seems to work even in my "big" example.

However it would be cool if there was a meaningful error message stating that a file path can't be resolved. If one is not an expert in the cljs compiler this is almost impossible to figure out. After all the error message in the CLJS-1196 issue and in this wrongfully reported one are exactly the same.

You may close this issue.

Comment by David Nolen [ 20/May/15 11:55 AM ]

We'll leave it open for the improving the error message.

Comment by Sebastian Bensusan [ 22/May/15 7:16 AM ]

Added the check in cljs.closure/source-on-disk where there is info for the error message.

For the supplied case, the error message is:

java.lang.IllegalArgumentException: The file file:/home/carlos/Playground/cljs-testcase/src/hello_world/closure.js 
lacks an associated source file. If it is a JavaScript library please add it to :libs}}

If a different wording or location of the check is needed, I'll submit a new patch with corrections.

Notes:

  • Changed:(:provides js) to (-provides js) in order to be consistent with IJavaScript.
  • cljs.clojure/source-on-disk takes a js argument that should satisfy with IJavaScript and ISourceMap if :source-map is enabled but the implementation is hardcoded to maps because :source-map and :source-url are used instead of ISourceMap methods -source-map and -source-url. I propose to extend PersistentMap and PersistentArrayMap to ISourceMap to make source-on-disk compliant with both protocols.




[CLJS-1259] Incorrect warnings on type hinted maths Created: 09/May/15  Updated: 14/Jul/15

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

Type: Defect Priority: Minor
Reporter: Erik Ouchterlony Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, math, typehints


 Description   

Variables type hinted as int or double are not recognized as numbers, e.g.

(def ^int i 1)
(+ i i)
WARNING: cljs.core/+, all arguments must be numbers, got [int int] instead. at line 1 <cljs repl>
2


 Comments   
Comment by David Nolen [ 14/Jul/15 6:07 AM ]

The real issue is that there is no support for numeric type hints.





[CLJS-1255] cljs.test file-and-line detection is not useful in browser testing Created: 07/May/15  Updated: 14/Jul/15

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

Type: Defect Priority: Minor
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Chrome



 Description   

cljs.test reports using do-report, which adds file and line information computed from javascript stack traces. In chrome at least, these stack traces are not useful:

"Error
    at http://localhost:3449/js/cljs/test.js:261:69
    at cljs$test$do_report (http://localhost:3449/js/cljs/test.js:268:3)
    at http://localhost:3449/js/test/test_tests.js:491:21
    at test.test_tests.test_has_fails.cljs$lang$test (http://localhost:3449/js/test/test_tests.js:502:4)
    at http://localhost:3449/js/cljs/test.js:384:42
    at http://localhost:3449/js/cljs/test.js:387:4
    at cljs$test$run_block (http://localhost:3449/js/cljs/test.js:320:13)
    ..."

The `file-and-line` stack trace parser doesn't parse this correctly, resulting in a message like this:

FAIL in (test-function) (at http:384:42)

Note the lack of a useful file/namespace reference, and that the line number refers to the compiled javascript rather than the source clojurescript.



 Comments   
Comment by Stephen Nelson [ 07/May/15 9:15 PM ]

Prior to the release of cljs.test my company maintained an internal port of clojure.test that did better reporting than cljs.test's by adding source metadata from &form to the do-report calls generated by assert-expr. This approach was great for internal use but might not be suitable for cljs.test as it could reduce portability of assert-expr between clojure and clojurescript. Another approach could be dynamically bind source metadata in the body generated by try-expr. I'd be willing to implement and contribute code if you can provide some indication of your preferred approach.

Our version of assert-expr also injected a 'reporter function', {{(function(a,b,c){a.apply(b.c)})}}, which we would invoke from report, e.g. (reporter (.-debug js/console) js/console args). This causes the clickable link on the right hand side of chrome's console output to link to the source map location of the test expression, rather than the report function.

Comment by David Nolen [ 14/Jul/15 6:09 AM ]

The correct thing to do here is to move the browser REPL stacktrace parsing into a shared library i.e. .cljc that can be loaded into either environment to handle browser difference.





[CLJS-1390] clojure.walk treats vectors diffently from Clojure version Created: 03/Aug/15  Updated: 03/Aug/15

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

Type: Defect Priority: Minor
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The latest patch to clojure.walk (https://github.com/clojure/clojurescript/commit/f706fabfd5f952c4dfb4dc2caeea92f9e00d8287) ports the line of the Clojure version

(instance? clojure.lang.IMapEntry form) (outer (vec (map inner form)))

with the line

(satisfies? IMapEntry form) (outer (vec (map inner form)))

ClojureScript implements IMapEntry on any vector which I assume is intended.

In Clojure, for vectors this case falls:

(coll? form) (outer (into (empty form) (map inner form)))

This makes a difference because empty preserves metadata.
I. e.

(meta (prewalk (fn [form]
                  (vary-meta form assoc :foo true))
               []))

gives {:foo true} on earlier ClojureScript versions and Clojure, but nil on the latest version.

I have relied on this which has likely not been a very good idea, but others might have too - Hence I created this ticket for consideration.






[CLJS-1421] Enable Asynchronous cljs.js/*eval-fn* Created: 14/Aug/15  Updated: 16/Aug/15

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

Type: Enhancement Priority: Minor
Reporter: Matthew Molloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: asynchronous, bootstrap


 Description   

In bootstrapped ClojureScript cljs.js/eval-fn receives javascript source and evaluates it, returning a result. In some contexts it is necessary to evaluate the js asynchronously, can we add this functionality?



 Comments   
Comment by David Nolen [ 14/Aug/15 7:49 PM ]

This ticket needs more rationale. Can you elaborate on the usecase?

Comment by Matthew Molloy [ 14/Aug/15 10:08 PM ]

My usecase is an asynchronous eval function

(fn *eval-fn*
  [{:keys [source]}]
  (js/chrome.devtools.inspectedWindow.eval source
    (fn [result err]
      (if result
        (callback result)
        (callback err))))

There must be other people who have situations like this.

Comment by David Nolen [ 16/Aug/15 12:16 AM ]

Interesting. I don't think this is a common use case, most JS engines provide synchronous eval. Not interested in any breaking changes but would be happy to take a patch that gives you the behavior you want via an option flag, :async-eval.





[CLJS-1428] Add a cljs.core/*command-line-args* var Created: 16/Aug/15  Updated: 01/Sep/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Add a cljs.core/*command-line-args* var that mimics Clojure's:

https://github.com/clojure/clojure/blob/4bb1dbd596f032621c00a670b1609a94acfcfcab/src/clj/clojure/core.clj#L6148

Rationale:
1) Simplifies writing command-line scripts in ClojureScript if this var is universally available.
2) Consistency with Clojure.

Existing tooling can ignore the var (it would be initialized with nil).

Command-line REPLs or other command-line environments can bind a sequence to this var when launched.



 Comments   
Comment by Justin Thomas [ 31/Aug/15 10:14 PM ]

In this file: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/nodejs.cljs Looks like you could set this: (def *command-line-args* (.-argv process)) and it'd be accessible under nodejs/*command-line-args*. Not sure where the star vars are set in the elsewhere in the code or if that's a good solution putting it in each repo.

Maybe get the target var somehow and set it based on that--similar to how the nodejs file is loaded?

Found this: src/main/clojure/cljs/core.cljc and saw references to a ns called env.

Comment by David Nolen [ 01/Sep/15 6:44 AM ]

This ticket needs more rationale and stronger outline of the design issues before proceeding. As it stands no one should be working on this yet.

Comment by Justin Thomas [ 01/Sep/15 4:27 PM ]

Just trying to learn the code at the moment. As for rationale, using the command line vars in node is a common use case for shell scripts. Accessing the command line vars in a more clojure-y way seems like a good idea.





[CLJS-1407] Exposing output file dependency graph in API Created: 09/Aug/15  Updated: 08/Nov/15

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

Type: Enhancement Priority: Minor
Reporter: Juho Teperi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Use case for boot-cljs and boot-reload:

After compilation boot-reload reloads the changed JS files. So that the files can be reloaded in correct order, boot-cljs uses dependency graph to sort the files. Currently boot-cljs accesses compiler state directly and uses data from :js-dependency-index to build the graph: https://github.com/adzerk-oss/boot-cljs/blob/0.0-3308/src/adzerk/boot_cljs/impl.clj#L17-L36

Simple solution:

If dependencies (requires) of namespace are exposed through API it is easy to build graph of cljs namespace dependencies: https://github.com/adzerk-oss/boot-cljs/blob/d479f10935be321232e2363e2ae3e9cc515a81af/src/adzerk/boot_cljs/impl.clj#L12-L32

Problem with this solution is that all-ns, ns-dependencies or target-file-for-cljs-ns do not work with foreign-deps. While foreign-dep files don't usually change and thus aren't reloaded, it's possible that user has local JS files in the project using foreign-deps and those can change.

Questions, notes and issues

  • Should cljs-dependency-graph be exposed in the API or is it enough to provide ns-dependencies and such which user can use to create dependency graph?
  • cljs.build.api/parse-js-ns can also be used to read provides and requires from compiled JS files, but this doesn't work with foreign-deps either
  • Perhaps there is some way in Closure library to reload files in correct order?
  • Supporting foreign-deps is not perhaps necessary, but if there is good way it would be nice to have.


 Comments   
Comment by Juho Teperi [ 11/Aug/15 3:18 AM ]

I would add the call to cljs.compiler.api and it could be called output-dependency-graph.

Creating the graph requires list of all the nodes and dependencies for each node. For Cljs namespaces
these are accessible through all-ns and ns analysis map :requires. Data about foreign-deps
and closure libs is available in the compiler state under :js-dependency-index key. To create the
graph we need to:

1. Get list of all nodes
2. Get dependencies for given node
3. Get output file for given node

Because steps 2 and 3 depend on the type of node, it would probably be easiest to collect those
values in step 1. So step 1 would do something like this:

{{(get-nodes ...) => {:provides "goog.net" :file "out/goog/net.js" :dependencies #{"goog.foo"}} {:provides "frontend.core" :file "out/frontend/core.js" :dependencies #{"cljs.core"}}}}

That could be implemented by concatenating data from cljs namespaces retrieved from all-ns etc. with
data from :js-dependency-index. The next and last step would be to construct the graph using reduce.

Using this implementation there would be just one new API call: output-dependency-graph.

I was thinking alternative approach with all-ns, find-ns etc. versions which would work also with foreign-deps and closure libs, but I don't think it's very easy (or efficient) e.g. to retrieve data for foreign-dep with just a name as they are indexed by file paths.

Comment by David Nolen [ 03/Nov/15 6:34 PM ]

Now that CLJS-1437 is merged what is needed to wrap this one up?

Comment by Juho Teperi [ 08/Nov/15 9:11 AM ]

My current plan with boot-cljs/boot-reload is to use Figwheel client code which uses Google Closure dependency graph for loading the files in correct order. Thus I don't need this anymore. Perhaps it's best to close this if no-one needs this currently?

Comment by David Nolen [ 08/Nov/15 9:28 AM ]

It may still be useful at some point. Will just lower the priority.





[CLJS-1502] Browser REPL broken when started with :optimizations :none Created: 05/Dec/15  Updated: 06/Dec/15

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

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

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

 Description   

Creating a browser-repl env like this: (cljs.repl.browser/repl-env :optimizations :none) does not work because client.js is not compiled in a single file. The browser throws the following error: goog is not defined.
:optimizations :simple (the default) works fine.
A quick fix would be to force :optimizations :simple in the REPL options. However, being able to set :optimizations :none would probably speed up compilation times.






[CLJS-1513] Javascript emitted for cljs.pprint is misinterpreted under Mobile Safari 7.0 Created: 14/Dec/15  Updated: 14/Dec/15

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

Type: Defect Priority: Minor
Reporter: Ruslan Prokopchuk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Tested with 1.7.28 and 1.7.170 versions of ClojureScript.

browser = Mobile Safari 7.0
os = iOS 7.1.2



 Description   

Create minimal ClojureScript project, add {{(:require [cljs.pprint :refer [pprint]])}} to the source file and compile with :optimizations :advanced. Output js will contain strings like

~\x3c\x3c-(~;~@{~w~^ ~_~}~;)-\x3c~:\x3e
which lead to the following error in the Mobile Safari 7.0:

Error: Directive "{" is undefined
~<<-(~;~@{~w~^ ~_~}~;)-<~:>
         ^





[CLJS-1104] Compute SHA for ClojureScript compiled file Created: 10/Mar/15  Updated: 22/Dec/15

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


 Description   

Needed for shared AOT cache






[CLJS-713] optimized case Created: 04/Dec/13  Updated: 22/Dec/15

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

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

Attachments: Text File 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch     Text File 0001-CLJS-713-first-cut-at-compiling-case-to-switch.patch    

 Description   

With the advent of asm.js many engines will like compile switch statements over integers into jump tables. We should provide a real `case*` ast node that compiles to JS `switch` when possible - i.e. numbers, strings, keywords etc.



 Comments   
Comment by Michał Marczyk [ 18/Feb/14 5:56 PM ]

First cut impl also available here:

https://github.com/michalmarczyk/clojurescript/tree/713-compile-case-to-switch

With this patch applied, case expressions are compiled to switch + some extra bits when all tests are numbers or strings, otherwise old logic is used.

For example, {{(fn [] (let [x 1] (case x 1 :foo (2 3) :bar :quux)))}} gets compiled to

function () {
    var x = 1;
    var G__6469 = x;
    var caseval__6470;
    switch (G__6469) {
      case 1:
        caseval__6470 = new cljs.core.Keyword(null, "foo", "foo", 1014005816);
        break;
      case 2:
      case 3:
        caseval__6470 = new cljs.core.Keyword(null, "bar", "bar", 1014001541);
        break;
      default:
        caseval__6470 = new cljs.core.Keyword(null, "quux", "quux", 1017386809);
    }
    return caseval__6470;
}

The existing test suite passes, but I suppose it wouldn't hurt to add some tests with case in all possible contexts.

Comment by Michał Marczyk [ 18/Feb/14 6:05 PM ]

As a next step, I was planning to arrange things so that numbers/strings are fished out from among the tests and compiled to switch always, with any leftover tests put in an old-style nested-ifs-based case under default:. Does this sound good?

It seems to me that to deal with symbols and keywords in a similar manner we'd have to do one of two things:

1. check symbol? and keyword? at the top, then compile separate switches (the one for keywords would extract the name from the given keyword and use strings in the switch);

2. use hashes for dispatch.

Which one sounds better? Or is there a third way?

Comment by Michał Marczyk [ 18/Feb/14 6:11 PM ]

Of course we'd need to compute hashes statically to go with 2. I'd kind of like it if it were impossible (randomized seed / universal hashing), but currently it isn't.

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

At least on v8, there are surprisingly few cases where a switch statement will be optimized to a jump table. Basically the type of the switched-over value must always (across calls) match the type of every case, and there must be fewer than 128 cases, and integer cases must be 31-bit ints (v8's smi type). So mixing string and number cases in the same switch guarantees the statement will never be compiled. In many cases an equivalent if-else will end up being significantly faster on v8 just because the optimizing jit recognizes them better. There's an oldish bug filed against v8 switch performance. Looking at the many jsperfs of switch statements, it doesn't seem that v8 has improved. Relevant jsperf

Firefox is much better at optimizing switch statements (maybe because of their asm.js/emscripten work) but I don't know what conditions trigger (de)optimization.

I suspect the best approach is probably going to be your option one: if-else dispatch on type if any case is not a number, and then a switch statement covering the values for each of the keyword/string/symbol types present (no nested switch statements, and outlining the nested switches might be necessary). Even with a good hash, to guarantee v8 optimizing-compilation you would need to truncate the hashes into an smi (signed-left-shift once?) inside the case*.

Comment by David Nolen [ 19/Feb/14 12:50 AM ]

There's no need for invention here. We should follow the strategy that Clojure adopts - compile time hash calculation.

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

The problem, as Michal alluded to, is that the hash functions in cljs's runtime environment are not available at compile-time (unlike in Clojure). This might be a good opportunity to clean up that situation or even use identical hash values across Clojure and Clojurescript (i.e. CLJS-754), but that's a much bigger project. Especially considering it will probably not bring much of a speedup over an if-else-if implementation except in very narrow circumstances.

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

Francis Avila I would make no such assumptions about performance without benchmarks. One of the critical uses for case is over keywords. Keyword hashes are computed at compile time, so that's one function call and a jump on some JavaScript engines. This is particularly useful for the performance of records where you want to lookup a field via keyword before checking the extension map.

This ticket should probably wait for CLJS-754 before proceeding.

Comment by Francis Avila [ 22/Feb/14 4:44 AM ]

Record field lookup is a good narrow use case to test. I put together a jsperf to compare if-else (current) vs switch with string cases vs switch with int cases (i.e., hash-compares, assuming perfect hashing).

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

I've merged the case* analyzer and emitter bits by hand into master.

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

I'v merged the rest of the patch into master. If any further optimizations are done it will be around dispatching on hash code a la Clojure.

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

Your keyword-test optimization has a bug: https://github.com/clojure/clojurescript/commit/9872788b3caa86f639633ff14dc0db49f16d3e2a

Test case:

(let [x "a"] (case x :a 1 "a"))
;=> 1
;;; Should be "a"

Github comment suggests two possible fixes.

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

Thanks fix in master.

Comment by Christoffer Sawicki [ 23/Jun/14 3:41 PM ]

case over "chars" is currently not being optimized to switch (in other words: (case c (\a) :a :other) uses if instead of switch).

Given that ClojureScript chars are just strings of length 1, could this perhaps simply be fixed by tweaking https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/core.clj#L1187 ?

Comment by Christoffer Sawicki [ 23/Jun/14 4:11 PM ]

OK, I couldn't resist trying and it seems to be that easy. Would be great if somebody more knowledgeable could look at it and say if it has any side-effects. (See patch with name 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch.)

Comment by David Nolen [ 23/Jun/14 4:15 PM ]

The patch looks good I would have applied it if I hadn't already gone and done it master myself just now

Comment by Christoffer Sawicki [ 23/Jun/14 4:22 PM ]

Hehe. Thanks! Don't forget to update the "case* tests must be numbers or strings" message on line 496 too.

Comment by David Nolen [ 23/Jun/14 4:48 PM ]

The existing docstring is inaccurate - case supports all compile time literals.

Comment by David Nolen [ 22/Dec/15 4:59 PM ]

There are quite a few optimization in master now.





[CLJS-1075] Generic inline source map support Created: 02/Mar/15  Updated: 22/Dec/15

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

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


 Description   

Currently hard coded to REPLs. Would simplify jsbin and similar integration.






[CLJS-575] cljsc.bat emit FileNotFoundException when compile samples in windows Created: 25/Aug/13  Updated: 05/Jan/16

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

Type: Defect Priority: Minor
Reporter: Park Sang Kyu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, bug, patch
Environment:

in windows 7


Attachments: File cljsc.bat.diff     File cljsc-path.bat    
Patch: Code

 Description   

cljsc.bat emit FileNotFoundException when it compile samples of the ClojureScript project in windows like below.

------------------------------------------------
Exception in thread "main" java.io.FileNotFoundException: Could not locate cljs/closure__init.class
or cljs/closure.clj on classpath:
------------------------------------------------

It is caused by lack of a backslash in the end of path of the system environment variable, %CLOJURESCRIPT_HOME% set by a user.
In the case CLASSPATH is set to "C:\\clojure\clojurescriptsrc\clj;C:\\clojure\clojurescriptsrc\cljs" and this make it impossible for javac to find cljs/clojure.clj file.

So it can be solved by adding a backslash to the path of %CLOJURESCRIPT_HOME%.

I attached the patched file, "cljsc-path.bat"



 Comments   
Comment by David Nolen [ 04/Sep/13 11:04 PM ]

Can we please get a proper git diff thanks (and please send in your CA)! Also would be nice to get Windows users to check this out.

Comment by Park Sang Kyu [ 15/Sep/13 3:16 AM ]

git diff

Comment by David Nolen [ 05/Oct/13 11:55 AM ]

Thank you! Have you sent in your CA? http://clojure.org/contributing

Comment by Park Sang Kyu [ 19/Jun/14 10:24 AM ]

Yes i have sent my CA.

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

Excellent, the patch is not correctly formatted. Can we get a new patch that conforms to http://github.com/clojure/clojurescript/wiki/Patches





[CLJS-404] Automate Browser REPL testing Created: 23/Oct/12  Updated: 05/Jan/16

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

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


 Description   

It's worth investigating Selenium, PhantomJS, etc. as solutions to sanity check the Browser REPL when we run the other tests.



 Comments   
Comment by Robert Krahn [ 22/Dec/14 1:22 PM ]

An attempt: https://github.com/clojure/clojurescript/pull/42

Comment by David Nolen [ 24/Dec/14 8:57 AM ]

This looks like an interesting patch, thanks!

Comment by Robert Krahn [ 26/Dec/14 10:57 AM ]

I'll post a patch here, first I'll investigate the load-file issue, though.





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 05/Jan/16

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

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





[CLJS-1047] externs checking for js/foo Created: 19/Feb/15  Updated: 30/Apr/15

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Maria Geller
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Worth looking into validating `js/foo` forms again the known externs set. Can probably be done by leveraging the Closure JS Parser.



 Comments   
Comment by Michael Griffiths [ 22/Feb/15 12:03 PM ]

Would you consider making the results of parsing available to tooling (e.g. in cljs.env/*compiler*)? I would use this to add support for autocompletion of js/ forms to CIDER.

Comment by David Nolen [ 23/Feb/15 8:15 AM ]

Definitely open to the idea of exposing this information to other tooling when we get there.





[CLJS-1548] cannot reference a JS global var with same name as current namespace Created: 19/Jan/16  Updated: 19/Jan/16

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

Type: Defect Priority: Minor
Reporter: Shaun LeBron Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Steps to reproduce:

1. Create a JS global var `foo`.
2. Create a `foo.core` namespace.
3. Try referencing `js/foo` in `foo.core`.

Problem:

`js/foo` does not resolve to the JS global var `foo`
(in advanced optimizations, but not w/ optimizations off).

Solution: Proposing we issue a warning.

Will put together a diagnostic project soon and see about working on a patch.






[CLJS-1059] Simple interface wanted to convert cljs forms to js Created: 22/Feb/15  Updated: 31/Jan/16

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

Type: Enhancement Priority: Minor
Reporter: Stuart Mitchell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: analyzer, compiler


 Description   

In our project (a clojurescript debugger) we want to convert cljs forms or a sequence of forms into javascript so that they can be executed in the javascript console.

We would like something similar to closure/compile-form-seq (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/closure.clj#L308)

However, we need to supply, the namespace requires and locals in an env like this

{:ns {:name "test.core" :requires {(quote gstring) (quote goog.string)}} :locals {}}

This code seems to do what we want.

(defn compile-form-seq
    \"Compile a sequence of forms to a JavaScript source string.\"
    [forms env]
    (env/ensure
    (compiler/with-core-cljs nil
      (fn []
        (with-out-str
            (doseq [form forms]
              (compiler/emit (analyzer/analyze env form))))))))

I am not sure why I need env/ensure.

Would you be able to patch compile-form-seq to provide the needed interface, or suggest what we should be doing.

Thanks
Stu



 Comments   
Comment by Mike Thompson [ 22/Feb/15 10:09 PM ]

Just to be clear:
1. when our debugger is at a breakpoint,
2. the user can type in an expression at the repl
3. in response, our debugger has to compile the user-typed-in expression to javascript (and then execute it, showing a result)
4. taking into account any local bindings. <---- this is the key bit.

To satisfy point 4, our tool extracts all the 'locals' from the current call-frame, and then supplies all these local bindings in env/locals, so the compiler doesn't stick a namespace on the front of them.

For example, if there was a local binding for 'x' in the callstack, and the user's repl-entered-expression involves 'x', then we want the compiler to leave the symbol 'x' alone and to not put some namespace on the front of it. In the final javascript, it must still be 'x', not 'some.namespace.x'

Our method to achieve this is to put 'x' into env/locals when compiling – and it all works. Except, with the recent changes this has become more of a challenge. Hence this ticket asking for a way to pass in env.

Comment by Thomas Heller [ 23/Feb/15 3:19 AM ]

You could wrap the user expression in an fn, that would allow you to skip messing with the locals. The REPL basically does the same trick for *1,*2,...

(fn [x]
  ~user-expression-here)
Comment by David Nolen [ 29/Apr/15 7:21 AM ]

Seems like something useful to add to a cljs.compiler.api namespace.





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

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

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

Attachments: Text File cljs_651.patch    
Patch: Code

 Description   

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



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

All paths taken on satisfies are now hinted as boolean

Comment by Mike Fikes [ 31/Jan/16 3:26 PM ]

cljs_651.patch no longer applies on master





[CLJS-1458] re-matches might give false negative when using match groups Created: 25/Sep/15  Updated: 31/Jan/16

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

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

Attachments: Text File cljs-1458-re-matches-might-give-false-negative.patch    
Patch: Code and Test

 Description   

Current behaviour:

(re-matches #"(a|aa)" "aa") => nil

Expected:

(re-matches #"(a|aa)" "aa") => ["aa" "aa"]

JVM version works as expected, only CLJS is affected



 Comments   
Comment by David Nolen [ 14/Oct/15 11:36 AM ]

This is the kind of ticket that tends to break existing code. We should get some people who are interested in this ticket to actually try it out.

Comment by Mike Fikes [ 31/Jan/16 3:45 PM ]

FWIW, I gave cljs-1458-re-matches-might-give-false-negative.patch a try in bootstrapped ClojureScript and it is working fine there (each of the additional unit tests produce the expected results in bootstrapped).





[CLJS-1559] Closure :libs ignored Created: 05/Feb/16  Updated: 05/Feb/16

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

Type: Defect Priority: Minor
Reporter: Dominykas Mostauskis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

:libs compilation option doesn't work. Whether specifying directories, or specific files. If trying to `import` one of the js classes (properly namespaced with `goog.provide`) into clojurescript, compilation fails with "no such namespace". If the libs code is not referenced in clojurescript, it compiles, and the output directory does not contain the libs js files.

Compilation options:

(cljs.closure/build
    "src/main/clojurescript"
    {:main 'example.core
     :libs ["/src/main/javascript/"]
     :optimizations :none
     :output-dir "js"
     :output-to "js/main.js"
     :source-map true
     :asset-path "/js"
     })

Javascript file:

goog.provide("test.Test");

test.Test = function(x) {
  this.x = x;
};


 Comments   
Comment by Mike Fikes [ 05/Feb/16 1:51 PM ]

Hi Dominykas, is the absolute path intentional? I suspect the intent was to not have the leading /.

Comment by Dominykas Mostauskis [ 05/Feb/16 2:01 PM ]

I made this typo when posting. On my setup paths are relative to project root.

Comment by David Nolen [ 05/Feb/16 2:38 PM ]

As far as I know quite a few people rely on this functionality. Please provide a complete minimal example or this issue will be closed. All source should be in the ticket or in this comment thread, no external links. Thanks.

Comment by Dominykas Mostauskis [ 05/Feb/16 3:50 PM ]

Can't reproduce. Tips would be appreciated. Banging my head against the wall here.





[CLJS-1153] Typed Array backed PersistentVector based on clojure.core/Vec Created: 19/Mar/15  Updated: 05/Feb/16

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

Type: Enhancement Priority: Minor
Reporter: Adrian Medina Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: enhancement

Attachments: Text File 1153.patch    
Patch: Code

 Description   

Goal is to add support for vectors based on clojure.core/Vec, built on top of JavaScript Typed Arrays.

My hope is that this would allow for both efficient creation of vectors from existing Typed Arrays without intermediate conversion to normal JavaScript arrays, as well as efficient concatenation of the composite arrays of the vector back into a Typed Array when necessary via an enhanced cljs.core/into-array.

Implementation is based heavily on clojure/core/gvec.clj, cljs.core/PersistentVector, and cljs.core/TransientVector.

Performance should be comparable to cljs.core/PersistentVector, although there is additional constant overhead with TypedArray instantiation compared to js/Array.

Adds cljs.core/Vec, cljs.core/TransientVec, cljs.core/vector-of, and updates cljs.core/into-array.



 Comments   
Comment by Adrian Medina [ 19/Mar/15 8:39 PM ]

I still have to test, I will update the issue when that is complete. I just wanted to get my first patch up for review as quickly as possible.

Comment by Francis Avila [ 19/Mar/15 11:59 PM ]

No mention of Uint8ClampedArray.

Should Vec type- or range-check assignments? In Clojure these fail (even with unchecked-math):

  • (vector-of :byte 128) returns [-128]
  • (vector-of :byte "1") returns [1]
  • (vector-of :byte (js-obj)) returns [0]

If we're going to expose host primitive arrays via cljs apis, should we also bring the various other array functions in line with Clojure (and ClojureCLR, which also has extra uint, ubyte, etc types) like you are doing with into-array? Some or all of these issues may warrant another ticket instead, or maybe even a design page:

  • make-array ignores type argument and lacks higher dimensions.
  • object-array, int-array, etc. maybe should return TypedArrays.
  • Missing ubyte-array, ushort-array, uint-array (like ClojureCLR)
  • Missing aset-* setters. (Meaningless in js unless we range-check.)
  • aclone and amap preserve type of input array in Clojure, but not in cljs.
  • missing array casters: bytes, shorts, chars, ints, etc.
  • While we're at it, primitive coercion functions (e.g. int, long, unchecked-int, etc) are either no-ops or differ from clojure. (e.g., int in cljs is like unchecked-int in clojure, but unchecked-int in cljs does nothing). Maybe these should be dropped or should match the javascript ToInt32, ToInt16, etc abstract operations (i.e. those used when assigning to TypedArrays). Maybe these match java semantics also?
Comment by Mike Fikes [ 05/Feb/16 8:18 PM ]

Patch 1153.patch no longer applies





[CLJS-1494] turn cljs.core/*assert* into a goog-define Created: 25/Nov/15  Updated: 22/Feb/16

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

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

Attachments: Text File goog-define-assert.patch    
Patch: Code

 Description   

This patch turns the cljs.core/*assert* boolean into a goog.define and also checks *assert* at runtime (instead of only at compile-time).

The closure define option allows the closure compiler to eliminate asserts in :advanced, while :none builds can keep the asserts. This is one of the few remaining issues that prevent :advanced builds to re-use :none compiled (cached) files.

:elide-asserts is unaffected to keep this as simple as possible, but could be built on top of the goog.define instead of actually affecting the compiled output.



 Comments   
Comment by Mike Fikes [ 20/Feb/16 8:02 AM ]

Patch no longer applies, probably owing to CLJS-970.

Comment by Thomas Heller [ 22/Feb/16 5:08 AM ]

There was one more issue I discovered with my approach. My goal was to enable the Closure Compiler to eliminate the asserts when using :advanced compilation. This works perfectly fine with using a goog.define for *assert* but the compiler will complain if you try to adjust the define later since goog.define vars are not allowed to be adjusted at runtime.

(binding [*assert* false]
  (something-that-asserts))

This works in CLJ but not in CLJS since *assert* is only checked at compile time. If compiled with :elide-asserts true you can't bind assert to true either since the code no longer exists.

So some compromise must be made either way, the best solution IMHO would be to have a goog.define which lets the compiler decide whether to eliminate the asserts or not, independent from the *assert* and then moving the assert check itself into js instead of the compiler.

Happy to write the patch if interested.





[CLJS-1561] WARN if recur passes non-inferred type Created: 06/Feb/16  Updated: 23/Feb/16

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

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

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

 Description   

Take this code as an example:

(defn f [^boolean b]
  (loop [x b]
    (if x
      (recur 0)
      :done)))

The type of x is inferred to be Boolean, but there is a recur form that can be statically deduced to be passing a non-Boolean.

This ticket asks that a WARN be issued for this case, and perhaps others (where maybe x itself is directly type hinted).



 Comments   
Comment by Mike Fikes [ 06/Feb/16 2:59 PM ]

Attached a patch which warns on for the case of boolean and number, since those two types have special handling.

Some example usage:

cljs.user=> (defn f [^boolean b]
       #_=>   (loop [x b]
       #_=>     (if x
       #_=>       (recur 0)
       #_=>       :done)))
WARNING: recur target parameter x has inferred type boolean, but being passed type number at line 4 
#'cljs.user/f
cljs.user=> (loop [x 1 y true z :hi]
       #_=>   (when false (recur 'a "hi" nil)))
WARNING: recur target parameter x has inferred type number, but being passed type cljs.core/Symbol at line 2 
WARNING: recur target parameter y has inferred type boolean, but being passed type string at line 2 
nil
cljs.user=> (loop [x 1 y true]
       #_=>  (when false (recur nil nil)))
WARNING: recur target parameter x has inferred type number, but being passed type clj-nil at line 2 
WARNING: recur target parameter y has inferred type boolean, but being passed type clj-nil at line 2 
nil
cljs.user=> (loop [x 1]
       #_=>   (let [y (inc x)]
       #_=>     (when false (recur (inc y)))))
nil
cljs.user=> (loop [b true]
       #_=>   (when false (recur (inc 1))))
WARNING: recur target parameter b has inferred type boolean, but being passed type number at line 2 
cljs.user=> (loop [x 1] 
       #_=>   (inc x) 
       #_=>     (when false (recur :hi)))
WARNING: recur target parameter x has inferred type number, but being passed type cljs.core/Keyword at line 3 
nil
cljs.user=> (loop [x :hello] 
       #_=>   (inc x) 
       #_=>     (when false (recur :hi)))
WARNING: cljs.core$macros/+, all arguments must be numbers, got [cljs.core/Keyword number] instead. at line 2 
nil




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

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

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


 Description   

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

Example in Clojure:

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

Compare Clojurescript:

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

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

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

Questions:

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


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

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

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

Essential points:

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

Thoughts?

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

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

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

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

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

Interop possibilities:

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

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





[CLJS-1599] UUIDs are not equal for upper/lower case strings Created: 07/Mar/16  Updated: 11/Mar/16

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

Type: Defect Priority: Minor
Reporter: Nikolay Durygin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 7x64



 Description   

UUIDs generated for strings in different case (one is upper and one is lower) are equal.

For example (= (uuid "071C600F-B72B-44AE-8A15-9366EA1BB9D9") (uuid "071c600f-b72b-44ae-8a15-9366ea1bb9d9")) returns false.

Spec http://www.itu.int/rec/T-REC-X.667/en says following:

6.5.4 Software generating the hexadecimal representation of a UUID shall not use upper case letters.
NOTE - It is recommended that the hexadecimal representation used in all human-readable formats be restricted to lower-case
letters. Software processing this representation is, however, required to accept both upper and lower case letters as specified
in 6.5.2.



 Comments   
Comment by Gary Fredericks [ 07/Mar/16 8:17 AM ]

Would this be a good time to change the internal representation from a string to either a pair of goog.math.Longs or a quartet of "32-bit" integer doubles?

Comment by Nikolay Durygin [ 09/Mar/16 2:22 AM ]

Is there any special need? Issue described above can be solved by lower casing all strings inside uuid. Another problem - the fact that uuid doesn't complain if non uuid format string is passed can be solved with regex.





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

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

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


 Description   

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

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

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

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

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

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



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

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

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

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

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

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

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

Node.js

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

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

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

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

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

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

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

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

Speaking from nodejs perspective:

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





[CLJS-1575] Combination of - and _ params causes JSC_DUPLICATE_PARAM Created: 17/Feb/16  Updated: 18/Mar/16

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

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


 Description   

The following function definitions each cause ERROR: JSC_DUPLICATE_PARAM. Parse error. Duplicate parameter name "_" at compile time with :advanced compilation:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defn causes-duplicate-param [{_ :foo}]
  (reify
    Object
    (a-function [-]
      (fn [] "arbitrary function inside a-function body"))))

(defn causes-duplicate-param [{- :foo}]
  (reify
    Object
    (a-function [_]
      (fn [] "arbitrary function inside a-function body"))))

They generate, respectively:

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1601.prototype.a_function = ((function (map__1599,map__1599__$1,_){
return (function (){
var self__ = this;
var _ = this;
return ((function (_,map__1599,map__1599__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(_,map__1599,map__1599__$1,_))
});})(map__1599,map__1599__$1,_))
;

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1612.prototype.a_function = ((function (map__1610,map__1610__$1,_){
return (function (){
var self__ = this;
var _ = this;
return ((function (_,map__1610,map__1610__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(_,map__1610,map__1610__$1,_))
});})(map__1610,map__1610__$1,_))
;

(Notice the duplicate _ param on the 5th line of each.)

The following do not:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defn causes-duplicate-param [{- :foo}]
  (reify
    Object
    (a-function [-]
      (fn [] "arbitrary function inside a-function body"))))

(defn causes-duplicate-param [{_ :foo}]
  (reify
    Object
    (a-function [_]
      (fn [] "arbitrary function inside a-function body"))))

They generate:

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1601.prototype.a_function = ((function (map__1599,map__1599__$1,_){
return (function (){
var self__ = this;
var ___$1 = this;
return ((function (___$1,map__1599,map__1599__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(___$1,map__1599,map__1599__$1,_))
});})(map__1599,map__1599__$1,_))
;

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1612.prototype.a_function = ((function (map__1610,map__1610__$1,_){
return (function (){
var self__ = this;
var ___$1 = this;
return ((function (___$1,map__1610,map__1610__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(___$1,map__1610,map__1610__$1,_))
});})(map__1610,map__1610__$1,_))
;

(Notice that one of the {} params has become __$1.)

My guess, though I haven't looked into the compiler code, is that the compiler escapes {} to _$1 when it would conflict with another {}, and also it translates - to {}, but it doesn't notice the conflict when the _ClojureScript symbols are different.



 Comments   
Comment by Peter Jaros [ 17/Feb/16 11:44 AM ]

Forgive the weird formatting errors. I couldn't find a preview function and there doesn't appear to be a way to edit the issue now that it's posted.

Comment by David Nolen [ 18/Mar/16 1:46 PM ]

This is because of munging they will become the same thing. Patch welcome.





[CLJS-1610] Refs api Created: 26/Mar/16  Updated: 28/Mar/16

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

Type: Enhancement Priority: Minor
Reporter: James Laver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It would be nice to support the refs API in clojurescript to ease porting of libraries which require it. Obviously usage of it would not bring concurrency benefits, but it would at least allow some existing clojure code to run.

I've satisfied myself it can be done reasonably easily (see https://github.com/jjl/clojurescript/tree/ref-support ), and I can't really see any downsides.

For the minute, this has been put into the cljs.stm namespace. I don't know whether it should be a feature you are automatically opted into, so I didn't put it into core. Thoughts?



 Comments   
Comment by David Nolen [ 28/Mar/16 6:44 AM ]

It's definitely worth considering. If you've done the work, attach an actual patch for review. In general we try to avoid links outside.





[CLJS-1574] CLJS string equivalence is very slow in Chrome Created: 16/Feb/16  Updated: 22/Feb/16

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

Type: Defect Priority: Minor
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

Google Chrome 48.0.2564.109 on Mac OS X 10.11.3
Safari 9.0.3 (11601.4.4) on Mac OS X 10.11.3



 Description   

Clojurescript's equivalence for strings in Google Chrome is ~1000 times slower than equivalent javascript functionality, and ~1000 times slower than the same function in Safari.

Google Chrome
js equiv:  0.005 seconds
cljs equiv:  1.898 seconds
Safari
js equiv:  0.005 seconds
cljs equiv:  0.006 seconds
(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(deftest test-js-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (js* "~{} == ~{}" str1 str2)
                  "js equivalence")
        end   (now)
        ]
    (println "js equiv: " (delta end start))))

(deftest test-cljs-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (= str1 str2)
                  "cljs equivalence")
        end   (now)
        ]
    (println "cljs equiv: " (delta end start))))


 Comments   
Comment by Stephen Nelson [ 16/Feb/16 6:06 PM ]

This bug only occurs when cljs.pprint has been required.

Comment by Stephen Nelson [ 16/Feb/16 6:38 PM ]

After a whole lot of binary search, here's a minimal reproduction. When cljs.pprint is loaded it constructs write-option-table. It seems that constructing a hash map with the keys :added and :ns causes a call to (= :added :ns), which is sufficient to cause string equality to become extremely slow.

(ns hello-world.core)

(enable-console-print!)

(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(defn test [] 
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (= str1 str2)
        end   (now)
        ]
      (println "cljs equiv: " (delta end start))))

(test)

(= :added :ns)

(test)
Comment by Peter Schuck [ 17/Feb/16 4:50 PM ]

Is the ClojureScript compiled with options :optimizations :advanced or :static-fns true? Compiling ClojureScript without those options results in call indirection for all function calls which might explain the slowdown. See http://swannodette.github.io/2015/03/16/optimizing-clojurescript-function-invocation/ for more information.

Comment by Stephen Nelson [ 17/Feb/16 9:06 PM ]

This happens with :advanced, :simple, and without optimisations. Stepping through the generated javascript seems to indicated that the slow down comes from the VM's internal function dispatch. Regardless, I don't think that the extra function calls related to dynamic dispatch in clojurescript could add minutes of overhead per call. Note that the test case above only uses 128k of string data, the case where I encountered this issue first used ~512K and took about 5 minutes to complete a single function call.

Comment by Francis Avila [ 17/Feb/16 9:14 PM ]

I have reproduced this in Chrome for Linux, on :none and :advanced optimization levels using different test code. I verified the result of the compare so the JIT won't optimize it away and I used performance.mark() and performance.measure() for timing, although none of this should have mattered.

Every subsequent string compare after the first -equiv-invoking use of equal is significantly slower for no reason I can see. There are no intermediate GCs or anything to suggest that it should be slower--it just takes longer! The only thing I can think of is maybe the keyword-equals triggers a deopt because it makes the equal-function megamorphic, but the code is run so few times that there should not be jit optimizations kicking it at all. Also, the keyword-compare itself remains fast.

I suspect a Chrome/v8 bug. Possibly a different internal string representation kicks in for some reason which has a slower compare? This is only an issue for largish, non-constant strings, and the slowdown is proportional to string size. I'm going to try and reproduce this with pure JS.

Comment by Francis Avila [ 18/Feb/16 12:33 AM ]

All you need to reproduce this is to use the strict equality operator in a function body non-monomorphically. Subsequent executions of the function with strings (at least) which have not been compared before the polymorphic call will be very slow.

If you replace strict equality (triple-equal) with normal equality (double-equal), this issue goes away.

This is clearly a Chrome/v8 bug, but I'm not sure where to report it.

Minimal pure-javascript reproduction:

function str(size) {
  var s = "";
  for (var i = 0; i < size; i++) s += "a";
  return s;
}

function eq(x, y) {
  performance.mark("start");
  x === y; // No slowdown if use == instead
  performance.mark("end");
}

function print_measures() {
  performance.getEntriesByType("measure")
  .forEach(entry => console.log(entry.name, entry.duration));
}

var s1 = str(64 * 1024);
var s2 = str(64 * 1024);
var s3 = str(64 * 1024);

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(0, 0);
performance.measure("eq(0, 0)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

print_measures();

Results with Chrome 48.0.2564.109 (64-bit) on a slow iMac with OS X 10.11.3

eq(s1, s2)   4.465000000000003     // fast string compare
eq(0, 0)     0.009999999999990905  // break monomorphism of eq()
eq(s1, s3) 259.665                 // Now string compare is slow
eq(s1, s2)   0.019999999999924967  // Repeated call still fast
eq(s1, s3) 232.52499999999998      // ... but not from after the polymorphic invoke
Comment by Francis Avila [ 22/Feb/16 3:14 AM ]

Issue added to v8: https://bugs.chromium.org/p/v8/issues/detail?id=4773





[CLJS-1598] Honor printing of function values via IPrintWithWriter Created: 03/Mar/16  Updated: 08/Apr/16

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

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

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

 Description   

If a user wishes to define how function values are printed, allow that to be controlled via IPrintWithWriter with code like

(extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer opts]
    ,,,))


 Comments   
Comment by Mike Fikes [ 03/Mar/16 10:28 AM ]

Can be tested manually:

$ script/nashornrepljs 
To quit, type: :cljs/quit
cljs.user=> inc
#object[cljs$core$inc "function cljs$core$inc(x){
return (x + (1));
}"]
cljs.user=> (extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer _]
    (let [name (.-name obj)
          name (if (empty? name)
                 "Function"
                 name)]
      (write-all writer "#object[" name "]"))))
#object[Function]
cljs.user=> inc
#object[cljs$core$inc]
Comment by David Nolen [ 11/Mar/16 1:04 PM ]

The problem is this makes printing slower. For people using EDN as interchange format this may be a problem. Would need to see some numbers.

Comment by Antonin Hildebrand [ 08/Apr/16 2:11 PM ]

I'm not sure what is the difference between implements? and satisfies?. But by reading the code I would assume that it should be printed by this line:
https://github.com/clojure/clojurescript/blob/9a2be8bc665385be1ef866e2fd76b476c417d2bf/src/main/cljs/cljs/core.cljs#L9056-L9057

Don't we want to change implements? to satisfies? there? Not sure about (perf) implications.





[CLJS-1620] In JavaScript ES2015 modules default export name is munged to default$ Created: 08/Apr/16  Updated: 08/Apr/16

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

Type: Defect Priority: Minor
Reporter: Roman Liutikov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When using a foreign lib which is ES2015 module with default export, the value which is being exported is assigned to a property default on a namespace object. In ClojureScript code this means one should call to default var of that namespace. However in complied output of ClojureScript code the name default is getting munged to default$.

export default function inc(v) {
  return v + 1;
}
(ns cljs-example.core
  (:require [lib.inc :as lib]))

(lib/default 0)
goog.provide("module$lib$inc");
function inc$$module$lib$inc(v){return v+1}
module$lib$inc.default=inc$$module$lib$inc
// Compiled by ClojureScript 1.8.40 {}
goog.provide('cljs_example.core');
goog.require('cljs.core');
goog.require('module$lib$inc');
module$lib$inc.default$.call(null,(0));

//# sourceMappingURL=core.js.map


 Comments   
Comment by David Nolen [ 08/Apr/16 2:42 PM ]

One possible path around this is to respect the Closure Compiler language setting when munging instead of blindly munging ECMA-262 keywords. A patch that adopts this approach would be welcome.





[CLJS-1618] `extend-type string` doesnt work without wrapping the string object into `(str)` Created: 07/Apr/16  Updated: 10/Apr/16

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(ns my.car)

(defprotocol Car
(drive [this]))

(extend-type js/String
Car
(drive [this] (map #({"a" "A"} %) [this (str this)])))

(drive "a"); (nil "A") expected ("A" "A")

See the reproduction of the bug in a live environment with KLISPE here: http://app.klipse.tech/?sourcce=jira&cljs_in=(ns%20my.car)%0A%0A(defprotocol%20Car%0A%20%20(drive%20%5Bthis%5D))%0A%0A(extend-type%20js%2FString%0A%20%20Car%0A%20%20(drive%20%5Bthis%5D%20(map%20%23(%7B%22a%22%20%22A%22%7D%20%25)%20%5Bthis%20(str%20this)%5D)))%0A%0A%0A(drive%20%22a%22)%0A



 Comments   
Comment by Francis Avila [ 07/Apr/16 6:27 PM ]

This is because of boxing and the implementation of cljs.core._EQ_

By extending type js/String (the String object/class in javascript) instead of "string", your "this" will be the boxed string rather than the primitive string (in non-strict js mode--in strict mode it will be the primitive also). The (str this) is coercing the boxed string back to a primitive string.

The core issue is really:

(= (js/String. "a") "a") ;=> false
;; thus
({"a" "A"} (js/String. "a")) ;=> nil

You should really use

(extend-type string ...)
Comment by Francis Avila [ 07/Apr/16 6:41 PM ]

BTW this appears to be different from Clojure where primitives and boxed-primitives appear equal:

;; Clojure code
(= (String. "a") "a")
=> true
(= (Long. 1) 1)
=> true
(= (Long. 1) (long 1))
=> true

Not sure if clojurescript should try to replicate this more closely or not.

Clojurescript bottoms out with triple-equals in most cases, which is why primitives and boxes do not compare equal. To get them to compare equal would require adding special (instance? js/BOXED x) checks and some modifications to existing -equiv implementations which extend primitive types. (e.g. (extend-type number IEquiv ...) uses identical? without checking if the right-hand side is boxed or not.)

Comment by Mike Fikes [ 10/Apr/16 12:24 AM ]

As Francis alludes to, this is not a bug. If you do (doc extend-type), it indicates the type-sym can be string to cover the base type js/String, and it elaborates with

Note that, for example, string should be used instead of js/String.
and if a user does try to use js/String as the type-sym argument, a diagnostic is issued:

WARNING: Extending an existing JavaScript type - use a different symbol name instead of js/String e.g string at line 1




[CLJS-1625] Clojurescript macros used in named function are expanded two times because the analyzer performs a two pass analysis when analyzing named functions Created: 16/Apr/16  Updated: 19/Apr/16

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

Type: Defect Priority: Minor
Reporter: Ewen Grosjean Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1625.patch    

 Comments   
Comment by Ewen Grosjean [ 16/Apr/16 9:41 AM ]

During the first analysis of named functions, only the function definition is analyzed in order to know its function-ness/arity. Its body is only analyzed during the second pass.

Comment by Kevin Downey [ 17/Apr/16 12:09 AM ]

http://dev.clojure.org/jira/browse/CLJS-1617 seems to add a similar issue





[CLJS-1639] Invalid file path for cljs.core.load_file on windows Created: 13/May/16  Updated: 21/May/16

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

Type: Defect Priority: Minor
Reporter: Jay Lee Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Windows 10, CLJS 1.8.51



 Description   

When I tried to build reagent based app with nodejs target,
I got an invalid file path generated case which is basically loading external javascript file.
I captured the image as following:

At line 4, the path should have double backward-slash on windows.

I has built a CLJS app which is based on reagent framework with nodejs target. The build environment is somewhat strange but I have a case to use it. Here is a reproduce steps.

  1. Open command prompt on windows 10 and execute command as following:
    lein new figwheel sample00 – --reagent
  2. Open project.clj file and update one of dependencies:
    project.clj
    ;; ...
    :dependencies [[org.clojure/clojure "1.8.0"]
                     [org.clojure/clojurescript "1.8.51"]
                     [org.clojure/core.async "0.2.374"
                      :exclusions [org.clojure/tools.reader]]
    
                     [reagent "0.6.0-alpha" :exclusions [cljsjs/react]]
                     [cljsjs/react-with-addons "0.14.3-0"]
                     ]
    
    ;; ...
    :cljsbuild {:builds [{:id "dev"
                          ;; ... 
                          :compiler {
                            ;; ...
                            :target :nodejs
                          }
                         }]
               }
    ;; ...
  3. Build with lein cljsbuild once dev
  4. Open <project_root>\resources\public\js\compiled\out\reagent\impl\util.js
  5. At line number 4 in my environment, the generated code is
    cljs.core.load_file("resources\public\js\compiled\out\react-with-addons.inc.js");
    However I believe the correct path string should be cljs.core.load_file("resources\\public\\js\\compiled\\out
    react-with-addons.inc.js");
    .

Backward-slash needs to be double on Windows env.
When I lunched doo test command with nodejs target, it complained about the given path cannot be loaded.

Thanks.



 Comments   
Comment by David Nolen [ 21/May/16 5:07 PM ]

This ticket is in danger of being closed. The ticket should demonstrate a reproducible bug without relying on any 3rd party tools or libraries. No Leiningen, Figwheel, or Reagent. Please demonstrate the Windows issue with only ClojureScript.

Thanks.





[CLJS-1572] REPL doesn't give error for expressions with too many right parentheses. Created: 15/Feb/16  Updated: 24/May/16

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

Type: Defect Priority: Minor
Reporter: J David Eisenberg Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: repl
Environment:

Fedora 23, java version "1.8.0_40", javac 1.8.0_40, clojure 1.7.0


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

 Description   

I was expecting an error message from this; using [org.clojure/clojurescript "1.7.228"]; the Clojure REPL does produce an error.

To quit, type: :cljs/quit
cljs.user=> (+ 3 5)))))
8


 Comments   
Comment by Mike Fikes [ 16/Feb/16 12:49 PM ]

A suggestion on a strategy to fix this: Make the ClojureScript REPL sequentially process all of the forms it can read on a line, just like the Clojure REPL does:

user=> 3 (+ 3 5) 7
3
8
7

If this is done, then the fix for this ticket will fall out “for free” and the ClojureScript REPL will error when it hits a form that appears to start with ).

Comment by Mike Fikes [ 21/Feb/16 4:01 PM ]

The REPL code is very close to working the way mentioned in the previous comment. It currently does not only because this line

https://github.com/clojure/clojurescript/blob/c59e957f6230c07e7a228070dd8eb393d5b8ce40/src/main/clojure/cljs/repl.cljc#L100

invokes code that causes a new PushbackReader to wrap things (discarding things):

https://github.com/clojure/clojurescript/blob/c59e957f6230c07e7a228070dd8eb393d5b8ce40/src/main/clojure/cljs/repl.cljc#L773-L775

If you either let the PushbackReader once and let that reader fn close over it, or otherwise comment out things so that a new PushbackReader is not created for each loop / recur, you will see that the code behaves as suggested in the previous comment, having the desired effect.

The only thing I see that would need to be additionally sorted out with such a patch is being a little more clever about when need-prompt evaluates to true, etc. (otherwise polishing thing so there are no missed corner cases).

Comment by Mike Fikes [ 21/Feb/16 11:02 PM ]

Attached a patch that, in essence makes the ClojureScript REPL behave like the Clojure REPL with respect to multiple items on a line and with respect to detecting malformed input. The patch is fairly straightforward, but could use some testing. I've tried things like

cljs.user=> 3_    ; where _ here is a space

cljs.user=> 3 4 5

cljs.user=> 3)

cljs.user=> 3))

cljs.user=> 3 [4
5]

cljs.user=> (let [x 1]
(+ 1 "a"))         ;; testing to make sure line numbers are right

All the above is looking good to me.

Here is the commit comment:

Take extra care to preserve the state of in so that anything beyond
the first form remains for reading. This fundamentally makes the
ClojureScript REPL behave like the Clojure REPL. In particular, it
allows entering multiple forms on a single line (which will be evaluated
serially). It also means that if malformed input lies beyond the initial
form, it will be read and will cause an exception (just like in the
Clojure REPL).

The bulk of the complexity in this commit has to do with the case where
a new line-numbering reader is established, so that errors in forms
can be associated with line numbers, starting with line 1 being the
first line of the form. This requires a little extra handling because
the source-logging-push-back-reader introduces an extra 1-character
buffer which must be transferred back to the original (pre-bound) in,
otherwise things like an unmatched extra paren right after a well-formed
form won't be detected (as the paren would be in the 1-char buffer and
discarded.)

Also, a Java PushbackReader needs to be eliminated, as it causes things
to fail to behave like the Clojure REPL.

Comment by Mike Fikes [ 21/Feb/16 11:14 PM ]

Note that one extremely useful thing this patch enables is pasting of multiple forms into a ClojureScript REPL!

This fails if pasted using the current cljs.jar, but works with the patch applied:

(def a 1)

(def b 2)

(def c (+ a b))

c
Comment by Mike Fikes [ 24/May/16 3:19 PM ]

I tested this with Figwheel [figwheel-sidecar "0.5.0-6"] and it worked properly evaluating multiple forms on a single line, evaluating pasted forms (as in the previous comment), and it properly indicates Unmatched delimiter ) for the case in the description.





[CLJS-1644] Recursive protocol method for a record never terminates Created: 23/May/16  Updated: 26/May/16

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

Type: Defect Priority: Minor
Reporter: Brian Stiles Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Any



 Description   

When extending a protocol for a record, a recursive method can never terminate in some cases.

(defrecord R [a])

(defprotocol P
  (f [x]))

(extend-protocol P
  R
  (f [x]
    (if x
      (recur nil)
      x))
  default
  (f [x]
    (if x
      (recur nil)
      x)))

(prn (f 1))        ; #1
(prn (f (R. 1)))   ; #2

prn call #1 prints nil as expected, but prn call #2 never terminates.

It looks like the compiler creates a variable assigned to this within the while loop such that the test of "x" is always really testing this when it should be testing the value of x passed in by the call to recur.

Note, I'm testing ClojureScript 1.8.51. The "Affects Version/s" field above only gives 1.7.228 as the most recent version.



 Comments   
Comment by Brian Stiles [ 23/May/16 4:14 AM ]

Actually, "always really testing 'this' when it should be testing the value of x passed in by the call to recur" is only true if the type of the value represented by x remains the same. If, as in the example, the type changes, the call should be dispatched to the properly matching method implementation.

This seems to behave as expected in Clojure.





[CLJS-1678] variadic defn can be called for missing fixed arities, overlapping arity Created: 11/Jun/16  Updated: 12/Jun/16

Status: Reopened
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   

For defns with a variadic arity: if invoked with a missing fixed arity, they use the variadic method instead of erroring; if invoked with a fixed arity that is the max fixed arity, variadic mathod instead of fixed form is invoked.

(defn f-hole
  ([a] 1)
  ([a b c d & args] "4 or more"))

(f-hole 1 2) ; =>"4 or more", should be error

(defn f-overlap-mfa
  ([a b] 2)
  ([a b & c] "2+"))

(f-overlap-mfa 1) ;=> "2+", should be error
(f-overlap-mfa 1 2) ;=> "2+", should be 2
(f-overlap-mfa 1 2 3) ;=> "2+", correct

A way to fix the f-hole bug is to emit a "case X:" into the switch statement for all X with no signature or less than max-fixed-arity.

The f-overlap-mfa I'm not sure why is happening and didn't investigate deeply.



 Comments   
Comment by Francis Avila [ 11/Jun/16 8:31 AM ]

Sorry, filed against CLJ instead of CLJS!

Comment by Rohit Aggarwal [ 12/Jun/16 9:41 AM ]

The behaviour I am seeing for f-overlap-mfa is:

(f-overlap-mfa 1) ;=> "2+"
(f-overlap-mfa 1 2) ;=> 2
(f-overlap-mfa 1 2 3) ;=> "2+"

So the two argument result is different for me than you, Francis Avila.

The call with just one argument does give a warning though:

WARNING: Wrong number of args (1) passed to cljs.user/f-overlap-mfa





[CLJS-1587] Duplicate keys via quoting Created: 24/Feb/16  Updated: 15/Jun/16

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

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

Attachments: File CLJS-1587.diff    
Patch: Code and Test

 Description   

For

#{1 '1}

you get

#{1 1}


 Comments   
Comment by Peter Schuck [ 03/Mar/16 10:01 PM ]

This happens for the has-set macro and the hash-set literal. Here's what I get from the repl

cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}

Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.

Comment by Rohit Aggarwal [ 15/Jun/16 8:02 AM ]

The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.

Comment by Rohit Aggarwal [ 15/Jun/16 9:32 AM ]

I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function.

Patch has the fix and a test.





[CLJS-527] Support dynamic runtime extension of protocols to types Created: 20/Jun/13  Updated: 11/Dec/13

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

Type: Enhancement Priority: Minor
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File CLJS-527.diff    
Patch: Code and Test

 Description   

Here is a transliteration of a semi-common pattern used with Clojure protocols to dynamically extend protocols to concrete types implementing other protocols (or interfaces, on the JVM):

(defprotocol P (m [this]))

(extend-protocol P
  object
  (m [this]
    (if (seq? this)
      (do
        (extend-type (type this) P
          (m [this] (count this)))
        (m this))
      (throw (ex-info "Cannot extend m to type" {:type (type this)})))))

(I think dnolen was the first to talk about this outside of irc.) Unfortunately, this does not work in ClojureScript; extend-type currently requires that the type be specified as a symbol:

clojure.lang.ExceptionInfo: clojure.lang.PersistentList cannot be cast to clojure.lang.Named at line 4  {:tag :cljs/analysis-error, :file nil, :line 4, :column 5}

I can (hackily?) make this work by simply not attempting to resolve tsym here. However, that leaves lists in as values for :tag metadata (which might be used by the analyzer and/or other tools that depend upon it?), which I presume is not OK.

If someone can provide guidance on a sane path from here, I'll do what I can to produce a plausible patch.



 Comments   
Comment by Chas Emerick [ 21/Jun/13 12:08 PM ]

Looks like jvm.tools.analyzer emits a :tag of nil for some corresponding Clojure code; this can be seen by running this:

(require '[clojure.tools.analyzer :refer (ast)])
#_= nil
(defprotocol P (m [this]))
#_= P
(ast (fn [x]
       (extend-type (type x)
         P
         (m [this] (count this)))))
#_= ...

(The output is verbose enough that I'm not bothering to paste it here.) So, that's easy enough to do, and makes the original example work in ClojureScript.

However, simply suspending the lookup of what is currently assumed to be a symbol naming the type being extended isn't enough. With only that, dynamic usage of extend-type will affect js native prototypes, e.g.:

ClojureScript:cljs.user> (defprotocol P (m [this]))
nil
ClojureScript:cljs.user> (defn naive-dynamic-extend [x]
  (extend-type (type x)
    P
    (m [this] "hi")))
...
ClojureScript:cljs.user> (naive-dynamic-extend true)
...
ClojureScript:cljs.user> js/Boolean.prototype.cljs$user$P$m$arity$1
#<
function (this$) {
    return "hi";
}
>

So the bits in extend-type that handle base types (boolean, string, function, array, etc) need to be brought over to runtime. Looking into this now.

Comment by Chas Emerick [ 24/Jun/13 8:22 AM ]

Patch attached. All previously-allowed usage of extend-type continues to emit exactly the same code. Extensions without a statically-named type include both possible code paths:

1. When the type is a JavaScript native, the extension is made on the prototype's fns using the same base type names as are used for static extensions to e.g. string, object, etc
2. When the type is some other prototype, the extension is made on it directly.

This yields code like:

ClojureScript:cljs.user> (defprotocol P (m [this]))
nil
ClojureScript:cljs.user> #(extend-type (type %) P (m [this] "hi"))
#<
function (p1__4810_SHARP_) {
    var G__4813 = cljs.core.type.call(null, p1__4810_SHARP_);
    var temp__4090__auto__ = (cljs.core.base_type[G__4813]);
    if (cljs.core.truth_(temp__4090__auto__)) {
        var G__4814 = temp__4090__auto__;
        (cljs.user.P[G__4814] = true);
        return (cljs.user.m[G__4814] = ((function (G__4814, temp__4090__auto__, G__4813) {
            return (function (this$) {
                return "hi";
            });
        })(G__4814, temp__4090__auto__, G__4813)));
    } else {
        G__4813.prototype.cljs$user$P$ = true;
        return G__4813.prototype.cljs$user$P$m$arity$1 = ((function (temp__4090__auto__, G__4813) {
            return (function (this$) {
                return "hi";
            });
        })(temp__4090__auto__, G__4813));
    }
}
>

The duplication of the prototype method implementation bodies is unfortunate, a side effect of keeping the extend-type macro and supporting emit-* fns relatively simple. (Note that advanced compilation doesn't lift and merge those fns.) I'm inclined to say that it's a reasonable tradeoff, at least for now, as it only affects the dynamic type extension case; a reasonable TODO later, perhaps.

Comment by Brandon Bloom [ 03/Jul/13 3:44 PM ]

At Chas' request, I took a look at the patch. Tests pass locally & my few small toy projects run fine. I haven't benchmarked.

My only real concern is pretty minor: I'm terrified of JavaScript's semantics around typeof, toString, etc. The existing code paths leverage goog.typeOf, which has some pretty hairy internals. Meanwhile, Chas is just implicitly toString-ing on some type objects with an array set. The code of goog.typeOf also discusses oddities of Object.prototype.toString in firefox, but presumably that won't matter via the implicit conversion present in the array set. So if this works in all the major browsers, the patch LGTM.

Comment by Chas Emerick [ 03/Jul/13 6:29 PM ]

Just a point of documentation w.r.t. the stringifying of js-native prototypes: given the initial example above, if (type x) (or, whatever expression the user is providing that will return a "type" to extend) returns a js-native prototype, we need some way to map that at runtime to the strings that ClojureScript uses for those types when performing protocol dispatch. Using a js object containing as literal a representation of that mapping as possible seemed like a reasonable option. Providing a fn that cond's through the various options would be equivalent AFAICT.

A separate larger issue is, what is a type in ClojureScript? As far as protocols are concerned, the type of types is approximately the union of all non-native js prototypes, and symbols identifying those natives. However, type (and, really, any user of ClojureScript writing expressions provided to extend-type) doesn't know about the latter or the carve-out w.r.t. prototypes, thus some implicit runtime conversion is needed. Alternatively, one could say that any expression provided to extend-type must respect that contract, but then (a) users would need to explicitly handle js native types, and (b) Clojure/ClojureScript portability would be further complicated in this department.

Comment by David Nolen [ 03/Jul/13 8:02 PM ]

Reviewing the patch, thanks all.

Comment by David Nolen [ 03/Jul/13 8:09 PM ]

Ok what is the base-type js-obj for? Why aren't we using goog.typeOf?

Comment by Chas Emerick [ 03/Jul/13 9:06 PM ]

We can't use goog.typeOf because extend-type works with a type (i.e. the return of (type x)), not a value the type of which should be extended to the given protocol(s). (goog.typeOf will always return "function" for prototypes, js-native or not.)

The ClojureScript cljs.core/base-type js-obj is simply a runtime-accessible analogue of the (Clojure) cljs.core/base-type map, except it maps js-native prototypes to the goog.typeOf strings that are used for protocol dispatch.

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

Ok I looked at the patch some more, I don't really like the string coercion aspect around base-type. Let's switch this to an array-map.

Comment by Chas Emerick [ 16/Jul/13 6:48 AM ]

Sure, I can do that. FWIW, that will rope in PAM and whatever other persistent data structure and printing bits it depends upon by default…is that considered acceptable?

Comment by David Nolen [ 16/Jul/13 10:31 AM ]

Hrm, that's actually a good point. Perhaps better to do a array + scan. I thought about this patch some more and it really needs more work. One thing this doesn't handle is objects from foreign contexts. ClojureScript can currently handle this by combining default cases with goog.typeOf.

I think extend-type should probably work with strings and/or symbols that represent the base types so that objects from other contexts can also be handled. I think automating this will be unweildy but at least it gives users the flexibility to handle these cases themselves.

Comment by Chas Emerick [ 16/Jul/13 7:14 PM ]

What do you mean by "foreign contexts"? I did a bit of searching on the term, and didn't turn up anything promising in connection with either ClojureScript or JavaScript. I assume you're not referring to e.g. types loaded via :foreign-libs, but who knows…

Re "strings and/or symbols", are you suggesting that dynamic usage of extend-type should not perform any translation of js-native prototypes to their string names, i.e. an expression being evaluated to determine the type to extend would need to return "string" (or 'string) rather than js/String?

Comment by David Nolen [ 16/Jul/13 9:00 PM ]

JavaScript objects from other JS execution contexts, IFrames are the most common source of these. This is why goog.typeOf implementation is so complex, it handles these cases.

I'm saying that extend-type should do run time extension to JS natives if the user specifies the extension at runtime via a string or symbol for the native cases because an Array from another JS Execution context is not equal to the Array in the current one.

Comment by Brandon Bloom [ 23/Jul/13 2:04 PM ]

It seems silly to argue about all the edge cases here, considering how many edge cases pertaining to "types" are already broken in ClojureScript.

For example, currently (= (type :foo) (type "foo"))

This is because cljs.core/type simply calls accesses the constructor field, and keywords are strings at runtime. Meanwhile, the (type (type x)) is always a function, since there is no Type type.

There are three problems:

1) Type equality

2) Getting an object's type

3) Runtime protocol extension

This patch delegates #2 to cljs.core/type and properly addresses #3.

#1 is a bit trickier, since there are three valid approaches I can think of:

A) Nominal equality - Enhance cljs.core/type to return sensible symbols, by implementing the crux of the goog/typeOf behavior plus some extra behavior for extracting type names out of function string representations.

B) Constructor equality - Simply compare .constructor; This is basically what happens now, but has 2 problems: B1) Doesn't provide for types at compile time B2) might not work correctly with IFrame execution environments

C) Hybrid/Heuristic - (defprotocol IType ...) and implement some Type objects with equality sensible operators; lazily stuff those type objects into a reflection map of some sort.

Personally, I think that B (the current state of the world) is hopelessly broken. Despite my initial reservations regarding the toString coercion, I think this patch does a reasonable job of eschewing B for a stop-gap A (with compile time interop). Given this analysis, I think the string coercion for natives actually does a better job than one could do with a PAM of constructors: ie the coercion covers the remote execution state. Unless this is provably broken for some key scenarios with IFrames, I think the patch is good as is, but we need to think about a follow on patch for fixing up runtime types in general.

Comment by Brandon Bloom [ 23/Jul/13 2:23 PM ]

I should also point out: Unlike JavaScript, Java has a unified nominal type system. Name equality is type equality (ignoring custom class loaders). However, JavaScript with Google Closure has a stratified type system: The dynamic type system utilizes object identity for equality. The GClosure static type system is (mostly) nominal with some fudge factor for the mismatch with the runtime type system (mostly around inheritance/mixins/array-like/etc). I think that ClojureScript should strive for a runtime reification of the Google Closure type system, since that would be most compatible with the Clojure/JVM type system.

Comment by David Nolen [ 23/Jul/13 3:06 PM ]

We are not going to follow goog.typeOf.

Comment by Brandon Bloom [ 23/Jul/13 3:22 PM ]

Follow it where?

Comment by David Nolen [ 23/Jul/13 3:29 PM ]

We're not going to use it nor follow its example for determining types unless we are trying to detect natives.

Comment by Brandon Bloom [ 23/Jul/13 3:34 PM ]

Getting back on topic: Getting some type-like-thing from an object is not this patch.

This patch is about extend-type, which I think it implements reasonably well given our current failings at runtime type reification.

Chas has this working with user defined types as well as with natives. Are there any particular scenarios that are provably broken? Either in general or on a particular browser/runtime?

Comment by David Nolen [ 23/Jul/13 3:38 PM ]

Chas's patch can't catch natives from IFrame contexts, I'd rather this patch move forward with at least the ability for a user to handle that situation themselves which I said above.

Comment by Brandon Bloom [ 23/Jul/13 3:59 PM ]

I think this does handle natives from iframe contexts, since extend-type takes a "type" not an object. Getting the type from an object does not need to happen here. The patch coerces types to a string via toString, which is precisely how goog.typeOf works internally on natives. Search for Object.prototype.toString.call in http://docs.closure-library.googlecode.com/git/closure_goog_base.js.source.html

Are you speculating that the patch doesn't work, or have you tried it?

If the former, Chas: Can you provide a test project that demonstrates extension of the cross product of these two sets:

1) local type
2) request remote object, coerce to type locally
3) request remote type object

A) native objects
B) deftype-ed objects

Comment by Chas Emerick [ 23/Jul/13 7:42 PM ]

Whatever the semantics and dark corners of JavaScript "types" — or, what they should be, at least w.r.t. ClojureScript — extend-type has very little latitude to operate.

The runtime-dynamic variant of the code it generates will be expecting something typeish coming out of whatever expression the user provides to it.
AFAICT, the only sane possibilities for "typeish" in this context are strings naming javascript natives (e.g. "string", or perhaps 'string if we want to be generous), or a constructor fn (cljs.core/PV, or js/String, or anything else returned by type). The current patch only accepts the latter, done to preserve as much as possible the existing patterns of extend-type usage in Clojure, and hopefully avoid foisting conversion of js/String to "string" at runtime onto users. String coercion is used to normalize the former into the latter; since the code determining the typeish value is entirely in the hands of the user (we don't have access to an object that exemplifies the type to which the user is extending, so we can't wedge in anything particularly clever), I believe it (or something similar) is all we can do.

From here, the only other option I see would be to expand the patch to eliminate this coercion, accepting strings or symbols naming js natives ("string", "boolean", and so on), and allow extensions to js natives at runtime without restriction. This may be a feature for some (perhaps if someone wants to extend a protocol to a js native only withing a particular iframe context?); on the other hand, we should probably document heavily that runtime usage of extend-type should take care to perform the sort of coercion the current patch does (and maybe provide some kind of helper function?), insofar as extension to natives directly is considered harmful in general (e.g. http://dev.clojure.org/jira/browse/CLJS-528, which was viewed favorably in irc some weeks ago?).

I'm happy to produce further tests (up to the suite that Brandon suggested above) if that would be helpful.

Comment by Michał Marczyk [ 26/Jul/13 7:04 PM ]

Just wanted to note that I've run into a situation where runtime extension of protocols to types would AFAICT be the next best thing to "extending protocol to protocol". Here's a link to the relevant ticket in fipp's issue tracker: https://github.com/brandonbloom/fipp/issues/6 (relevant part starts in the 8th comment).





[CLJS-1076] :nashorn target Created: 02/Mar/15  Updated: 02/Mar/15

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

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


 Description   

To run well on Nashorn the target should supply CLOSURE_IMPORT_SCRIPT as well as setTimeout or setImmediate for core.async.






[CLJS-1712] Make PersistentHashSet implement IReduce Created: 21/Jul/16  Updated: 21/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File difference-benchmark.txt     Text File into-benchmark.txt     Text File phs-reduce.patch     Text File union-benchmark.txt    
Patch: Code

 Description   

This improves speed of many reduce based operations on set which were falling back to seq-reduce, including code in `clojure.set` namespace such as `clojure.set/union` and `(into [] some-set)`.

I've included a few benchmarks I performed using `simple-benchmark` in a JavascriptCore environment (Planck REPL)



 Comments   
Comment by Rohit Aggarwal [ 21/Jul/16 3:35 PM ]

I think the code currently is faithful to Clojure's implementation of PersistentHashSet. So any change from that would probably require more thought and/or history.

Also someone else also raised a similar issue on ClojureScript mailing list.





[CLJS-1709] clojure.data/diff throws an exception when comparing map keys of different types when used on sorted maps Created: 19/Jul/16  Updated: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Thomas Scheiblauer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   

e.g.
(clojure.data/diff (sorted-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (sorted-map :foo 42) (sorted-map "x" 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map "x" 42))
will throw e.g.
Error: Cannot compare :foo to 1
while e.g.
(clojure.data/diff (hash-map :foo 42) (hash-map 1 42))
(clojure.data/diff (hash-map :foo 42) (hash-map "x" 2))
(clojure.data/diff (sorted-map :foo 42) (sorted-map :bar 42))
will not.

The same applies to Clojure with a different exception (e.g. "java.lang.Long cannot be cast to clojure.lang.Keyword")






[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-1109] Record type name and advanced optimization Created: 12/Mar/15  Updated: 12/Mar/15

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

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


 Description   

It is not possible to query type name in advanced compilation.
Code below prints correct record name in other compilation modes, but under advanced compilation it prints constructor source code.

(defrecord FooBar [a])

(def fb (FooBar. 1))

(prn (-> fb))
(prn (-> fb type))
(prn (-> fb type pr-str))





[CLJS-677] cljs.reader doesn't support keywords starting with a digit Created: 12/Nov/13  Updated: 10/Sep/15

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

Type: Defect Priority: Minor
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: None


 Description   
ClojureScript:foo> (r/read-string ":0")
"Error evaluating:" (r/read-string ":0") :as "cljs.reader.read_string.call(null,\":0\")"
org.mozilla.javascript.EcmaError: TypeError: Cannot read property "0.0" from null (file:/home/chas/dev/clojure/cljs/.repl/cljs/reader.js#451)

The topic of leading digits in keywords came up separately, as they've been supported in Clojure for some time, but can now be considered part of the spec, as it were. See CLJ-1286.

BTW, this is another simple-check win...



 Comments   
Comment by Chas Emerick [ 21/Nov/13 9:38 AM ]

This is not a simple regex change, as I had hoped given the recent flurry in Clojure. The symbol pattern in cljs.reader is faithful to Clojure HEAD, but the processing of matches isn't. I think it may be a wash as to whether it'd be easier to fix what's there vs. porting clojure.tools.reader.impl.commons/parse-symbol (which incidentally doesn't use a regex)…either way, leaving it for another day (or someone else, if they're up for it).

Comment by Francis Avila [ 02/Jul/14 12:35 AM ]

I think I fixed the match processing issue you're talking about (CLJS-775 CLJS-776)? However I'm still confused by this and CLJ-1286. The clojure reader docs and edn spec still say they should reject `:0`, but 1.6.0 doesn't. What's the expected behavior? Is the spec going to be fixed, or clojure reader fixed once downstream packages are fixed?

Comment by Jozef Wagner [ 02/Jul/14 1:50 AM ]

AFAIK EDN specs do not reject :0 (no rule that the second character cannot be a digit). See https://github.com/wagjo/serialization-formats for my interpretation of existing specs.

Comment by Francis Avila [ 02/Jul/14 1:35 PM ]

Ah, I think I see the source of the confusion. Both EDN and the clojure reader spec both say something like "keywords are like symbols, except beginning with a colon." The confusion lies in whether we interpret that as meaning

  1. First character is a colon, then the second character and after are matched against the symbol definition.
  2. The first character is a colon, and the whole form is matched against the symbol definition.

CLJ-1003 CLJ-1252 and CLJ-1286 and myself all seem to understand the first meaning. This might be because when we say "the first character of a keyword" we typically mean the first character after the colon, as if the colon is "special" and not part of the keyword (e.g. like a reader macro character).

However clojure 1.6 seems to be following the second meaning (and explains why `:0/a` is ok but not `:0/0`), and I'm not sure from the cited tickets and google group discussions whether this is because of downstream breakage or if this is the intended interpretation and the patch from CLJ-1252 was accepted by Alex Miller erroneously.

Note if we accept the second interpretation, then the restriction "A symbol can contain one or more non-repeating ':'s." from the clojure reader docs is incorrect for keywords. (EDN doesn't allow namespace-expanded keywords, it seems, so it's not an issue there.)

Also EDN allows contiguous colons in symbols, whereas clojure 1.6 and the reader spec do not.

Comment by Francis Avila [ 02/Jul/14 2:11 PM ]

Also clojure 1.6 allows a/:a and :a/:a (where name part violates first-character rule for symbols), even though the specs do not. (This is something your table doesn't mention. Very thorough work BTW! I wish the reader spec was more formalized and unambiguous...)

Comment by Francis Avila [ 02/Jul/14 3:08 PM ]

I think this pattern follows the specs:

#"(?x)
(?!///) # edge case: / only allowed in name part.
# name or namespace part of symbol or keyword
(?:
 #division symbol
 (/
 # normal symbol
 |[a-zA-Z*!_?$%&=<>][0-9a-zA-Z*!_?$%&=<>\#:+.-]*
 # symbol starting with [-+.]
 |[-+.](?:[a-zA-Z*!_?$%&=<>\#:+.-][0-9a-zA-Z*!_?$%&=<>\#:+.-]*)?)
 # keyword
 |(::?)([0-9a-zA-Z*!_?$%&=<>\#:+.-]+))
# name part when namespace is present
(?:/(/ # division symbol
    |[a-zA-Z*!_?$%&=<>][0-9a-zA-Z*!_?$%&=<>\#:+.-]*
    |[-+.](?:[a-zA-Z*!_?$%&=<>\#:+.-][0-9a-zA-Z*!_?$%&=<>\#:+.-]*)?))?
# groups:
# 1: symbol name or namespace 2: keyword colon(s) 3: keyword name or namespace
# 4: keyword or symbol name (and groups 1 and 3 are namespaces)"

Problems:

  1. Does not enforce no-repeating-colon rule (but it is easy to validate after matching).
  2. Rejects violations of first-character-rule in symbols which clojure accepts.
  3. Accepts a trailing colon on namespace (unlike clojure).
  4. Accepts foo// or :foo//, which are not clearly addressed by the specs. (Jozef's table has more background). These are both allowed in Clojure 1.6, but not 1.5 or (arguably) edn.
Comment by Francis Avila [ 02/Jul/14 6:28 PM ]

Another problem: Accepts :::a/b, which I think is ok per the specs but is not read by 1.6. Crazy example:

user=> (require ['clojure.core :as (symbol ":a")])
nil
user=> :::a/map

RuntimeException Invalid token: :::a/map  clojure.lang.Util.runtimeException (Util.java:221)
user=> (resolve (symbol ":a" "map"))
#'clojure.core/map

Theoretically I might expect :::a/map to be read as :clojure.core/map?

Comment by Nicolás Berger [ 10/Sep/15 6:44 AM ]

Bumping this up, as I just scratched my head for an hour to find out this was the culprit

Comment by David Nolen [ 10/Sep/15 7:15 AM ]

Nicolás, the premise of the ticket is that this should be supported when clearly the Clojure documentation about valid keywords states that it isn't. The Clojure implementation just happens to allow it. In anycase, this needs to be sorted out in Clojure first.

Comment by Francis Avila [ 10/Sep/15 9:35 AM ]

I think CLJ-1527 is currently the ticket where this problem is pursued.





[CLJS-1726] demunge is too agreesive and incorrect in some cases Created: 04/Aug/16  Updated: 04/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I have implemented some "demunging" logic in cljs-devtools (and dirac) to present original user-friendly names in UI.

During my testing I spotted some wrong edge-cases and incorrect behaviours of demunge:

1) it is too aggressive in replacing dollars - some dollars can be "real" dollars as part of original name
2) it does not revert js-reserved? transformation applied during munging
3) it is oblivious to underscores/dashes - some underscores were "real" underscores before munging
(this may be not complete)

I have worked around those issues on my side and implemented some heuristics[1] based on context, but it is far from perfect.

I'm not sure how to properly fix those, so I wanted to open a ticket with discussion. Maybe people will have some clever ideas.

Currently munging is lossy and we probably don't want to touch it for compatibility reasons.
Maybe we could mark original underscores and dollars in some way, so demunge could properly skip them.

1) One strategy could be to use some (rare) unicode characters, but that would be problematic for people to type.
2) Another strategy could be to escape original dollars and underscores somehow (using more dollars and underscores .
3) Better ideas?

[1] https://github.com/binaryage/cljs-devtools/blob/52899e61e33373df36be8dcb23c69377936821b2/src/lib/devtools/munging.cljs#L154-L185






[CLJS-1722] Upgrade ExceptionInfo to proper deftype Created: 03/Aug/16  Updated: 04/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to the deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0



 Comments   
Comment by Thomas Heller [ 04/Aug/16 4:25 AM ]

Why not just add the missing getBasis, cljs$lang$type, cljs$lang$ctorStr bits per set!?

The patch looks like it would mess up advanced compilation although that is just an instinct not something I verified, did you?

Comment by Antonin Hildebrand [ 04/Aug/16 4:44 AM ]

I ran clojurescript tests and I assumed they run also against advanced-mode build. During development when my tests were failing I saw error messages about minified names.

This may seem as a hacky solution, but IMO the original code was also a hack. My hack will stay up-to-date with future changes to deftype implementation. I can imagine people would forget to update this part when touching deftype.

btw. there is another patch coming related to discrepancies between deftype and defrecord. That could have been avoided if defrecord shared common implementation with deftype.

Comment by Thomas Heller [ 04/Aug/16 6:27 AM ]

Closure is usually very strict about re-defining stuff but I guess my instincts were wrong, the tests should cover advanced.

My issue with this is that deftype is for defining Clojure-specific types. ExceptionInfo is not since it inherits from Error, just like you can't have a superclass in Clojure you can't in CLJS. So IF we were to change deftype in the future we might break things in unexpected ways that just re-use deftype but aren't actually deftype.

Yes, you have to do some house-keeping but you can't enforce the rules of deftype when dealing with inheritance.

Just my 2 cents, it has advantages to re-use deftype too (as you suggested).

Comment by Antonin Hildebrand [ 04/Aug/16 6:36 AM ]

Unfortunately I was unable to look up any comments or docs explaining the reasoning why we do that js/Error inheritance there.

My first attempt to "fix" ExceptionInfo was to simply implement it as an ordinary deftype. And that worked just fine (for my tests). Then I tried to re-implement original behaviours on top just to make it 100% compatible.

Comment by Antonin Hildebrand [ 04/Aug/16 12:47 PM ]

Just adding a motivational screenshot:

https://dl.dropboxusercontent.com/u/559047/CLJS-1722-example.png

Those yellow warnings are listing which properties are getting copied by gobject/extend call.
The expanded log item is new implementation logged via cljs-devtools v0.8.0.
The last log item is the old implementation logged via cljs-devtools v0.8.0 (cljs-devtools does not recognise ExceptionInfo as CLJS type, but detects IPrintWithWriter and uses it to present the value)





[CLJS-1474] Error if reserved symbol is defined Created: 21/Oct/15  Updated: 31/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

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

 Description   

Currently a definition like

(defn set! [] ...)

will not cause any warning. Any usage of it (without :as namespace aliasing) however will not use the defined var but the set! special form.

A warning seems appropriate.



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 1:19 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 2:09 PM ]

I know David suggested that a hard error is probably the right thing to do for this one, but one consequence is that the cljs.spec/def macro cannot be defined in bootstrap with this change. (I haven't investigated thoroughly, but this may simply be the result of macros being processed as ClojureScript in bootstrap, and thus being subject to this new guard.)

Regardless of the root cause, you'll see this if you try to run script/test-self-parity:

#error {:message "Could not eval cljs.spec", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't def special form at line 51 ", :data {:file nil, :line 51, :column 1, :tag :cljs/analysis-error}}}

For reference: Line 51 currently points at the def macro: https://github.com/clojure/clojurescript/blob/e2db5d9ff8cb6a099ebc2a8cd379385bf4649b38/src/main/cljs/cljs/spec.cljc#L51





[CLJS-1164] quot and rem are inefficient Created: 24/Mar/15  Updated: 20/Feb/16

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

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: math

Attachments: Text File CLJS-1164-1.patch     Text File cljs-1164.patch    
Patch: Code and Test

 Description   

The implementation of the quot and rem functions are needlessly complicated. Currently they are:

(defn quot [n d] (fix (/ (- n (js-mod n d)) d)))
(defn rem [n d] (- n (* d (quot n d))))

However all numbers in js are doubles already, so all this is unnecessary:

(defn quot [n d] (fix (/ n d)))
(defn rem [n d] (js-mod n d)))

Notice that "rem" is simply js-mod, and I'm not sure why no one noticed this before. I keep js-mod for now since a lot of code uses it, and if cljs ever grows a number tower the distinction may be important.

Patch attached, which also:

  • Creates a macro version of quot and rem.
  • Updates documentation for quot, rem, js-mod and mod for clarity.
  • Implement fix (private function to round to zero) with ES6 Math.trunc() if available.

Existing quot and rem tests pass, although there could be some better tests of edge cases (negative decimal num or div, NaN and +-Infinity args).



 Comments   
Comment by Francis Avila [ 24/Mar/15 12:27 PM ]

Better tests found rounding errors in my updated rem, which should stay as-is. (Not simply js-mod after all! Seems to round args first? Not obvious from the spec.) Changed quot however is correct and introduces less error than the current one. Will update patch and tests when I get a chance.

Comment by Francis Avila [ 29/Mar/15 12:39 AM ]

Working patch with tests attached. Tests expanded to cover floating-point cases. rem is now fundamentally the same as master (was more accurate than js-mod!!), but returns results consistent with js-mod for non-finite args or zero divisor.

Comment by Mike Fikes [ 31/Jan/16 3:23 PM ]

cljs-1164.patch no longer applies on master

Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch now applies. I only tested with Nashorn:

V8_HOME not set, skipping V8 tests
SPIDERMONKEY_HOME not set, skipping SpiderMonkey tests
JSC_HOME not set, skipping JavaScriptCore tests
Testing with Nashorn

...

Ran 185 tests containing 17195 assertions.
0 failures, 0 errors.
Tested with 1 out of 4 possible js targets
Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch cleaned up

Comment by Mike Fikes [ 20/Feb/16 10:11 PM ]

Successfully ran Andrea's update to Francis's patch through V8, SpiderMonkey, JavaScriptCore, and Nashorn unit tests.

I also manually ran some of the unit tests in bootstrapped ClojureScript built with the patch.

LGTM.

Comment by Mike Fikes [ 20/Feb/16 10:23 PM ]

Since this is a low-level numerics update, also ran the unit tests through ChackraCore (successfully).





[CLJS-1755] Support sourcesContent in source maps Created: 16/Aug/16  Updated: 16/Aug/16

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

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


 Description   

This issue adds sourcesContent support for source maps: https://github.com/google/closure-compiler/issues/1890. This means that your source maps can include your source as well in one bundled file. This makes handling sourcemaps much easier for things like error tracking services. It could also simplify config for source mapping as everything is included in the source map and you don't need to specify relative paths, e.t.c.

This will need to wait for the next release of the Closure Compiler.






[CLJS-1628] Make instances of js/Symbol printable Created: 20/Apr/16  Updated: 21/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Roman Scherer Assignee: Roman Scherer
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

At the moment instances of js/Symbol are not printable. Trying to
print them or enter them in the REPL causes an exception.

cljs.user> (.for js/Symbol "react.element")
#object[TypeError TypeError: Cannot convert a Symbol value to a string]

Symbols are supported in all major browsers, except Internet Explorer and Nashorn.
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol



 Comments   
Comment by Roman Scherer [ 20/Apr/16 10:23 AM ]

The attached patch adds an additional clause to `pr-writer-impl` and
implements the printing of Symbol instances, in the same way as it is
done for other JavaScript objects. Here's an example of a printed
js/Symbol:

(.for js/Symbol "react.element")
;;=> #object[Symbol "react.element"]

@david: Regarding your point about shims, do you think the
implementation of `js-symbol-defined?`, which was used for the ES6
iterator support, is enough for this patch? I'm not too familiar with
JavaScript and not sure if this already addressed the "shim" issue.

Another thing I stumbled upon is, that my test currently generates a
compiler warning when using the default compiler options. The warning
is generated when compiling the following ClojureScript form:

(.for js/Symbol "react.element")

The following snippet shows the warning and the generated code from my
test:

WARNING - Keywords and reserved words are not allowed as unquoted
property names in older versions of JavaScript. If you are targeting
newer versions of JavaScript, set the appropriate language_in option.

try{var values__13328__auto__ = (function (){var x__6628__auto__ = cljs.core.pr_str.cljs$core$IFn$_invoke$arity$variadic(cljs.core.array_seq([Symbol.for("react.element")], 0));

I think this has nothing to do with this patch, but with the emitted
code not being legal Ecmascript 3, since "for" is a reserved word.

The warning goes away when changing the :language-in option to
something newer than Ecmascript 3, or doing something like this:

((gobj/get js/Symbol "for") "react.element")

So, the questions is: Should the ClojureScript compiler handle those
reserved words when they appear in a function call or property lookup?

If that's the case I would leave the warning in that patch, and open
another issue for this new problem.

What do you think?

Roman

Comment by Roman Scherer [ 20/Apr/16 11:33 AM ]

Ok, given advanced mode and renaming of symbols I think there's not much the ClojureScript compiler can do about this.
I think I'll change the test to use ((gobj/get js/Symbol "for") "react.element") to remove the warning.
https://github.com/google/closure-compiler/wiki/FAQ#i-get-invalid-property-id-errors-but-it-works-on-firefox

Comment by David Nolen [ 20/Apr/16 2:21 PM ]

ECMAScript 3 is not an issue for people who are ok with outputting ECMAScript 5. The goog.typeOf of check on "symbol" does not seem like it would work with Symbol shims, have you confirmed?

Comment by Roman Scherer [ 21/Apr/16 10:57 AM ]

David, yes you are right about goog.typeOf, it returns "object" when
js/Symbol is implemented via shims. I updated the patch and tested it
against the following shim implementation in Nashorn so far.

Start a Nashorn REPL.

./script/nashornrepljs

js/Symbol does not exist in Nashorn yet.

(exists? js/Symbol)
;;=> false

Load core-js shim into Nashorn.

(js/load "https://raw.githubusercontent.com/zloirock/core-js/master/client/shim.min.js")
(print (.for js/Symbol "x"))
;;=> #object[Symbol "x"]

I gave up on this shim implementation because I got an error when
trying to load the minified js into Nashorn.

https://github.com/medikoo/es6-symbol

This is another shim implementation, but it only enhances js/Symbol
when it already exists, so not useful in the context of Nashorn.

https://github.com/paulmillr/es6-shim

Comment by Roman Scherer [ 21/Apr/16 2:07 PM ]

Which implementations do support js/Symbol?

JavaScriptCore, NodeJS, SpiderMonkey, Chrome and Firefox support
js/Symbol. Evaluating (instance? js/Symbol (.for js/Symbol "x")) on
those engines returns false. On all those implementations symbols
generated via (.for js/Symbol "x") are primitive values and can be
identified via typeof. See the "Using the typeof operator with
symbols" section in [1].

Nashorn and Rhino do not support js/Symbol.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 2:27 PM ]

Looking around I found that the exception caused by printing symbols
happens because the str [1] function uses implicit string conversion
to convert it's argument into a string. This is explained in the
"Coercing a symbol to string" section of [2]. One way to solve the
problem is to use (.toString x) instead.

(.join #js [(.for js/Symbol "x")] "")
;;=> TypeError: Symbol is not a constructor

(.toString (.for js/Symbol "x"))
;;=> "Symbol(x)"

[2] https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/core.cljs#L2695
[2] http://www.2ality.com/2014/12/es6-symbols.html

Comment by Roman Scherer [ 21/Apr/16 2:38 PM ]

(= (goog/typeOf x) "symbol") vs (instance? js/Symbol x)

Symbols are primitive types and must be constructed via
Symbol("x"). They can't be constructed via new Symbol("x"), trying
to do so raises a TypeError. This is explained in more detail in the
"Safety checks" section of [1]. The "Using the typeof operator with
symbols" section of [2] explains how to identify symbols. It has to be
done via typeof, because symbols are primitive types and not
instances of Symbol.

[1] http://www.2ality.com/2014/12/es6-symbols.html
[2] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 3:02 PM ]

I think we have 2 options now to fix exceptions when printing symbols:

  • Change str to handle symbols as well.
  • Implement custom printing for symbols.

I think I would go for the first option. I think calling str on a
symbol can't be seen as implicitly coercing a symbol to a string, I
would say this is as explicit as it can get and we don't have to raise
an exception in this case. I think str is basically the Clojure
equivalent to toString in JavaScript, at least this is what the out
of date doc string of str says.

The file CLJS-1628-fix-str has an implementation of the first option.

Comment by Francis Avila [ 21/Apr/16 3:35 PM ]

Changing str to use .toString is not possible because of CLJS-847: there are unconfirmable reports that .toString() causes TypeError on Safari 6.0.5.

This problem has spilled over into the unresolved CLJS-890. This is a long thread, so see my recap comment, my failed efforts to reproduce the bug and my proposed final patch, which can be easily extended for the symbol case with a new "symbol" clause in the case statement.

Comment by Roman Scherer [ 21/Apr/16 4:20 PM ]

Thanks Francis, I'll take a look at this discussion.

Comment by Roman Scherer [ 21/Apr/16 5:12 PM ]

Since one can still make so called Symbol wrapper objects via the
Object() function [1], I added one more test case for this:

(js/Object (.for js/Symbol "x"))

Also the updated patch defines the test case only if js/Symbol exists?
in the JavaScript engine.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol





[CLJS-1627] jsdoc parsing fails to recognize union types, breaking resolution Created: 18/Apr/16  Updated: 02/Sep/16

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

Type: Defect Priority: Minor
Reporter: Patrick Killean Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File CLJS-1627-4.patch     Text File CLJS-1627-5.patch    
Patch: Code and Test

 Description   

The Closure Spec For Union Types states that parentheses are necessary for union type expressions. Trying this ...

(defn foo
  "@param {(IBar|IMap)} x"
  [x] 
  ...)

Raises a Closure Error :

...ERROR - Bad type annotation. expected closing }
* @param {user.(IBar|user.IMap)}

This is because comp/resolve-types treats the parentheses as a part of the type tokens and incorrect var resolution occurs as a result. In addition, the compiler emits multiple resolved types separated by "|" characters but does not enclose them in parentheses to create a valid union type.



 Comments   
Comment by Patrick Killean [ 18/Apr/16 4:36 PM ]

This patch includes:

  • comp/resolve-types now removes parentheses when present and emits them when >1 type is detected. This makes parenthesis use optional and existing code remains unbroken (with the added benefit that it may work now)
  • changes to comp/resolve-type
    1. checks for js globals like document or window which are recognized by closure
    2. allows dot.delimited.forms to pass through so we can use types defined in externs and avoid unnecessary resolution
    3. uses ana/resolve-existing-var with a "unresolved jsdoc type" warning
    4. checks if a resolved var is a protocol and warns otherwise. This is more informative than Closure's standard unrecognized type error
  • a test for comp/resolve-types
Comment by David Nolen [ 21/Apr/16 12:45 PM ]

Thanks will try to look more closely at this tomorrow.

Comment by David Nolen [ 23/Apr/16 2:03 PM ]

The patch is getting there, please remove the `js-doc-type` meta stuff. Just extend the signature of resolve-existing-var to take an additional parameter - the confirm-var-exists handler.

Comment by Patrick Killean [ 09/May/16 4:58 PM ]

CLJS-1627-1.patch:
resolve-existing-var now has an additional arity that accepts a missing-var handler passed to confirm-existing-var

Comment by Patrick Killean [ 10/May/16 6:16 AM ]

This has revealed a problem where deftype + defrecord using Object protocols emit resolved names when really they shouldn't. For example : "@implements {cljs.core.async.impl.timers.Object}" --> Bad Type Annotation

Since Object is a special case simply excluding it from the comments should fix it. Another patch incoming

Comment by Patrick Killean [ 10/May/16 7:42 AM ]

CLJS-1627-2.patch:
The emit* methods for deftype and defrecord now filter out Object protocols.

This produced an interesting result! With no more bad type annotations, static analysis can now proceed... and it has alot to say. Theres all kinds of info now about arity discrepencies (particularly cljs.core.IndexedSeq), type mismatches, and more. It even includes a type coverage percentage. Lots to parse here but very cool.

Comment by Patrick Killean [ 18/May/16 4:26 PM ]

CLJS-1627-3.patch:

  • fix require extern
  • add type application support for Array & Object
  • GC likes uppercase for Object & Array, lowercase for string, number.
  • support for explicit nullable types, variable typed arg
  • function type context modifiers this + new

Missing is the GC 'record type' . It also may be useful to fill out the node externs for common types

Comment by Patrick Killean [ 20/May/16 11:42 AM ]

CLJS-1627-4.patch:

  • fix a few problems in last patch
  • add record type support. Everything here should be covered
Comment by Patrick Killean [ 02/Sep/16 8:21 AM ]

update patch





[CLJS-1766] Set literals in REPL end up reified as ArrayMap backed PersistentHashSets. Created: 28/Aug/16  Updated: 28/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Description   

Entering a set literal in the REPL with more than 8 elements should create a PHM backed set but instead it is array backed.

Example (in REPL):
cljs.user=> (type (.-hash-map #{1 2 3 4 5 6 7 8 9}))
cljs.core/PersistentArrayMap

This means operations such as `get` and `contains?` end up doing long scans and are slower than a user would expect.






[CLJS-1776] Add fixed arities for mapcat Created: 13/Sep/16  Updated: 13/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Robert C Faber Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS_1776__Add_fixed_arities_for_mapcat.patch     Text File CLJS-1776.patch    
Patch: Code

 Description   

Following the pattern of map, this patch adds three fixed arities for mapcat.



 Comments   
Comment by Alex Miller [ 13/Sep/16 10:25 AM ]

Presumably this is to improve performance. Please include a benchmark showing the difference.





[CLJS-1048] support function values in static vars compile time metadata Created: 20/Feb/15  Updated: 20/Feb/15

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

Type: Enhancement Priority: Minor
Reporter: Ivan Mikushin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Function values are currently only supported for :test metadata key as a special case.






[CLJS-1783] Unify List creation code Created: 20/Sep/16  Updated: 20/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

There is some duplication and redundant functions around List creation.

In this patch a fromArray method was added to List, consistent with other persistent data structures in the code base.






[CLJS-1601] Optimize cljs.core dump Created: 12/Mar/16  Updated: 01/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Nikita Beloglazov Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: bootstrap

Attachments: Text File CLJS-1601.patch     Text File CLJS-1601.patch    

 Description   

When building cljs source that uses cljs.js namespace the final js file is quite huge: 6.4M. As described in wiki: https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting it mostly consists of analysis cache of the cljs.core namespace. As a workaround, the wiki article suggests dumping cache to a separate file and load it at runtime instead of bundling in js binary. I think it is possible to have something in between that doesn't require additional efforts from a user and also optimizes the size of the js file. The idea that instead of dumping cache as raw clojure data-structure it is serialized to string. This way compiler won't compile cache into js (which adds a lot of code) and leave it a string. At runtime, this string will be parsed back to clojure using tools.reader.

Here is the proposal: https://gist.github.com/nbeloglazov/0bf163fb62fa4b61d446

Checking locally it reduces the size of js file from 6.4M to 2.7M which I think quite good. The downside is that now js has to do more work on runtime (parse huge string) when today it simply read js code and evaluates it. But I don't think if it's a big concern. If it is desired to keep all behavior a new option can be added for :dump-core compiler setting, something like :dump-core :string that enables string serialization of the cache.

Does it sound reasonable?



 Comments   
Comment by Nikita Beloglazov [ 27/Mar/16 8:54 PM ]

Attaching suggested fix. Analysis cache is serialized to string and read back to clojure datastructure when cljs.js is initialized.

Comment by David Nolen [ 28/Mar/16 6:39 AM ]

Please change the patch so this optional as you've suggested.

Comment by David Nolen [ 28/Mar/16 6:40 AM ]

Also have you submitted your Clojure CA yet?

Comment by Nikita Beloglazov [ 28/Mar/16 1:35 PM ]

Will do. Yes, I've submitted CA. I used my official name, Mikita Belahlazau there.

Comment by Nikita Beloglazov [ 29/Mar/16 12:16 AM ]

Updated patch that adds option to serialize core analysis cache as string. Possible values of :dump-core are :raw, :string, :none. Old true/false values supported for backward compatibility.

As for default, current patch uses :raw, but I think it makes more sense to use :string. Saving extra few mb of final js is quite good. I think most devs won't go deep into figuring out why js is big and just leave it as it is. Additional one-time parsing performance hit :string introduces acceptable: when :string is used, page loads in 1s while with :raw the time is ~800ms.





[CLJS-1685] Incorrectly lazy subvec when start param is nil Created: 17/Jun/16  Updated: 26/Sep/16

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

Type: Defect Priority: Minor
Reporter: Alf Kristian Støyle Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 1.9.36 on Mac and Windows


Attachments: Text File cljs-1685.patch    

 Description   

subvec in ClojureScript does not fail when start param is nil. This is different than in regular Clojure.

In Clojure:

(def foo (subvec nil 1))
CompilerException java.lang.IndexOutOfBoundsException, compiling:(form-init4645269128697935824.clj:1:10) 
(def foo (subvec nil nil))
CompilerException java.lang.NullPointerException, compiling:(form-init4645269128697935824.clj:1:10)

In ClojureScript:

(def foo (subvec nil 1))
#object[Error Error: Index out of bounds]
   cljs.core/build-subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5316:16)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$3 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5328:7)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$2 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5326:7)
   cljs$core$subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5319:1)
=> nil
(def foo (subvec nil nil))
=> #'user/foo

foo is of course not usable after this:

foo
#object[Error Error: No protocol method IIndexed.-nth defined for type null: ]
   cljs.core/missing-protocol (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:264:4)


 Comments   
Comment by Joshua Miller [ 26/Sep/16 1:37 PM ]

Added fix and test.





[CLJS-1784] Cleanup set creation functions Created: 20/Sep/16  Updated: 28/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Use .fromArray for consistency/speed when handling zeroed IndexedSeqs.

Use reduce as the default construction path to take advantage of reducible collections.



 Comments   
Comment by Rohit Aggarwal [ 26/Sep/16 4:13 PM ]

Thomas Mulvaney, could you provide some benchmarks for the speed assertion? It would be nice to run it on Chrome/Firefox/Safari.

Comment by Thomas Mulvaney [ 28/Sep/16 1:30 AM ]

Sure thing, I'll do some more benchmarks.





[CLJS-1797] Update aot_core to support build with MINGW on Windows Created: 30/Sep/16  Updated: 30/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Kidd Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 10


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

 Description   

When using Git Bash (which uses MINGW), ./script/build can get nearly all the way through it's work. However, it dies in aot_core because of the way that the classpath is generated by the Maven dependency:build-classpath plugin.



 Comments   
Comment by Thomas Kidd [ 30/Sep/16 7:40 AM ]

I was able to get this working locally, and will submit a patch when I am sure things are working

Comment by Thomas Kidd [ 30/Sep/16 8:45 AM ]

Updates aot_core to set the classpath correctly. More comments in patch.

Comment by Thomas Kidd [ 30/Sep/16 8:49 AM ]

Tested that patched version of master works on Windows 10 and OSX

Comment by Thomas Kidd [ 30/Sep/16 8:54 AM ]

Needs review

Comment by David Nolen [ 30/Sep/16 10:49 AM ]

Issues don't get closed until the issue is actually resolved





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

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

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


 Description   

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

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

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

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



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

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





[CLJS-1631] The str function should handle JavaScript symbols Created: 21/Apr/16  Updated: 21/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The str function should handle primitive JavaScript symbols produced by Symbol.for. At the moment the str function raises an exception, because it runs into some JavaScript safety checks by using implicit string coersion via (.join #js [x] "").
More info on the safety check here: http://www.2ality.com/2014/12/es6-symbols.html
This ticket is also related to:

(def x (.for js/Symbol "x"))
(str x)
TypeError: Cannot convert a Symbol value to a string
    at Array.join (native)
    at Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9561:12)
    at Object.cljs$core$str [as str] (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9543:22)
    at repl:1:100
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:54:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:228:14)
    at Socket.<anonymous> ([stdin]:40:25)

Calling the toString method on a symbol directly works

(.toString x)
;;=> "Symbol(x)"





[CLJS-1630] Add unit test for static dispatch Created: 21/Apr/16  Updated: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1630.patch    

 Description   

This unit test is an edge case that illustrates why in the code of `emit :invoke` we must stay with `call` for the high order case where static information is missing .