<< Back to previous view

[CLJS-1176] Push node.js REPL output through *out* and *err* Created: 27/Mar/15  Updated: 27/Mar/15

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

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


 Description   

The node.js REPL environment currently lets the child node process inherit the JVM's stdout/err. This is bad for nREPL, and for any other setup where one would like to rebind *out* and/or *err* in order to redirect content print ed by ClojureScript code. Turning off the inheritance and pumping output from the process to *out* and *err* should be straightforward.






[CLJS-1175] CLJS defmulti doesn't exhibit same defonce behavior as Clojure's defmulti, suggesting an even better reloading behavior Created: 27/Mar/15  Updated: 27/Mar/15

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

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


 Description   

I just noticed that the redefinition of a defmulti completely destroys the dispatch table associated with it.

This is not the case in Clojure.
http://dev.clojure.org/jira/browse/CLJ-900
https://github.com/clojure/clojure/commit/1b8d5001ba094053b24c55829994785be422cfbf

I actually think the better behavior is to create a new MultiFn with the new options and steal the dispatch table state from the current MultiFn.

I think this is the least surprising behavior.



 Comments   
Comment by David Nolen [ 27/Mar/15 1:32 PM ]

defmulti in Clojure has defonce semantics. I don't think we need to do anything at all beyond that. Patch welcome for this.





[CLJS-1174] Simple warning if a namespace with dashes not found but a file path with dashes exists Created: 27/Mar/15  Updated: 27/Mar/15

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

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





[CLJS-1173] standard way to identify types originating from ClojureScript Created: 27/Mar/15  Updated: 27/Mar/15

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

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





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

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

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

Attachments: File CLJS-742.diff    
Patch: Code

 Description   

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

Expected:
a file called foo.js

Actual:
Compiler throws an internal error

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



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

Fixes handling of compiling one IJavaScript or many

Comment by Timothy Pratley [ 08/Oct/14 11:03 PM ]

Patch is attached, updating ticket to reflect that

Comment by David Nolen [ 27/Mar/15 7:44 AM ]

Can we get a patch rebased to master? Thanks.





[CLJS-1168] REPL fails to find .js files in :libs Created: 25/Mar/15  Updated: 25/Mar/15

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

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

org.clojure/clojurescript 0.0-3149
Tested with both basic clojurescript REPL and new com.cemerick/piggieback 0.2.0-20150324.120715-1



 Description   

We use Google E2E library for encryption. This library is written using Google Closure.
The library is stored locally in a directory ("closure-libs/e2e") and we pass :libs ["closure-libs/e2e"] to the compiler.

Compilation of our project works fine. Compiler does find the library without problems in all optimization modes.

Recently we have problem with REPL. With same parameters, REPL fails to load the library:

java.lang.IllegalArgumentException: Namespace e2e.async.Result does not exist
at cljs.closure$source_for_namespace.invoke(closure.clj:501)
at cljs.repl$load_namespace.invoke(repl.clj:181)
at cljs.repl$load_dependencies.invoke(repl.clj:201)
at cljs.repl$evaluate_form.invoke(repl.clj:444)
at cljs.repl$eval_cljs.invoke(repl.clj:527)
at cljs.repl$repl_STAR_$read_eval_print__4890.invoke(repl.clj:783)
at cljs.repl$repl_STAR_$fn_4896$fn_4903.invoke(repl.clj:821)
at cljs.repl$repl_STAR_$fn__4896.invoke(repl.clj:820)
at cljs.compiler$with_core_cljs.invoke(compiler.clj:951)
at cljs.repl$repl_STAR_.invoke(repl.clj:785)
at cryptelo.dev.cljs.env$eval11953.invoke(02b7dffab3345931421313ec2fe5d506990ab5e5-init.clj:1)

I spent long time investigating the issue (which is not easy given jungle of compiler and repl option maps, bindings of global variables etc).

What I found:

  • compiler finds the library without problem, all our tests invoked from the command line pass
  • both basic cljs REPL and piggieback (0.2.0 experimental branch) fails to find the library
  • :libs files are not copied to :output-dir
  • at the beginning, repl calls default-compiler-env with options
  • default-compiler-options run js-dependency-index, which scans all libraries and stores it in compiler atom
  • strace proves that clojurescript finds closure-libs/e2e/deps.js and reads it
  • calling (js-dependency-index {:libs ["closure-lib/e2e"]}) proves that the e2e.async.Result was found

But:

  • library-dependencies stores file path as URL in {:url ...}
  • source-for-namespace (from error stack) finds the file, but expects relative path with key :file

This is where my investigation stops. I am sure that fix would be probably easy, but I do not know all details of information cljs stores about .js files. Moreover I do not know why it works in compiler and not in REPL. Probably compiler does not open file and passes
it as a argument to Google Closure, while REPL actually wants to load the file itself.

Hope you have all information to fix the problem.

Thank you.



 Comments   
Comment by David Nolen [ 25/Mar/15 8:49 AM ]

Please do not include piggieback information when reporting issues to JIRA. We do not consider downstream tooling here at all. Will take a look, thanks for the report!





[CLJS-1167]  Using (require 'namespace.core :reload) causes "too much recursion" - one process only. Created: 24/Mar/15  Updated: 24/Mar/15

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

Type: Defect Priority: Minor
Reporter: J David Eisenberg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Linux Fedora 21, openjdk version "1.8.0_31"


Attachments: File m060.tgz    

 Description   

At shell prompt, using "mies" from github: lein new mies m060
In core.cljs uncomment: (repl/connect "http://localhost:9000/repl")
In core.cljs add (defn doubled [x] (* 2 x))
At shell prompt: scripts/build # finishes successfully
At shell prompt: scripts/brepl

This will generate two warnings about goog.next.xpc.TransportNames and goog.net.xpc.CfgFields

In browser, connect to localhost:9000

In REPL: (require 'm060.core)
In REPL: (in-ns 'm060.core)
In REPL: (doubled 10) ;; this works correctly

In core.cljs, add: (defn tripled [x] (* 3 x))
in REPL: (require 'm060.core :reload)

browser shows "too much recursion" error twice (in Firefox)
in REPL: (tripled 10) ;; also works correctly

in core.cljs, add: (defn quad [x] (* 4 x))

in REPL: (require 'm060.core :reload)
Internal error: Too much recursion
Many repetitions of clojure$browser$repl$bootstrapgoog.require (out/clojure/browser/repl.cljs:158:33)


Note: this error can be avoided by replacing the (repl/connect "http://localhost:9000/repl") by
(defonce conn (repl/connect "http://localhost:9000/repl")) ;; as in quick start guide






[CLJS-1164] quot and rem are inefficient Created: 24/Mar/15  Updated: 24/Mar/15

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

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


 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.





[CLJS-1166] Browser REPL stacktraces unprocessed if explicit IP used Created: 24/Mar/15  Updated: 24/Mar/15

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

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

QuickStart Browser REPL OS X Safari (other browsers as well)



 Description   

Go through the Quick Start but change the repl/connect form to use an explicit IP instead of localhost. (In this case the IP I am trying is indeed the same host, if that matters.)

(defonce conn
  (repl/connect "http://10.96.82.207:9000/repl"))

Then connect to the REPL using the explicit IP http://10.96.82.207:9000.

The REPL will function properly but stacktraces are not properly processed. Here is an example (generated using Safari):

ClojureScript:cljs.user> (ffirst [1])
Error: 1 is not ISeqable
cljs$core$seq@http://10.96.82.207:9000/out/cljs/core.js:4667:17
cljs$core$first@http://10.96.82.207:9000/out/cljs/core.js:4697:22
cljs$core$ffirst@http://10.96.82.207:9000/out/cljs/core.js:5774:23


eval code
eval@[native code]
http://10.96.82.207:9000/out/clojure/browser/repl.js:42:271
clojure$browser$repl$evaluate_javascript@http://10.96.82.207:9000/out/clojure/browser/repl.js:45:4
http://10.96.82.207:9000/out/clojure/browser/repl.js:242:173
deliver@http://10.96.82.207:9000/out/goog/messaging/abstractchannel.js:142:21
xpcDeliver@http://10.96.82.207:9000/out/goog/net/xpc/crosspagechannel.js:733:19
messageReceived_@http://10.96.82.207:9000/out/goog/net/xpc/nativemessagingtransport.js:321:23
fireListener@http://10.96.82.207:9000/out/goog/events/events.js:741:25
handleBrowserEvent_@http://10.96.82.207:9000/out/goog/events/events.js:862:34
http://10.96.82.207:9000/out/goog/events/events.js:276:42





[CLJS-1165] Support for multiple source directories in cljs.closure/build and watch Created: 24/Mar/15  Updated: 24/Mar/15

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

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


 Description   

Quick-Start wiki page describes build and watch as functions with source (directory) as first argument

However more complex projects have multiple source paths (eg "src/cljs", "test/cljs" or cljx output "target/generated/cljs" and in future also "src/cljc").

Maybe I am missing some important point of a puzzle but I do not see a way how to (for example) build or watch multiple directories. Should we extend these function to support vectors of directories?






[CLJS-1160] Source map js / cljs line number mixup Created: 23/Mar/15  Updated: 23/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:

QuickStart Browser REPL Chrome OS X



 Description   

Note the source location for cljs.core.LazySeq.sval in the following 0.0-3126 stacktrace:

ClojureScript:cljs.user> (map ffirst (range))
Error: 0 is not ISeqable
	 cljs$core$seq (out/cljs/core.cljs:951:13)
	 cljs$core$first (out/cljs/core.cljs:960:7)
	 cljs$core$ffirst (out/cljs/core.cljs:1393:3)
	 cljs.core.map.cljs$core$map__2 (out/cljs/core.cljs:4046:30)
	 cljs.core.LazySeq.sval (out/cljs/core.cljs:11400:3)
	 cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 (out/cljs/core.cljs:2884:12)
	 cljs$core$seq (out/cljs/core.cljs:938:7)
	 cljs$core$pr_sequential_writer (out/cljs/core.cljs:8426:13)

It turns out that line 11400 is actually in core.js, but the source map logic is evidently getting tripped up by this one, and core.cljs gets reported with the JS line numbers (core.cljs doesn't even have that many lines).

Note that this also occurs on master (where CLJS-1154 and CLJS-1157 have landed):

ClojureScript:cljs.user> (map ffirst (range))
Error: 0 is not ISeqable
	 cljs.core/seq (out/cljs/core.cljs:951:20)
	 cljs.core/first (out/cljs/core.cljs:960:16)
	 cljs$core$ffirst (out/cljs/core.cljs:1393:11)
	 cljs.core.map.cljs$core$map__2 (out/cljs/core.cljs:4046:30)
	 cljs.core.LazySeq.sval (out/cljs/core.cljs:11400:3)
	 cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 (out/cljs/core.cljs:2884:12)
	 cljs.core/seq (out/cljs/core.cljs:938:25)
	 cljs$core$pr_sequential_writer (out/cljs/core.cljs:8426:20)





[CLJS-1158] Regression: compiler fails to see symbols defined in another namespace Created: 23/Mar/15  Updated: 23/Mar/15

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

Type: Defect Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Zip Archive cljs-1158.zip    

 Description   

Regression happened between 0.0-2985 and 0.0-3030.

E.g. I have this setup:

(ns datascript
  (:require [datascript.core :as dc]))

(def filter dc/filter)
(ns datascript.js
  (:require
    [datascript :as d]))

d/filter

This is the output of compiler for 0.0-2985:

Compiling ClojureScript.
Compiling "target/datascript.js" from ["src" "test"]...
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-2985/clojurescript-0.0-2985.jar!/cljs/core.cljs
Compiling src/datascript/btset.cljs
Compiling src/datascript/core.cljs
Compiling src/datascript/debug.cljs
Compiling src/datascript/impl/entity.cljs
Compiling src/datascript/js.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript.cljs
WARNING: datascript is a single segment namespace at line 1 /Users/prokopov/Dropbox/ws/datascript/src/datascript.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript/query.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript/parser.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-2985/clojurescript-0.0-2985.jar!/clojure/set.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript/pull_parser.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript/pull_api.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-2985/clojurescript-0.0-2985.jar!/cljs/reader.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-2985/clojurescript-0.0-2985.jar!/clojure/walk.cljs
Compiling src/datascript/parser.cljs
...

Note that

Compiling src/datascript/js.cljs
triggers
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript.cljs

And on 0.0-3030 and later versions I get this:

Compiling ClojureScript.
Retrieving org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.pom from central
Retrieving org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar from central
Compiling "target/datascript.js" from ["src" "test"]...
Reading analysis cache for jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar!/cljs/core.cljs
Compiling src/datascript/btset.cljs
Compiling src/datascript/core.cljs
Compiling src/datascript/debug.cljs
Compiling src/datascript/impl/entity.cljs
Compiling src/datascript/js.cljs
WARNING: Use of undeclared Var datascript/filter at line 11 src/datascript/js.cljs
Compiling src/datascript/parser.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar!/clojure/set.cljs
Compiling src/datascript/pull_api.cljs
Analyzing file:/Users/prokopov/Dropbox/ws/datascript/src/datascript/pull_parser.cljs
Compiling src/datascript/pull_parser.cljs
Compiling src/datascript/query.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar!/cljs/reader.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar!/clojure/walk.cljs
Compiling src/datascript/query_v3.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/com/cemerick/clojurescript.test/0.3.3/clojurescript.test-0.3.3.jar!/cemerick/cljs/test.cljs
Analyzing jar:file:/Users/prokopov/.m2/repository/org/clojure/clojurescript/0.0-3030/clojurescript-0.0-3030.jar!/clojure/string.cljs
Compiling src/datascript.cljs
WARNING: datascript is a single segment namespace at line 1 src/datascript.cljs
...

The problem is in this line:

WARNING: Use of undeclared Var datascript/filter at line 11 src/datascript/js.cljs

I have fully legit use of datascript/filter in datascript.js, and yet get this warning.

To reproduce, just lein cljsbuild once advanced on master of https://github.com/tonsky/datascript with 0.0-3030 or later



 Comments   
Comment by David Nolen [ 23/Mar/15 5:45 AM ]

In general we only consider minimal cases not complete projects as there are too many variables at play. We no longer consider anything involving cljsbuild. Please attempt to produce a minimal case first. Given the above you should be able produce something quite small and simple using the Quick Start and the standalone JAR. Thanks!

Comment by David Nolen [ 23/Mar/15 6:09 AM ]

I just tried the standalone 0.0-3126 JAR with the following:

src/alias_bug/core.cljs

(ns alias-bug.core)

(defn foo [a b]
  (+ a b))

src/alias_bug/js.cljs

(ns alias-bug.js
  (:require [alias-bug :as alias]))

alias/foo

src/alias_bug.cljs

(ns alias-bug
  (:require [alias-bug.core :as core]))

(def foo core/foo)

build.clj

(require '[cljs.closure :as cljsc])

(cljsc/build "src"
  {:output-to "out/main.js"
   :verbose true})

I compiled this with:

java -cp cljs.jar:src clojure.main build.clj

I did not see this issue. I tried touching one file then running build again one at a time, still did not see this issue. The implicit options here are :optimizations :none, :cache-analysis true.

Comment by Nikita Prokopov [ 23/Mar/15 6:37 AM ]

Sorry, for some reason I decided that’s something trivial that was just overlooked and is easy to spot without a minimal test case. Now I see it’s more deep/complex than that. Attaching a minimal use case. Looks like it has something to do with :require-macros

Comment by Nikita Prokopov [ 23/Mar/15 6:58 AM ]

Just for information: I also noted that compilation order is alphabetical, and naming files plays a role here: when it compiles a, then b, then c, bug reproduces, if we rename files to change that order, it vanishes.





[CLJS-1159] compiled files with warnings that otherwise don't need recompilation will not emit warnings on the next compile Created: 23/Mar/15  Updated: 23/Mar/15

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

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


 Description   

The aggressive caching approach is odds with warning visibility. It probably makes sense for a compiled file with warnings to always return true for requires-compilation?.






[CLJS-994] print a warning when :externs file paths can't be found. Created: 30/Jan/15  Updated: 23/Mar/15

Status: Reopened
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3058
Fix Version/s: Next

Type: Enhancement Priority: Major
Reporter: Crispin Wellington Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: cljs, enhancement, errormsgs, patch,
Environment:

Linux 64bit

java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)


Attachments: Text File clojurescript-extern-missing-warning.patch    
Patch: Code

 Description   

clojurescript silently ignores missing externs files possibly leading a developer to chase their tail.

Presently it can be very confusing using advanced compilation if you have made a mistake in the path name of one of your :externs files. This patch makes the compiler print a warning on stderr so you can quickly determine the cause of the broken advanced compilation output.

As a side effect, when doing a basic lein-cljsbuild a warning is always printed:

```
WARNING: js resource path closure-js/externs does not exist
```

This is because lein-cljsbuild quietly adds this extra path to your :externs listing without you knowing.



 Comments   
Comment by David Nolen [ 31/Jan/15 1:59 PM ]

You need to bind *out* to *err*, or just print to it directly a la cljs.util/debug-prn.

Comment by Crispin Wellington [ 31/Jan/15 7:30 PM ]

I did bind out to err. Check the patch.

Comment by David Nolen [ 01/Feb/15 12:30 PM ]

Crispin, oops sorry you are correct. Thanks.

Comment by David Nolen [ 13/Mar/15 7:33 AM ]

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

Comment by David Nolen [ 14/Mar/15 5:55 AM ]

The solution does not work for cljsbuild. It's unclear why there so much machinery in place over the approach taken for deps.clj.

Comment by David Nolen [ 15/Mar/15 10:37 AM ]

Stalled on this cljsbuild issue https://github.com/emezeske/lein-cljsbuild/issues/383

Comment by Crispin Wellington [ 23/Mar/15 2:50 AM ]

This lein-cljsbuild issue is what made me make it just a warning initially, and not a hard error like raising IllegalArgumentException does. Though I agree it should be a hard error. If we start with a warning, it enables the immediate problem for the developer to be resolved, and leaves a wart that the cljs-build project can then see that need fixing on their end. Then when that end is fixed it could be made a hard error. If cljsbuild is fixed fairly soon then all is well, but if it takes a long time, a warning might be a good first step.





[CLJS-1136] Initial require fails to fully load added symbols Created: 17/Mar/15  Updated: 22/Mar/15

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

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

Quick Start Browser REPL (OS X / Safari)



 Description   

In the Quick Start, a portion runs the user through adding a symbol (a function named foo) and then requiring the namespace and using that symbol. I'm finding that require fails and that I need to add the :reload directive.

To reproduce:

  1. Run through the Quick Start up through the browser REPL section.
  2. Set src/hello_world/core.cljs so that it does not have the foo function defined.
  3. Remove the out directory: rm -rf out
  4. Start up the REPL: rlwrap java -cp cljs.jar:src clojure.main repl.clj
  5. Connect Safari by going to http://localhost:9000
  6. Show the error console in Safari. (You'll see Hello world.)
  7. Run tail -f out/watch.log
  8. Add the foo function that adds a b to src/hello_world/core.cljs and save it.
  9. Observe that watch.log reflects recompilation
  10. Do {{ (require '[hello-world.core :as hello]) }}
  11. Do {{ (hello/foo 2 3) }}

At this point you will get:
TypeError: undefined is not an object (evaluating 'hello_world.core.foo.call')

But:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{foo #'hello-world.core/foo, conn #'hello-world.core/conn}
ClojureScript:cljs.user> (source hello/foo)
(defn foo [a b]
  (+ a b))
nil

Now, if you :reload

ClojureScript:cljs.user> (require '[hello-world.core :as hello] :reload)
nil
ClojureScript:cljs.user> (hello/foo 2 3)
5


 Comments   
Comment by Mike Fikes [ 17/Mar/15 11:30 AM ]

Prior to step 8:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{}

Between steps 9 and 10:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{foo #'hello-world.core/foo, conn #'hello-world.core/conn}

My guess: Watching is causing symbols to be interned, but not usable, and this is interfering with require forcing you to include :reload.

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

I'm not sure that this is actually an issue, the browser has already required the namespace, it's the entry point. Thus you really do need a :reload. But the reason you see interned symbols is that the watch process shares the compilation environment with the REPL. It may be the case that with the dramatically improved REPLs the watch option becomes entirely unnecessary and counterintuitive, let's see how it goes.





[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-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 21/Mar/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3126
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: 21/Mar/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3123
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    

 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.





[CLJS-1153] Typed Array backed PersistentVector based on clojure.core/Vec Created: 19/Mar/15  Updated: 19/Mar/15

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

Type: Enhancement Priority: Major
Reporter: Adrian Medina Assignee: Unassigned
Resolution: Unresolved Votes: 0
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?




[CLJS-1129] :modules tutorial for wiki Created: 16/Mar/15  Updated: 19/Mar/15

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation, newbie


 Description   

The documentation is nice but something that walks people through the steps would be nicer.






[CLJS-1128] Describe internationalization strategies via Google Closure on the wiki Created: 16/Mar/15  Updated: 19/Mar/15

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation, newbie


 Description   

This can be done via Google Closure defines or via pulling a specific locale. A page should document how this can be done.






[CLJS-1138] Add page to Wiki "Slow dev builds" Created: 17/Mar/15  Updated: 19/Mar/15

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

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


 Description   

A common mistake with cljsbuild is to include test directories in the source paths. Due to :recompile-dependents defaulting to true, tests will get unnecessarily recompiled.






[CLJS-1145] numeric type inference should accept JVM primitive numeric type hints Created: 18/Mar/15  Updated: 18/Mar/15

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

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





[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 18/Mar/15

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

Types defined with deftype/defrecord which have a default field will incorrectly return nil with property access. The following example will return nil.

(deftype Foo [default])

(let [foo (Foo. "bar")]
  (.-default foo))


 Comments   
Comment by Joel Holdbrooks [ 13/Oct/14 4:19 PM ]

Patch attached. I should point out that I had to borrow js-reserved from the compiler namespace and the warning message provided hard codes the munged symbol information instead of reusing the compiler's munge fn.

Comment by Joel Holdbrooks [ 13/Oct/14 9:41 PM ]

For the sake of history, I should provide more context to this patch (I'm unable to edit the issue title for some reason). It isn't just .-default it is any field name that is also a JavaScript identifier (eg. public, private, if).

Comment by David Nolen [ 14/Oct/14 5:26 AM ]

Please lift js-reserved and any helpers like munge into the shared namespace cljs.util so that logic an be shared and hard coding avoided. Thanks.

Comment by Joel Holdbrooks [ 14/Oct/14 5:03 PM ]

Are you sure, David? That might make this patch a bit more noisy. If it's not a problem I'm happy to do it.

Comment by David Nolen [ 14/Oct/14 6:06 PM ]

I'm sure, I'd like to avoid this kind of code duping. Cleaner in the end and better moving forward.

Comment by Joel Holdbrooks [ 18/Mar/15 11:43 AM ]

Updated to use new refactorings

Comment by David Nolen [ 18/Mar/15 11:46 AM ]

The warning is not desirable. Instead we should just munge and ensure property access always works.





[CLJS-1124] Have cljs.repl.browser/repl-env serve more static file types Created: 16/Mar/15  Updated: 18/Mar/15

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

Type: Enhancement Priority: Minor
Reporter: Sander Dijkhuis Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie
Environment:

cljs 0.0-3115


Attachments: Text File CLJS-1124-attempt001.patch     GZip Archive CLJS-1124-test.tar.gz     GZip Archive CLJS-1124-test.tar.gz    

 Description   

I'm developing a webpage using CLJS that can be served statically. To iterate quickly on the script, I'm hosting the page using

(cljs.repl/repl
(cljs.repl.browser/repl-env :static-dir ["." "out/" "img/" "fonts/"])
:watch "src")

I expect images and webfonts from the img/ and fonts/ directories to be served as well, but the server dispatch code only seems to deal with some other file types:

https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/repl/browser.clj#L104

(server/dispatch-on :get
(fn [{:keys [path]} _ _]
(or
(= path "/")
(.endsWith path ".js")
(.endsWith path ".cljs")
(.endsWith path ".map")
(.endsWith path ".html")
(.endsWith path ".css")))
send-static)

Could this be extended to support all types of files in the static directories? Or could more cases be added for .jpg, .woff and .png?

My workaround is to load index.html from a separate static server, and use the REPL server only for the REPL connection. But in this setup, somehow (require my-ns.core :reload) keeps reloading the old code. This problem doesn't occur when I load index.html from the REPL server.



 Comments   
Comment by Sander Dijkhuis [ 16/Mar/15 8:04 AM ]

Here’s a quick first attempt. It tries to serve all static files that can be found. I don’t see downsides to this approach yet.

Now images get downloaded but don’t render. I guess this is because the served files’ Content-Type is wrong: e.g. image/jpeg; charset=utf-8. If I’m not mistaken no charset should be specified, and this requires some more checking in cljs.repl.server/end-and-close.

Is it OK to stretch the experimental server’s use to support binary files, or should I look for a more specialised development server?

Comment by Sander Dijkhuis [ 18/Mar/15 3:35 AM ]

I’m giving up on my patch for now. It’s not enough to remove the charset from the Content-Type header. I guess cljs.repl.browser/send-static needs to be adjusted to deal with binary streams instead of slurping binary files into strings. It should also have a more extensive list of MIME types. But then I’m not sure if cljs wants to be a complete experimentation server.

In the attachment is a minimal test case:
1. put a cljs.jar inside the extracted directory
2. run make
3. open http://localhost:9000/

Comment by Sander Dijkhuis [ 18/Mar/15 3:47 AM ]

(Slightly friendlier version of the test case.)





[CLJS-1142] ClojureScript stream socket REPL Created: 18/Mar/15  Updated: 18/Mar/15

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

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


 Description   

Port relevant bits of CLJ-1671






[CLJS-1139] Repeated applications of `ns` form at the REPL are not additive Created: 17/Mar/15  Updated: 17/Mar/15

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

Type: Defect Priority: Major
Reporter: Michael Griffiths Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Quick start guide with Node REPL



 Description   

In a Clojure REPL, it is possible to declare the same namespace again, without existing namespaces aliases being altered or removed:

user=> (ns my-test-ns.core (:require [clojure.string :as string]))
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=> (ns my-test-ns.core)
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=>

ClojureScript REPLs do not behave in the same way:

ClojureScript:cljs.user> (ns my-test-ns.core (:require [clojure.string :as string]))
true
ClojureScript:my-test-ns.core> (def a string/blank?)
#<function clojure$string$blank_QMARK_(s){
return goog.string.isEmptySafe(s);
}>
ClojureScript:my-test-ns.core> (ns my-test-ns.core)
nil
ClojureScript:my-test-ns.core> (def a string/blank?)
WARNING: No such namespace: string, could not locate string.cljs at line 1 <cljs repl>
WARNING: Use of undeclared Var string/blank? at line 1 <cljs repl>
repl:13
throw e__3919__auto__;
      ^
ReferenceError: string is not defined
    at repl:1:109
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:197:16)
    at Socket.<anonymous> ([stdin]:40:25)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)
ClojureScript:my-test-ns.core>





[CLJS-1133] REPL require results in warnings to be emitted twice Created: 17/Mar/15  Updated: 17/Mar/15

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

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

Quick Start Browser REPL with :watch off



 Description   

Run through the Quick Start and go down through to the Browser REPL portion (https://github.com/clojure/clojurescript/wiki/Quick-Start#browser-repl), but exclude the :watch option from repl.clj.

Then further down, where the new symbol is introduced

;; ADDED
(defn foo [a b]
  (+ a b))

instead cause some duplicate symbols to be introduced in order to provoke compiler warnings:

(def a 1)
(def a 1)

(defn foo [a b]
  (+ a b))
(defn foo [a b]
  (+ a b))

Then evaluate the require statement in the tutorial and observe that the warnings are emitted twice:

ClojureScript:cljs.user> (require '[hello-world.core :as hello])
WARNING: a at line 11 is being replaced at line 12 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: foo at line 14 is being replaced at line 16 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: a at line 11 is being replaced at line 12 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: foo at line 14 is being replaced at line 16 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
nil





[CLJS-1135] Present more cljs.foo.api interfaces for stable parts of cljs.analyzer, cljs.compiler, and cljs.closure Created: 17/Mar/15  Updated: 17/Mar/15

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

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





[CLJS-1134] Lift protocols from cljs.closure into cljs.protocols ns Created: 17/Mar/15  Updated: 17/Mar/15

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

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


 Description   

This is task towards presenting a stable API to users without reaching into the implementation namespaces.






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

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

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

r2173



 Description   

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

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



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

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

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

Bump, this enhancements sound simple & fine.

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

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

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

Comment by Francis Avila [ 16/Mar/15 11:14 AM ]

I completely forgot about this, sorry. I see you have scheduled it for the "next" release. Are you assigning it as well or will you still accept a patch?

Comment by David Nolen [ 16/Mar/15 11:26 AM ]

Be my guest





[CLJS-1113] Compile in dependency order Created: 12/Mar/15  Updated: 16/Mar/15

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

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


 Description   

Because we do not compile in dependency order we do a lot of needless analysis.



 Comments   
Comment by David Nolen [ 14/Mar/15 6:13 AM ]

With the latest changes to master this will only improve the performance of the first compile.





[CLJS-1127] validate compiled file written to disk Created: 16/Mar/15  Updated: 16/Mar/15

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

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


 Description   

If we validate the file written to disk then we can catch common error of running multiple build processes and abort.






[CLJS-1125] Simple corrupted compiled file detection Created: 16/Mar/15  Updated: 16/Mar/15

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

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


 Description   

We should include a line at the end of the file that we check for to determine that the file was not corrupted due to either an incomplete write or a clobbered write. It should be be the SHA of the ClojureScript source it was generated from.






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

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

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

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

 Description   

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

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

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






[CLJS-1070] top-level boolean inference does not work Created: 28/Feb/15  Updated: 16/Mar/15

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

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


 Description   

Problem for using boolean Closure defines






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

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

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


 Description   

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

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

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

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

Thanks



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

A patch is welcome for this. Thanks.





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

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

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





[CLJS-688] Unhelpful error message when compiling with blank .cljs file Created: 20/Nov/13  Updated: 16/Mar/15

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

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


 Description   

When compiling, if you have a blank .cljs file in your source path, you get the following unhelpful message:

java.lang.NullPointerException: 
          closure.clj:370 cljs.closure/compiled-file
            core.clj:2485 clojure.core/map[fn]
          LazySeq.java:42 clojure.lang.LazySeq.sval
          LazySeq.java:67 clojure.lang.LazySeq.seq
              RT.java:484 clojure.lang.RT.seq
             core.clj:133 clojure.core/seq
             core.clj:678 clojure.core/concat[fn]
          LazySeq.java:42 clojure.lang.LazySeq.sval
          LazySeq.java:60 clojure.lang.LazySeq.seq
             Cons.java:39 clojure.lang.Cons.next
             RT.java:1654 clojure.lang.RT.boundedLength
          RestFn.java:130 clojure.lang.RestFn.applyTo
             core.clj:619 clojure.core/apply
         closure.clj:1018 cljs.closure/build
          closure.clj:981 cljs.closure/build
          compiler.clj:58 cljsbuild.compiler/compile-cljs[fn]
          compiler.clj:57 cljsbuild.compiler/compile-cljs
         compiler.clj:158 cljsbuild.compiler/run-compiler
form-init6897480153702409709.clj:1 user/eval2998[fn]
form-init6897480153702409709.clj:1 user/eval2998[fn]
          LazySeq.java:42 clojure.lang.LazySeq.sval
          LazySeq.java:60 clojure.lang.LazySeq.seq
              RT.java:484 clojure.lang.RT.seq
             core.clj:133 clojure.core/seq
            core.clj:2780 clojure.core/dorun
            core.clj:2796 clojure.core/doall
form-init6897480153702409709.clj:1 user/eval2998
       Compiler.java:6619 clojure.lang.Compiler.eval
       Compiler.java:6609 clojure.lang.Compiler.eval
       Compiler.java:7064 clojure.lang.Compiler.load
       Compiler.java:7020 clojure.lang.Compiler.loadFile
             main.clj:294 clojure.main/load-script
             main.clj:299 clojure.main/init-opt
             main.clj:327 clojure.main/initialize
             main.clj:362 clojure.main/null-opt
             main.clj:440 clojure.main/main
          RestFn.java:421 clojure.lang.RestFn.invoke
             Var.java:419 clojure.lang.Var.invoke
             AFn.java:163 clojure.lang.AFn.applyToHelper
             Var.java:532 clojure.lang.Var.applyTo
             main.java:37 clojure.main.main


 Comments   
Comment by Travis Thieman [ 06/Dec/13 11:30 AM ]

I tried this using both cljsbuild (from a mies template) and bin/cljsc on a directory. Both of these methods give the following error message, which seems reasonable:

java.lang.AssertionError: Assert failed: file:/repos/empty/src/empty/empty.cljs does not provide a namespace

How are you compiling to get what you're seeing?





[CLJS-620] Warnings are generated when using a macro in argument position Created: 14/Oct/13  Updated: 16/Mar/15

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

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

Attachments: File CLJS-620.diff    

 Description   

Using a macro in argument position (e.g. (map macro [])) generates a warning:
WARNING: Use of undeclared Var test/node at line 4 src/test.cljs

Find a reproduction project here.



 Comments   
Comment by Jozef Wagner [ 15/Oct/13 3:30 AM ]

and what would you like, a better warning? Clojurescript allows same name for macro and for function, so you can both have macro + and fn +. Macro version will be used when is first in the list, fn version otherwise.

Comment by Jonas Enlund [ 15/Oct/13 3:38 AM ]

For reference, Clojure generates the following error message:

user=> (map when [])
CompilerException java.lang.RuntimeException: Can't take value of a macro: #'clojure.core/when, compiling:(NO_SOURCE_PATH:1:1)

The "obvious" approach would be to add

(when-let [m (get-expander sym env)]
  (throw (error env (str "Can’t take value of a macro: " m))))

to resolve-var[1]. Unfortunately this doesn’t work in ClojureScript due to the way inlining works. A simple workaround is to add {:inline true} metadata to macros that are later redefined as functions in core.cljs and check for invalid macro usage like this:

(when-let [m (get-expander sym env)]
  (and (-> m meta :inline not)    
       (throw (error env (str "Can’t take value of a macro: " m)))))

Another approach would perhaps be to rethink how inlining works as it seems kind of brittle to have macros in cljs/core.clj with the same name as functions in cljs/core.cljs (especially since both namespaces are auto-imported. Is cljs.core/inc a function, a macro, or both?). Maybe there’s a better way?

[1] https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L193

Comment by Julien Eluard [ 15/Oct/13 6:23 AM ]

My bad, didn't realize it didn't make sense. Of course it's obvious now. I was confused by recent changes around function/macro name validation.
Now the warning could probably be improved a little but that doesn't seem very important. The Clojure warning is not that much better.

Comment by David Nolen [ 15/Oct/13 11:58 AM ]

We're not going to change how inlining works - the whole point is that we can reuse the same names. Adding :inline metadata seems like a promising way to warn of incorrect usage of inlining macros.





[CLJS-485] clojure.string/replace ignores regex flags Created: 12/Mar/13  Updated: 16/Mar/15

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

Type: Defect Priority: Minor
Reporter: Esa Virtanen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, patch, test

Attachments: Text File 0001-Take-regex-flags-m-i-into-account-in-clojure.string-.patch    
Patch: Code and Test

 Description   

The replace function in namespace clojure.string ignores regex flag provided in the match pattern. For example:

CLJS
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am NOT matched"
CLJ
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am matched"

The attached patch fixes this by parsing the m and i flags, if set, from the match object, instead of explicitly setting only "g".



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

I can confirm the bug. The attached patch applies cleanly, and works as expected.

Esa, sorry for the long delay (this one must have slipped through the cracks)! Could you please submit a contributor's agreement, so that your patch can be merged? More info is here:

http://clojure.org/contributing





[CLJS-434] ClojureScript compiler prepends "self__" to defmulti forms when metadata in form of ^:field. Created: 01/Dec/12  Updated: 16/Mar/15

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

Type: Defect Priority: Minor
Reporter: Andrew Mcveigh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Mac OS X (10.7), java version "1.6.0_37", leiningen 2 preview 10, cljsbuild 0.2.9.
clojure/clojurescript master 01 December 2012 - 5ac1503



 Description   

Using the def form, with the specific metadata ^:field causes the cljs compiler
to prepend "self__" to the output js form.

The browser (latest chrome/firefox) does not recognize "self__".

Test Case: Tested against master: 5ac1503
-------------

(ns test-def)

(def ^:foo e identity)
e
; test_def.e = cljs.core.identity;
; test_def.e;

(def ^:field f identity)
f
; test_def.f = cljs.core.identity;
; self__.test_def.f;
; Uncaught ReferenceError: self__ is not defined

https://gist.github.com/4185793



 Comments   
Comment by Brandon Bloom [ 01/Dec/12 5:37 PM ]

code tags

Comment by David Nolen [ 20/Jan/13 12:54 AM ]

This one is a bit annoying. We should probably use namespaced keywords internally.





[CLJS-428] Using */ inside of a docstring causes compiler to produce invalid JavaScript Created: 25/Nov/12  Updated: 16/Mar/15

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

Type: Defect Priority: Minor
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Linux, lein-cljsbuild



 Description   

Due to how function docstrings are output by the ClojureScript compiler, the use of

*/
within a docstring causes the compiler to produce invalid JavaScript, breaking compilation, since '*/' will close the docstring's JavaScript comment block and the remaining docstring text will be uncommented as a result.



 Comments   
Comment by Murphy McMahon [ 25/Nov/12 12:32 PM ]

I didn't realize JIRA treats asterisks as markup, so just for clarification: the characters that produce the defect are slash asterisk, ie JavaScript's block comment closing syntax.

Comment by David Nolen [ 22/Dec/12 3:30 PM ]

Do you have a suggested fix for this?





[CLJS-375] loop doesn't seem to preserve tag information as evidenced by extra cljs.core.truth_ calls Created: 06/Sep/12  Updated: 16/Mar/15

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

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





[CLJS-349] cljs.compiler: No defmethod for emit-constant clojure.lang.LazySeq Created: 30/Jul/12  Updated: 16/Mar/15

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

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

Attachments: Text File 0001-CLJS-349-Allow-ISeq-to-be-emitted-by-macros-as-a-con.patch     File fixbug349.diff    

 Description   

The cljs compiler errors when trying to emit-constant for a clojure.lang.LazySeq.

Example : https://www.refheap.com/paste/3901

Here syms is defined as a LazySeq on line 3, then on line 7 it is quoted. The error is included in the refheap.

Emitting a cljs.core.list for this type seems to solve the issue.



 Comments   
Comment by David Nolen [ 31/Aug/12 9:27 AM ]

Can you identify precisely where a LazySeq is getting emitted here? A LazySeq is not literal so this seems like a bug in the macro to me. I could be wrong. Thanks!

Comment by Herwig Hochleitner [ 28/Oct/12 9:31 PM ]

The lazy seq seems to be introduced on line 7, the '~syms form

`(let [mappings# (into {} (map-indexed #(identity [%2 %1]) '~syms))

Clojure allows lazy-seqs to be embedded: https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4538

As an aside: The relevant protocol is not literality, but the print-dup multimethod. Do / Should we have print-dup in CLJS?

Comment by Herwig Hochleitner [ 31/Oct/12 10:10 PM ]

Attached patch 0001 doesn't add a case for LazySeq, but folds two cases for PersistentList and Cons into one for ISeq.

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

This approach seems acceptable but this is an old patch can we update for master?





[CLJS-710] port clojure.pprint Created: 02/Dec/13  Updated: 16/Mar/15

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

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


 Comments   
Comment by Shaun LeBron [ 15/Aug/14 1:29 PM ]

working on this here: https://github.com/shaunlebron/cljs-pprint

Comment by Shaun LeBron [ 05/Oct/14 12:40 PM ]

I'm ceasing development on the port.

fipp should be ported in place of pprint in cljs. pprint is slow and complex, whereas fipp is fast, simple, and very customizable.

Brandon Bloom is awaiting a go-ahead from the cljs community on whether the fipp port should be completed:
https://github.com/brandonbloom/fipp/issues/7

Comment by David Nolen [ 05/Oct/14 12:55 PM ]

fipp only covers a very small (though important part) of clojure.pprint's functionality. I think it's fine that fipp is a standalone library that user's can adopt in their own projects if they desire. This doesn't change the desire for a full port of clojure.pprint.

Comment by Shaun LeBron [ 27/Oct/14 1:13 PM ]

k, resuming.





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

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3115
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-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 16/Mar/15

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

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


 Description   

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

Example:

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

Whereas foo.getBarRight expands to something like

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

foo.getBarWrong on the other hand expands to

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





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

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

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

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


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

 Description   

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



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

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

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

Patch welcome for this.

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJS-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: Joshua 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-836] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 16/Mar/15

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

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

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

 Description   

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



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

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

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

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





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

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

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


 Description   

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






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

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

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: Unassigned
Resolution: Unresolved Votes: 1
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".





[CLJS-1061] Enhanced control over printing configuration Created: 23/Feb/15  Updated: 15/Mar/15

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

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


 Description   

enable-console-print! is convenient from some developers on a team who like println to print to the console. However, for others, like myself, it is more desirable to print directly to the REPL (e.g. w/in Emacs) instead of the browser console. On occasion enable-console-print! get's checked and can cause errors that result from calling println before the *print-fn* is set. It then becomes a hassle to either comment out the println or the enable-console-print!. Having a disable-console-print! which could restore the previous *print-fn*, whatever that may have been, would be nice to have.

As it currently stands, one must do this manually, which frankly is kind of awkward and not immediately obvious. It is much like having a light you can turn on but not off unless you cut the main power.



 Comments   
Comment by Thomas Heller [ 24/Feb/15 6:01 AM ]

I think the whole print-fn handling is not optimal. The problem is that each namespace can set it and will override the previous setting and the last one wins. While this is most likely one of "my" namespaces and "my" print-fn it may be someone else's while initializing (loading dependant namespaces). So you might run into issues where some log output goes to console and some goes to util.print or whichever other method the library author decided to use. Some libraries already call (enable-console-print!) as the very first thing their namespace.

What I recently added to shadow-build is an option to make this a compiler option (eg. {:print-fn :console}). Code can just use prn, println and never worry about that *print-fn* might not be set. I think this is a very convenient option that can default to :console and one less hurdle new users may stumble over. I don't need to initialize println in Clojure either.

Maybe this option should make it into core.

Comment by David Nolen [ 12/Mar/15 10:03 AM ]

Not going to do disable-console-print!. This ticket is important but it needs further consideration.

Comment by Joel Holdbrooks [ 12/Mar/15 5:39 PM ]

I'm okay with that. I'm just interested in the solution. Thanks for considering it.

Comment by David Nolen [ 15/Mar/15 10:26 AM ]

Putting this in the Backlog for now. It needs more hammock time. More solution ideas welcome on this ticket.





[CLJS-1121] QuickStart a little unclear for browser REPL Created: 13/Mar/15  Updated: 14/Mar/15

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

Type: Defect Priority: Minor
Reporter: Brent Vukmer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Yosemite (10.10.2), Google Chrome


Attachments: File core.cljs     File repl.clj    

 Description   

I'm at the point in the QuickStart where it says "Point your web browser at http://localhost:9000. You should get a REPL."

I don't see the browser REPL when I go to http://localhost:9000, just a blank page. I'm attaching my repl.clj and core.cljs.



 Comments   
Comment by Brent Vukmer [ 14/Mar/15 9:08 AM ]

If "We also need to modify our script to load the browser REPL" refers to core.cljs, it would be clearer to say so.





[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-324] cljs.core/format Created: 24/Jun/12  Updated: 11/Mar/15

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

Type: Enhancement Priority: Trivial
Reporter: Gary Fredericks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-CLJS-324-implement-cljs.core-format-as-wrapper-for-g.patch     File goog.string.format-0.0-1913.tgz    

 Description   

Implement cljs.core/format. Make sure there is nothing in gclosure for this. I suppose we should try to emulate the jvm version as much as that makes sense?



 Comments   
Comment by Michał Marczyk [ 28/Jun/12 11:29 AM ]

GClosure does actually include a string formatter – goog.string.format. Note that that's both a non-ctor function name and the GClosure "namespace" name, so in order to use it one must require goog.string.format on top of gstring and say gstring/format (or perhaps leave out the gstring require spec and use goog.string/format – not tested, but should work).

The patch to introduce format and printf as wrappers for goog.string.format is naturally extremely simple, so here it is. Note that this particular patch must be applied on top of CLJS-328.

I have no idea how goog.string.format compares to the JVM's String/format (basic number formatting seems to work as it should in sprintf-like functions, but other than that I haven't tested it much).

Comment by David Nolen [ 29/Jun/12 10:44 AM ]

The tests fail for me with this patch applied.

Comment by David Nolen [ 29/Jun/12 11:07 AM ]

fixed, http://github.com/clojure/clojurescript/commit/8f518760a3df8b351208e97bb70270856623bb0a

Comment by David Nolen [ 11/Sep/13 5:05 PM ]

Backing this one out, goog.string.format defies advanced optimization and it provides few of the capabilities of Clojure's format - which does a lot because of java.util.Formatter. Apologies for the churn, but this is a simple thing for people to stub in themselves for the little bit of functionality it actually delivers.

Comment by Lars Bohl [ 12/Oct/13 6:33 AM ]

Uploading a slighly modified version lein-cljsbuild's "advanced" demo, to demonstrate that using goog.string.format produces a runtime error with clojurescript 0.0.1913. Run "lein ring server" and navigate to http://localhost:3000/

The code in hello.cljs shows that goog.string.toTitleCase works, but not goog.string.format.

Comment by Julien Eluard [ 12/Oct/13 6:43 AM ]

It looks like you are not requiring correctly format. See a working example here.

Comment by Lars Bohl [ 12/Oct/13 6:58 AM ]

Julent, thanks! It needs another [goog.string.format] after [goog.string :as gstring] before you can use gstring/format. hello.cljs now looks like this, and throws no exceptions: https://www.refheap.com/19693





[CLJS-1104] Compute SHA for ClojureScript compiled file Created: 10/Mar/15  Updated: 10/Mar/15

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   

Needed for shared AOT cache






[CLJS-1092] Basic CommonJS processing Created: 07/Mar/15  Updated: 07/Mar/15

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

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


 Description   

Given a CommonJS file we should able to produce an IJavaScript map with :requires, :provides, etc.






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

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

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


 Description   

Currently 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-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: 0
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-1075] Generic inline source map support 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: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently hard coded to REPLs. Would simplify jsbin and similar integration.






[CLJS-1074] Externs inference 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: 0
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.






[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-911] Cljs's clojure.string.replace replacement fn takes different args to Clj's clojure.string.replace Created: 17/Dec/14  Updated: 24/Feb/15

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

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

ClojureScript 0.0-2411



 Description   

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

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

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

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

Thanks!



 Comments   
Comment by Joseph Smith [ 24/Feb/15 2:33 PM ]

+1 fixing this (preferably making the ClojureScript version work like the Clojure version).





[CLJS-1060] simplify nREPL integration Created: 23/Feb/15  Updated: 23/Feb/15

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   

-setup and -teardown need to be elidable. If cljs.env/*compiler* already bound use that. The eval of (ns cljs.user ...) form for auto-import of cljs.repl helpers needs to respect *cljs-ns* and restore it afterwards.






[CLJS-1047] externs checking for js/foo Created: 19/Feb/15  Updated: 23/Feb/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   

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-1059] Simple interface wanted to convert cljs forms to js Created: 22/Feb/15  Updated: 23/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Stuart Mitchell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: analyzer, compiler

Patch: Code

 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)




[CLJS-1009] Allow deps.cljs to declare a foreign lib as remote Created: 05/Feb/15  Updated: 21/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When using things like the Google Maps JS API [1] the Javascript that is required can't be bundled inside a jar as it depends on the used API key.

To be able to provide externs for those kind of libraries there should be a way to declare them as "remote" in deps.cljs.

[1] https://developers.google.com/maps/documentation/javascript/tutorial






[CLJS-450] (ns) within (do) inconsistent with Clojure behaviour Created: 27/Dec/12  Updated: 21/Feb/15

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

Type: Defect Priority: Trivial
Reporter: Stuart Campbell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

An (ns) within a (do) doesn't quite work as expected at the REPL:

ClojureScript:cljs.user> (do (ns foo) (def x 42))   
nil
ClojureScript:foo> x
nil
ClojureScript:cljs.user> cljs.user/x
42

The Clojure equivalent:

user=> (do (ns foo) (def x 42))
#'foo/x
foo=> x
42


 Comments   
Comment by David Nolen [ 05/Jan/13 2:05 PM ]

Looks like we need to do something similar to what is done in Clojure with top level do - http://github.com/frenchy64/typed-clojure/pull/4





[CLJS-1050] Warn on code that will obviously prevent dead code elimination or cross module code motion 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: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Might make sense to support a module level flag like *warn-on-reflection*, perhaps *warn-on-deoptimization*? Simple cases of this are top-level data structures and and top-level side effects.






[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-1044] Investigate Closure CommonJS Support Created: 16/Feb/15  Updated: 16/Feb/15

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


 Description   

It appears Google Closure has basic support for CommonJS modules. Apparently mixing CommonJS and Closure style libraries seems somewhat supported but this is not a necessary goal. For our purposes it may be satisfactory to simply allow CommonJS libs to be a part of the build, likely as a separate pass, in which require is compiled away.






[CLJS-1033] take and drop accept nil as n argument Created: 12/Feb/15  Updated: 12/Feb/15

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

Type: Defect Priority: Minor
Reporter: Sean Grove Assignee: Sean Grove
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript


Attachments: Text File require-integer-for-take-and-drop.patch    
Patch: Code and Test

 Description   

(take nil [1 2 3]) in Clojure raises an error
In ClojureScript, it's the same as (take 0 [1 2 3])

This patch checks that take and drop both check that n is an integer, so that it doesn't silently behave differently.






[CLJS-1029] Investigate how ns aliasing can be supported in ClojureScript macro files Created: 11/Feb/15  Updated: 11/Feb/15

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 we just require the file. Perhaps possible to control compilation of the macro file such that ClojureScript ns aliases are established. This may not bear fruit but worth thinking about.



 Comments   
Comment by David Nolen [ 11/Feb/15 4:05 PM ]

Any design ideas along this path needs to keep .cljc files in mind.





[CLJS-1024] Double analysis warnings for dependencies in JARs Created: 10/Feb/15  Updated: 10/Feb/15

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

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


 Description   

If there is a problem with a CLJS source in a JAR, warnings will be emitted twice - once when analyzing the source in the JAR and again when the file is compiled to the output directory. Because the path has changed we don't realize we have already seen it. This probably just needs minor tweaks so that we drop warnings from the second pass.






[CLJS-1017] Support :main for :advanced builds Created: 06/Feb/15  Updated: 06/Feb/15

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

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


 Description   

This would use the supplied entry point to compute the build instead of looking at everything in the source directory.






[CLJS-901] Protocolize compiler access to the file system Created: 03/Dec/14  Updated: 04/Feb/15

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

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


 Description   

Currently builds are based on files on disk. It is desirable to be able to instead get in memory builds, WebDAV based builds, S3 based builds, etc. Many of these alternative strategies are not in scope for the ClojureScript compiler but this does not mean we should not supply the needed hooks for users to control the behavior.



 Comments   
Comment by Thomas Heller [ 03/Dec/14 9:31 AM ]

This and some other issues opened recently (CLJS-900, CLJS-851, CLJS-899, ...) have some overlap with what I have done in shadow-build [1]. Memory based builds are actually already possible cause it will only touch the disk when asked to, although the API could use some cleanup.

Anyways, might be worthwhile to coordinate these efforts to make CLJS more accessible for everyone.

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

Comment by Alan Dipert [ 04/Feb/15 11:36 AM ]

I too think it would be totally awesome to have builds based on sources from disparate places.

One alternative in this spirit I have been thinking about is a "SourceSet" approach. The idea is, instead of teaching CLJS how to consume various place-types directly via protocols, provide an API for building a "SourceSet" value and also a build function that takes the SourceSet as input. I imagine the SourceSet in its simplest form as a map of namespaces to string sources.

With a value to represent sources that is place-agnostic and immutable, 3rd party tools can consume/emit/transform these values before invoking a compile without knowledge or interest in CLJS internals. Conversely CLJS need not be concerned with how SourceSets are constructed.

This whole idea is inspired by boot's FileSets, which work basically the same but can't have the "it fits in memory" assumption.





[CLJS-890] Incorrect behaviour of (str obj) when obj has valueOf method Created: 24/Nov/14  Updated: 03/Feb/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.)





[CLJS-957] Parallel compilation Created: 03/Jan/15  Updated: 02/Feb/15

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


 Description   

For non-interacting branches of the dependency graph we could compile in parallel.



 Comments   
Comment by Thomas Heller [ 03/Jan/15 3:06 PM ]

Just a heads-up: I attempted this in shadow-build.

The problem with this is macros, specifically their require. Granted you said non-interacting but suppose two namespaces require cljs.core.async.macros, they might trigger the clojure.core/require at the same time (parallel after all).

This resulted in a very weird exception: https://www.refheap.com/91731

I'm not sure how thread-safe clojure.core/require is supposed to be but it was as close as I was able to get tracking this weird error down. Removing reducers fixed everything. FWIW the gain was minimal to begin with.

Comment by David Nolen [ 03/Jan/15 3:09 PM ]

Making this really work I think largely depends on how much time you are willing to spend on the problem. Fork/join work stealing seems ideal for this not sure if you tried that. RE: macros, seems like you could use information collected from ns analysis to serialize calls to require.

Comment by Thomas Heller [ 03/Jan/15 3:28 PM ]

Lets throw some reducers at it was about as much thought I put into it.

Just beware of macros when you attempt to do something is all I wanted to say. Treating analysis seperate from cljs.compiler/emit might be a good idea though.

Comment by Dusan Maliarik [ 06/Jan/15 7:20 AM ]

I always thought parallel compilation belongs to the build tool (process), and should be parallelized per source file. Would there be any gain in using pmap in here ? It's used by lein-cljsbuild I suppose. Or parallelize this part?

Comment by David Nolen [ 06/Jan/15 7:54 AM ]

pmap won't work because of dependencies. A work queue approach is probably going to be most fruitful. Let's keep this thread clean unless you are actually going to work on this ticket and have a clear picture how to do it. Thanks.

Comment by John Chijioke [ 02/Feb/15 8:14 AM ]

David, Where exactly should the parallelization happen?





[CLJS-992] satisfies? does not work with locally bound symbols Created: 28/Jan/15  Updated: 30/Jan/15

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

Type: Defect Priority: Minor
Reporter: Richard Davies Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

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



 Description   
(ns sample)
(defprotocol ICustomer (doit [_]))
(defrecord Customer [fname])
(extend-type Customer
  ICustomer
    (doit [_] nil))
(let [t ICustomer]
  (js/alert (satisfies? t (Customer. nil))))

;satisfies? returns false but similar code in returns true.





[CLJS-27] Conditional compilation (or reading) Created: 22/Jul/11  Updated: 21/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Unresolved Votes: 12
Labels: None

Attachments: File cljs-27.diff     File cljs-27-v2.diff     File cljs-27-v3.diff     File cljs-27-v4.diff     File cljs-27-v5.diff     File cljs-27-v6.diff     File conditional-compilation-clojure.diff     File conditional-compilation-clojurescript.diff    
Patch: Code and Test
Approval: Vetted

 Description   

As people start trying to write libs that are portable between Clojure and ClojureScript they might need to have a bit of branching on target. N.B. supporting this means a change to Clojure, although it has general utility there as well.

Consider CL #+ #- reader macros - http://www.lispworks.com/documentation/lw50/CLHS/Body/02_dhq.htm

Patch: cljs-27-v6.diff

Related: CLJ-1424, TRDR-14



 Comments   
Comment by Roman Scherer [ 19/Jul/12 8:52 AM ]

The following patches include an implementation of Common Lisp's #+
and #- reader macros to allow conditional compilation/reading for
Clojure and ClojureScript.

The patches add a dynamic variable called features to the
clojure.core and cljs.core namespaces, that should contain the
supported features of the platform in question as keywords.

Unlike in Common Lisp, the variable is a Clojure set and not a list.
In Clojure the set contains at the moment the :clojure keyword, and in
ClojureScript the :clojurescript keyword.

I would like to get feedback on the names that are added to this
variable. Are those ok? Is :jvm for Clojure and :js for ClojureScript
better? Should ClojureScript add something like :rhino, :v8 or
:browser as well?

To run the ClojureScript tests, drop a JAR named "clojure.jar" that
has the Clojure patch applied into ClojureScript's lib directory.

Comment by David Nolen [ 19/Jul/12 12:18 PM ]

This is an enhancement so it probably requires a design page and extended discussion before it will go anywhere. Until that happens I'm marking this is as low priority.

Comment by Roman Scherer [ 19/Jul/12 1:45 PM ]

Ok. If someone could give me write access to the Clojure Dev Wiki I would be happy to start such a design page.

Comment by David Nolen [ 19/Jul/12 5:50 PM ]

If you've sent in your CA request permissions on the clojure-dev mailing list.

Comment by Roman Scherer [ 21/Jul/12 5:45 AM ]

I started a design page for this ticket in the Clojure Dev wiki:
http://dev.clojure.org/display/design/Feature+Expressions

Comment by Stuart Halloway [ 27/Jul/12 1:48 PM ]

Posting my comments over on the design page...

Comment by Alex Miller [ 06/Aug/14 7:42 AM ]

Latest patch updates into current ClojureScript and use of tools.reader/tools.analyzer etc. The reader changes are all in the accompanying tools.reader patch in TRDR-14. This patch adds support to allow a new option "features" which is expected to be a set of keywords. build will inject :cljs into this set. The feature set is maintained in clojure.tools.reader/*features*. set! is enhanced to special-case an update to *features* in the code (presumably a rarely-used feature).

Because tools.reader needs the supporting patch, I left in several changes that pull in a new version of tools.reader - I don't actually expect those to be the correct versions but they are there as a reminder to update all of the proper places.

Comment by Alex Miller [ 11/Sep/14 9:04 AM ]

cljs-27-v2.diff adds ability to load .clj files as well as .cljs files when compiling.

Comment by Alex Miller [ 07/Nov/14 10:55 AM ]

Fresh patch, switch to take .cljc files.

Comment by Alex Miller [ 06/Jan/15 3:04 PM ]

Freshened patch for current CLJS.

Comment by David Nolen [ 06/Jan/15 9:55 PM ]

Alex the freshened patch includes modifications to the compiler version dynamic var and compiler version fn. Can we remove these? Thanks!

Comment by Alex Miller [ 07/Jan/15 12:28 PM ]

Yep, updated to -v5 patch.

Comment by Alex Miller [ 21/Jan/15 4:36 PM ]

Updated to use new tools.reader patch and to remove the ability to dynamically set the features set. The active set of features can be set on startup and are held in the features var in the analyzer.





[CLJS-981] Better benchmarking infrastructure Created: 17/Jan/15  Updated: 17/Jan/15

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

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


 Description   

We should use ProcessBuilder to run the various benchmark scripts and control which benchmarks we test and which engines we run. Benchmarks should produce EDN data that can be written to a file, loaded into Incanter, etc.






[CLJS-934] In the REPL return vars after defs Created: 30/Dec/14  Updated: 14/Jan/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   

We don't want to emit these in during normal compilation. However it's nice to unify the REPL experience with Clojure's. Currently we just display the value of the def. REPLs could set a :repl build flag which is checked by the emit* :def case. For this to work the analyzer should compile the var AST and include it in the def AST so the compiler can optionally use it.



 Comments   
Comment by Mike Fikes [ 14/Jan/15 4:03 PM ]

A change was recently made to introduce new behavior if a :repl-verbose flag is set.

Perhaps all flags that control REPL behavior could be prefixed with :repl-, with the flag controlling this ticket controlled by :repl-def-vars or somesuch.





[CLJS-970] assert - generate Error string when compiling Created: 10/Jan/15  Updated: 12/Jan/15

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

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

Attachments: File cljs-generate-assert-message.diff    
Patch: Code

 Description   

Currently when using assert in CLJS the assert macro will generate the assert error message through CLJS. We can do that in CLJ which reduces the amount of code generated.

(let [bar 1]
  (assert (string? bar)))

Currently generates

if(typeof bar_11332 === 'string'){
} else {
throw (new Error([cljs.core.str("Assert failed: "),cljs.core.str(cljs.core.pr_str.call(null,cljs.core.list(new cljs.core.Symbol(null,"string?","string?",-1129175764,null),new cljs.core.Symbol(null,"bar","bar",254284943,null))))].join('')));
}

But could generate

if(typeof bar_23183 === 'string'){
} else {
throw (new Error("Assert failed: (string? bar)"));
}

Attached patch does that.



 Comments   
Comment by Thomas Heller [ 12/Jan/15 9:16 AM ]

Forgot to account for the fact that

(assert (string? something) "this might not be a string"))

New patch only generates the string of the form when compiling.





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

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

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

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


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

 Description   

If I evaluate

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

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

(lldb) bt

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

(Full stack trace attached as stacktrace.txt)

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

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



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

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

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

The construct

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

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

In r2014, the emitted JavaScript is:

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

while in r2024 the emitted JavaScript is:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

This JavaScriptCore perf problem is easily reproduced purely with JavaScript:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

For example:

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

Or maybe cheat and emit a ChunkedCons instead?

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

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

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

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

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

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

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

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

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

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

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

Comment by Mike Fikes [ 21/Dec/14 4:16 PM ]

Activity is occurring with the WebKit ticket:

1) It has been confirmed as reproducible
2) It appears to be imported into Apple's system (InRadar keyword added).

Additionally, I succeeded in reproducing the issue on an older PowerPC Mac OS X 10.4.11 Safari 4.1.3 (from 2010), so it is not a recent regression, FWIW.

Comment by Mike Fikes [ 03/Jan/15 7:43 PM ]

Note: CLJS-945 sets :static-fns true as the default.

Comment by David Nolen [ 03/Jan/15 7:49 PM ]

Mike this is true only for cljs.core.

Comment by Mike Fikes [ 06/Jan/15 1:42 PM ]

FWIW, the original ticket I filed with Apple (rdar://19310764) has subsequently been closed by Apple as a duplicate of rdar://19321122, which is the Apple ticket the WebKit team opened in association with https://bugs.webkit.org/show_bug.cgi?id=139847





[CLJS-404] Automate Browser REPL testing Created: 23/Oct/12  Updated: 26/Dec/14

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: 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-924] Better error message for mistaken use of 'def' Created: 24/Dec/14  Updated: 24/Dec/14

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

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

Attachments: Text File 0001-Better-error-message-if-def-is-mistakenly-substitute.patch    
Patch: Code

 Description   

ClojureScript shares what is IMHO one of the biggest weaknesses of the current JVM Clojure implementation: those (in)famous stack traces going down into the innards of the compiler if you get your syntax wrong. An easy mistake for noobs is to use 'def' in place of 'defn'; this patch makes that mistake a lot less painful to debug.

Feedback please!






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

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

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

Attachments: Text File cljs_651.patch    
Patch: Code

 Description   

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



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

All paths taken on satisfies are now hinted as boolean





[CLJS-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-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-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-507] Persistent Data Structure Benchmark Created: 20/May/13  Updated: 02/Dec/14

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

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


 Description   

Put together an easy to run series of persistent data structure benchmarks that can be easily run. For example we would like to submit to this Mozilla ticket http://bugzilla.mozilla.org/show_bug.cgi?id=874174






[CLJS-555] CLONE - Implement ratios Created: 27/Jul/13  Updated: 02/Dec/14

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

Type: Enhancement Priority: Trivial
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

Clojure.java contains support for Ratio types. It would be nice to have them in Clojurescript as well, but as far as I can tell this would be new development (please comment if I'm wrong). That is, there is no implementation of Ratio types in GClosure so this feature would need to be implemented from the ground up. In additional to the Ratio type, the following corresponding functions would also need implementations:

  • `ratio?`
  • `numerator`
  • `denominator`
  • `rationalize`

Plus the ratio type would need to be rolled into the existing mathematical frameworkings.

Imported from github issue #66



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

Lower priority, unsolvable without numerics overhaul.





[CLJS-886] Add a contains-key? function to the ITransientAssociative protocol Created: 14/Nov/14  Updated: 17/Nov/14

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

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


 Description   

I propose to add a contains-key? function to the ITransientAssociative protocol equivalent to the function with the same name in IAssociative.



 Comments   
Comment by David Nolen [ 17/Nov/14 7:49 AM ]

This is a departure from the interface design in Clojure. I recommend starting a discussion on clojure-dev.





[CLJS-884] with-redefs don't restores previous values when used inside go block Created: 09/Nov/14  Updated: 09/Nov/14

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

Type: Defect Priority: Trivial
Reporter: Vladimir Iakovlev Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojure 1.6, clojurescript 0.0-2371, core.async 0.1.346.0-17112a-alpha


Attachments: Text File cljs_884.patch    

 Description   

For example I have code and test:

(defn test-fn [] :original)

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))

And test failed in last `is`:

FAIL in (test-with-redefs-async) (:)
expected: (= (test-fn) :original)
  actual: (not (= :mocked :original))


 Comments   
Comment by Vladimir Iakovlev [ 09/Nov/14 1:26 PM ]

I attached little patch with fix. With it `with-redefs` forced to use previous value for restoring bindings.

But I don't know how to add test for this issue, because it requires core.async. So I tested with:

(defn test-fn [] :original)

(deftest test-with-redefs-sync
  (is (= (test-fn) :original))
  (with-redefs [test-fn (fn [] :mocked)]
    (is (= (test-fn) :mocked)))
  (is (= (test-fn) :original)))

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))




[CLJS-872] Ordered collections printed as if they were unordered at the REPL Created: 13/Oct/14  Updated: 14/Oct/14

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

Type: Defect Priority: Trivial
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is due to cljs.repl's read-then-print processing of string representations of return values that come back from the JS env. As of release 2371, the relevant code fragment lives here:

https://github.com/clojure/clojurescript/blob/r2371/src/clj/cljs/repl.clj#L156

A larger snippet to demonstrate this:

ClojureScript:cljs.user> (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14)
{1 2, 3 4, 5 6, 7 8, 9 10, 11 12, 13 14}
ClojureScript:cljs.user> (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)
{7 8, 1 2, 15 16, 13 14, 17 18, 3 4, 11 12, 9 10, 5 6}
ClojureScript:cljs.user> (seq (array-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18))
([1 2] [3 4] [5 6] [7 8] [9 10] [11 12] [13 14] [15 16] [17 18])
ClojureScript:cljs.user> (hash-map 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18)
{7 8, 1 2, 15 16, 13 14, 17 18, 3 4, 11 12, 9 10, 5 6}

This issue seems to be the most likely cause of the problem described in this StackOverflow question. It would be nice to print ordered collections in the "expected" way to prevent user confusion.



 Comments   
Comment by David Nolen [ 14/Oct/14 5:23 AM ]

How is this handled in Clojure?

Comment by Michał Marczyk [ 14/Oct/14 7:40 AM ]

The built-in REPL simply prints string representations of values to *out* without attempting to read them back in first.

Comment by David Nolen [ 14/Oct/14 7:54 AM ]

Well the result of REPL evaluation is a string unlike Clojure, in order to get a proper print at the ClojureScript REPL it seems necessary to read back the result. I will say it's not clear to me array-map promises anything about order anyway.

Comment by Michał Marczyk [ 14/Oct/14 8:02 AM ]

It does – see http://clojure.org/data_structures, where it says

ArrayMaps
When doing code form manipulation it is often desirable to have a map which maintains key order. An array map is such a map - it is simply implemented as an array of key val key val... As such, it has linear lookup performance, and is only suitable for very small maps. It implements the full map interface. New ArrayMaps can be created with the array-map function. Note that an array map will only maintain sort order when un-'modified'. Subsequent assoc-ing will eventually cause it to 'become' a hash-map.

This snippet has been there for as long as I can remember; and this particular feature comes up in various online discussions from time to time. The docstrings for array-map (the core function) are unambiguous in their promise to construct array maps in both Clojure and ClojureScript.

As for the issue at hand, I do think it's a very minor one (hence the "Trivial" priority), but it is real – the representation printed at the REPL is out of line with the string returned from pr-str on the same value. In fact, one way to fix this would be to use pr-str representations computed on the CLJS side. (There may be some complications here, though, which we'd have to work out.)

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

Thanks for pointing out the API promise. OK, yeah I don't really have a good idea for how do this but a patch that isn't too ugly is welcome





[CLJS-860] redundant analysis of source files in JARs Created: 18/Sep/14  Updated: 18/Sep/14

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

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


 Description   

Source files in JARs are analyzed once then analyzed again when copied to the output directory.






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

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

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

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

 Description   

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

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



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

Did you any benchmarks on other JS engines?

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

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

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

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

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

Is there a existing one that I can work from?

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

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

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

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

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

The code is at:

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

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

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

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

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

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





[CLJS-677] cljs.reader doesn't support keywords starting with a digit Created: 12/Nov/13  Updated: 02/Jul/14

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: 2
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?





[CLJS-713] optimized case Created: 04/Dec/13  Updated: 23/Jun/14

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

Type: Enhancement Priority: Major
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.





[CLJS-575] cljsc.bat emit FileNotFoundException when compile samples in windows Created: 25/Aug/13  Updated: 19/Jun/14

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

Type: Defect Priority: Major
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-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-145] Cannot create more than one browser evaluation environment Created: 06/Feb/12  Updated: 19/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When testing multi-user applications, it may be desirable to run multiple browser-connected REPLs from the same Clojure process. Currently, the socket connection for the browser-connected REPL is stored in a global atom, so it is not possible to create multiple browser-connected REPLs on different ports.



 Comments   
Comment by David Nolen [ 29/Jul/13 11:11 PM ]

More powerful browser REPLs may be better handled now by external tools.

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

If you haven't already, check out Austin, which does exactly this, and other helpful things.





[CLJS-335] user defined tagged literals in CLJS Created: 06/Jul/12  Updated: 12/Mar/14

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

Type: Enhancement Priority: Minor
Reporter: kovas boguta Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I'm trying to make my own tagged literals for cljs, and its a rough scene. I've probably spend 15 hours trying to get this to work properly.

The quick hack of binding cljs-data-readers is a pretty bad solution, and doesn't work with cljsbuild.

One solution is to just copy the mechanism of data_readers.clj, instead calling it cljs_data_readers.clj. That would help.

A better solution is to just pass through all the tagged literals, and resolve them on the cljs side, eg `((cljs.core/get (cljs.core/deref cljs.reader/tag-table) ~tag) ~data))

That way the definition of the tags doesn't have to be repeated twice, as is done now (once in cljs.reader, and once in the clj-side cljs.tagged-literals) .

The problem with this is that clj itself is incapable of passing through undefined tags without exploding, which is also a near showstopper. Having something like cljs_data_readers.clj would get the job done in the meantime.

(another minor issue is that the cljs reader doesn't handle namespaced symbols, so in #a/b, b is what is used as the key into cljs-data-readers)



 Comments   
Comment by kovas boguta [ 06/Jul/12 12:52 AM ]

I've never used this before, so just assume everything in bold has earmuffs around it.

Comment by David Nolen [ 30/Aug/12 7:38 PM ]

This seems like a lot of separate issues rolled into one. Can we break this ticket apart into simpler actionable tickets?

Comment by kovas boguta [ 15/Dec/12 10:35 PM ]

Now that we have default-data-reader-fn, its possible to have a clean fix. Just set default-data-reader-fn to the function described above.

Should I generate a patch?

Comment by David Nolen [ 21/Dec/12 4:58 PM ]

What function defined above?

Comment by kovas boguta [ 25/Dec/12 1:30 PM ]

the function is
(fn [tag data] `((cljs.core/get (cljs.core/deref cljs.reader/tag-table) ~tag) ~data))

The proposal is to bind default-data-reader-fn to the above, in compiler.clj for the compile-file* function. And get rid of the binding for data-readers in that function as well.

It should be possible to get rid of the tagged_literals.clj file as well, since those definitions are no longer needed after this change.

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

Is this still an issue today?





[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-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-741] seq error message readability is not optimal Created: 02/Jan/14  Updated: 02/Jan/14

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

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

Attachments: Text File cljs_741.patch    

 Description   

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

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



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

Excellent thanks!





[CLJS-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: 0
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-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-693] new and dot form work on locals Created: 20/Nov/13  Updated: 02/Dec/13

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

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


 Description   

Given some JavaScript library included in the same page as CLJS output:

var Lib = {};

Lib.Thing = function (val) {
    this.val = val;
};

Lib.Thing.prototype.log = function () {
    console.log(this.val);
};

I can bind the Thing property to a local and construct it using new or the dot form. This code compiles and runs without errors, and logs three lines to the console.

(ns cljs-construct-locals-bug.core)

; Legit
(let [thing (new js/Lib.Thing "hello")]
  (.log thing))

; Questionable
(let [Thing (.-Thing js/Lib)
      thing1 (new Thing "maybe")
      thing2 (Thing. "no way")]
  (.log thing1)
  (.log thing2))

I talked to David Nolen and he said this behavior is not correct.



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

Upon further consideration this probably requires some feedback from the community. I'd forgotten that (Thing. foo bar ...) is just sugar for (new Thing foo bar ...) ...





[CLJS-705] locals clearing Created: 27/Nov/13  Updated: 29/Nov/13

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   

Without locals clearing ClojureScript is likely to suffer from the same cases as Clojure did for common uses of lazy sequences.



 Comments   
Comment by David Nolen [ 29/Nov/13 3:03 PM ]

For this we'll need to introduce some special private way to set a local to nil, i.e. (_clear_local sym)





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 19/Nov/13

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





[CLJS-653] all core fns backed by a protocol should have inlining versions 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   

first should be a compiler macro that checks &env to see if it's argument is ^not-native, if ^not-native it should inline into -first.






[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-633] Missing Java type hints in closure.clj Created: 22/Oct/13  Updated: 22/Oct/13

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

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

Attachments: Text File cljs_ticket_633.patch    

 Description   

A number of reflection warnings are generated from closure.clj. Fixing those probably would not improved performance but it would be cleaner anyway.



 Comments   
Comment by Julien Eluard [ 22/Oct/13 8:03 AM ]

Attached patch.

There are a couple warnings left, similar to:
Reflection warning, cljs/closure.clj:232:1 - reference to field url can't be resolved.
They appear to be due to some unexpected protocol method name? It looks harmless.





[CLJS-625] apply constructor generates invalid JS Created: 18/Oct/13  Updated: 18/Oct/13

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

Type: Defect Priority: Trivial
Reporter: George Fraser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

0.0-1913



 Description   

(apply js/String. "foo")

generates the javscript:

cljs.core.apply.call(null,String.,"foo")

which does not parse. This seems like a bad failure mode.






[CLJS-497] Constant literal optimization Created: 25/Apr/13  Updated: 27/Aug/13

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

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


 Description   

We should optimize constant literals, in particular keywords. This optimization means that we will have to decide whether to make identical? slower by testing for keywords (this means it's probably a bad idea to continue to inline it) or to provide a special keyword-identical? that does the right thing.



 Comments   
Comment by Sean Grove [ 26/Aug/13 11:03 PM ]

This is related to the reified keywords in cljs, see http://dev.clojure.org/jira/browse/CLJS-576

Comment by Sean Grove [ 27/Aug/13 4:24 PM ]

There's another interesting twist while using piggieback + brepl that relates to a missing constants_table.js. Not sure what causes it (haven't found a way to repro), but only happens in a few circumstances, so the repl still mainly works.

The runtime part continues to works fine however.





[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-240] Warning under advanced compilation about incorrect protocol implementation signatures Created: 05/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: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-113] Allow colon as whitespace in map literals Created: 18/Dec/11  Updated: 29/Jul/13

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

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Comments   
Comment by Gary Fredericks [ 24/Apr/12 7:40 AM ]

Will this change cause strings like "{\"foo\" :true}" to have ambiguous parses? And if we want CLJS to be a superset of JSON do we need to parse "null" as well?





[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-436] defn missing arg vector gives error about max Created: 06/Dec/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: David Nolen Assignee: Michał Marczyk
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-436-validate-arguments-to-fn-forms.patch    

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

The issue is actually a bit subtle for example:

(defn foo "name")

is valid Clojure - however this doesn't work in ClojureScript. In this case ClojureScript generates the following:

function () {
    switch (arguments.length) {
    }
    throw (new Error("Invalid arity: " + arguments.length));
}
Comment by David Nolen [ 06/Dec/12 10:19 AM ]

After talking to Rich the above really should not compile.

Comment by Michał Marczyk [ 08/Dec/12 6:09 PM ]

Here's a patch with the fn* validations I could think of. It seems to cause the compilation time to increase by a measurable amount, which I find quite surprising (even though it's to be expected that there will be quite of few fn* forms in CLJS sources). I'd love to know if it's just on my box (let's hope so). If validations really have that sort of effect, I'm thinking about refactoring the analyser so that they can be turned on (which should also be the default) and off (for particularly fast compilations).

Comment by Michał Marczyk [ 08/Dec/12 6:11 PM ]

Ah, wait, there's a minor typo in the commit message, I'll fix it in a second...

Comment by Michał Marczyk [ 08/Dec/12 6:13 PM ]

...um, no there isn't. Sorry for the confusion.





[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-210] Implement Var form, var-get, and var? in CLJS Created: 27/Apr/12  Updated: 28/Oct/12

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

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

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

 Description   

See discussion of Vars in CLJS here: http://dev.clojure.org/display/design/Dynamic+Binding
My goal is to eventually implement dynamic scope capture, via bound-fn and friends. The attached patch is a step in that direction.

This patch adds support for parsing the (var foo) form. The #' reader macro is provided by JVM Clojure.

#'foo emits code to construct a Var object. In this patch, each invocation of 'var will create a unique Var object. This means they are '= by fully qualified symbol, but not 'identical?. Simple memoization would fix that, but I'm not going to bother until I get to Dynamic Var objects.

The main advantage of this level of Var support is for the interactive development convenience of being able to defer dereferencing top-levels. For example, (def fog (comp f #'g)) will pick up redefinitions of 'g, but not of 'f.



 Comments   
Comment by Brandon Bloom [ 28/Apr/12 10:07 PM ]

I found two issues with this patch:

1) I never used the IVar that I declared :-P

2) (apply #'+ (range 100)) throws "Invalid arity: 101" – this is related to http://dev.clojure.org/jira/browse/CLJS-211

I'll look into fixing both. However, we should discuss supporting variable arity protocol methods...

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

any support for reified vars is a low priority for the foreseeable future.

Comment by Brandon Bloom [ 01/Sep/12 7:46 PM ]

Reified vars (and binding frames!) are a requirement for a CPS transform to be correct with respect to dynamic bindings.

Comment by Tom Jack [ 10/Sep/12 5:11 PM ]

+1 towards bound-fn and friends. I had an explanation written here but now I see "async Javascript (ie. nearly all Javascript) makes the binding macro effectively useless without bound-fn". Indeed.

Comment by Herwig Hochleitner [ 28/Oct/12 7:34 PM ]

bound-fn could also be implemented by setting / resetting bound vars before / after the wrapped fn, similar to binding. We would just need to add tracking of the currently rebound dynamic vars.
That should also be CPS - friendly, no?

Comment by Tom Jack [ 28/Oct/12 9:03 PM ]

I've considered that and would love to see it happen.

But what about:

(def ^:dynamic *foo* 3)
(set! *foo* 4)
(def f (bound-fn [] *foo*))
(set! *foo* 5)
(f)




[CLJS-167] Allow exiting CLJS repl with EOF/ctrl-D as well as :cljs/quit Created: 24/Mar/12  Updated: 08/Oct/12

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

Type: Enhancement Priority: Minor
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File quit.patch    
Patch: Code

 Description   

The CLJS repl is not a perfectly well-behaved command-line tool, in that it does not respect the convention of quitting by sending an EOF character with ctrl-D. This must be causing a small amount of irritation to everyone who uses the repl and is used to the customary amenities of any command-line tool; we could ease a little bit of frustration by supporting EOF.

I've attached a patch to improve this behavior, without affecting anything else.



 Comments   
Comment by Frank Siebenlist [ 08/Oct/12 11:20 AM ]

I like the idea, but I normally start the cljs-repl from a clj-repl with

user=> (run-repl-listen 9000 ".lein-cljsbuild-repl")

and then the proposed solution seems to exit both the cljs-repl and the clj-repl as well with a single ctrl-D, which increases the irritation with a small amount...

Sorry, I'm not well versed enough in the IO intricacies to provide an alternative.





[CLJS-197] Minor additions to clojure.browser.dom Created: 23/Apr/12  Updated: 30/Sep/12

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

Type: Enhancement Priority: Minor
Reporter: Moritz Ulrich Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-add-clojure.browser.dom-remove-node.patch     Text File 0002-docstrings-for-clojure.browser.dom.patch    
Patch: Code

 Description   

These two patches add clojure.browser.dom/remove-node and some missing docstrings to clojure.browser.dom.

What else can be done in this namespace? I don't want to break things, but I think some functions (log, click) don't really belong there.



 Comments   
Comment by David Nolen [ 23/Apr/12 11:26 AM ]

It's not my impression that clojure.browser.dom is meant to be a library for general consumption (though I could be wrong). I think it's there to support browser REPL and the samples.

Comment by Moritz Ulrich [ 23/Apr/12 5:00 PM ]

You might be right. It's still a very useful piece of code and makes DOM manipulation much more "clojure-y". I think it should be kept and improved.

Comment by Edward Tsech [ 30/Sep/12 10:14 AM ]

As I can see clojure.browser.dom isn't used in clojure.browser.repl. Is it there only for samples?





[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-163] Using ^:extern in repl fails Created: 15/Mar/12  Updated: 17/Jun/12

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

Type: Defect Priority: Minor
Reporter: icyrock.com Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript version used:

commit aa51a01141131736871e791918df63f185155421
Author: David Nolen <dnolen@Davids-MacBook-Pro.local>
Date: Wed Mar 14 20:21:07 2012 -0400



 Description   
ClojureScript:cljs.user> (defn ^:export a [])
"Error evaluating:" (defn a []) :as "cljs.user.a = (function a(){\nreturn null;\n});\ngoog.exportSymbol('cljs.user.a', cljs.user.a);\n"
org.mozilla.javascript.EvaluatorException: missing ; before statement (<cljs repl>#41)

nil
ClojureScript:cljs.user> (defn a [])
#<
function a() {
    return null;
}
>
ClojureScript:cljs.user>





[CLJS-171] Implement Pods Created: 28/Mar/12  Updated: 17/Jun/12

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   

It would be interesting to implement Pods in ClojureScript so that transients can become an implementation detail.






[CLJS-81] cljsc :externs flag fails when opts map not quoted Created: 23/Sep/11  Updated: 02/Jun/12

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

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


 Description   

For example, in the samples/hello-js directory, running the following works fine:

    cljsc src '{:optimizations :advanced :output-to "hello-extern.js" :externs ["externs.js"]}'

However, the following fails:

    cljsc src {:optimizations :advanced :externs ["./externs.js"]} > hello-extern.js

With the error message "Exception in thread "main" java.lang.IllegalArgumentException: No implementation of method: :as-file of protocol: #'clojure.java.io/Coercions found for class: clojure.lang.Symbol".

Fully qualifying the path to externs.js seems to work, BUT it actually fails by placing an exception message into the hello-externs.js file.



 Comments   
Comment by Brian Taylor [ 02/Jun/12 4:48 PM ]

I think your shell (bash?) may be treating the [...] portion of that expression as a character class and substituting matches from the file system. I'm not aware of any method for avoiding this other than changing shells to one that won't try to expand the [...].

http://tldp.org/LDP/Bash-Beginners-Guide/html/sect_04_03.html

Perhaps we should change the documentation examples so that they always quote the opts map. Maybe that would help avoid confusion.





[CLJS-186] ClojureScript wiki/The-REPL-and-Evaluation-Environments - small issues Created: 19/Apr/12  Updated: 30/May/12

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

Type: Defect Priority: Minor
Reporter: Mike Lamb Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation, wiki


 Description   

I just worked through the "The REPL and Evaluation Environments" wiki page and had to make the following change to get the example to work.

— The-REPL-and-Evaluation-Environments.md.0 2012-04-19 17:40:23.000000000 -0400
+++ The-REPL-and-Evaluation-Environments.md 2012-04-19 17:40:48.000000000 -0400
@@ -57,6 +57,7 @@
<body>
<div id="content">
<script type="text/javascript" src="out/goog/base.js"></script>
+ <script type="text/javascript" src="out/goog/deps.js"></script>
<script type="text/javascript" src="foo.js"></script>
<script type="text/javascript">
goog.require('foo');

Something else I noticed was that I didn't get the "Starting server on port 9000" message.



 Comments   
Comment by Brian Taylor [ 30/May/12 4:26 PM ]

In my environment, the deps.js script tag is appended to the page automatically as a side-effect of loading base.js.

base.js (line 613 in my release)

// Allow projects to manage the deps files themselves.
  if (!goog.global.CLOSURE_NO_DEPS) {
    goog.importScript_(goog.basePath + 'deps.js');
  }




[CLJS-29] Automate pre-push testing Created: 22/Jul/11  Updated: 04/Apr/12

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

Type: Task Priority: Minor
Reporter: Brenton Ashworth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Document the steps to follow to test that nothing is broken before pushing changes.

Automate this process.



 Comments   
Comment by David Nolen [ 04/Apr/12 11:35 PM ]

Do we need something more sophisticated then this: http://github.com/clojure/clojurescript/wiki/Running-the-tests





[CLJS-61] All global vars (f.e. __dirname) of node.js should be in cljs.nodejs Created: 27/Aug/11  Updated: 04/Apr/12

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

Type: Defect Priority: Minor
Reporter: Javier Neira Sanchez Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojure 1.3.0-alpha-8
clojurescript commit 22a64ff17b343b6c61039fcb66fd9acf34d98522
windows vista
node.js version v0.4.11



 Description   

To get access of all node.js goodies without using js* it would be nice to all global variables of node.js (http://nodejs.org/docs/v0.4.11/api/globals.html#process) were part of the module cljs/nodejs
Thanks






[CLJS-91] cljsc does not copy dojo dependencies Created: 14/Oct/11  Updated: 14/Oct/11

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

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


 Description   

When I run "cljsc.bat src > hello.js" on as program that uses Dojo it does not copy the Dojo dependencies to the out directory. If I manually copy the Dojo dependencies it works fine. I did create a dojo.jar file in the clojurescript/lib folder.

Here is the program and html:

(ns hello (:require [dojo :as dojo]))
(defn ^:export say-hello [] (dojo/place "<div>hello world</div>" (dojo/body)))

If this is really a bug as opposed to expected behavior, I can sign a CA and take a crack at fixing it.






Generated at Sat Mar 28 00:14:23 CDT 2015 using JIRA 4.4#649-r158309.