<< Back to previous view

[CLJS-2047] Bump tools.reader to 1.0.0-beta4 Created: 23/May/17  Updated: 23/May/17

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


 Description   

[org.clojure/tools.reader "1.0.0-beta4"] is used by core.async and conflicts with CLJS.






[CLJS-2027] Add language-in for ECMA 2017 and ECMA Next Created: 08/May/17  Updated: 22/May/17

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

Type: Enhancement Priority: Trivial
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

GCC allows more language-in now so we can add:

(:ecmascript-2017)                CompilerOptions$LanguageMode/ECMASCRIPT_2017
    (:ecmascript-next)                CompilerOptions$LanguageMode/ECMASCRIPT_NEXT

to lang-key->lang-mode



 Comments   
Comment by António Nuno Monteiro [ 14/May/17 7:52 PM ]

This needs to wait until the Closure Compiler team releases the May 2017 version of the compiler, since the `ECMASCRIPT_2017` option was added in a recent patch: https://github.com/google/closure-compiler/pull/2453

Comment by António Nuno Monteiro [ 22/May/17 9:51 PM ]

Attached patch with fix. Also upgrades Closure to the most recent May 2017 release.





[CLJS-2046] Optimize expression in call position Created: 20/May/17  Updated: 20/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Just what CLJS-855 did for arguments, should be done for a function expression.

Examples:

(@a :b)
((:x m) :b)

Currently, even with static-fns these both emit (inefficient) .call code. The expression should be put into a let binding and then the compiler can emit efficient code that checks for the IFn protocol.

Also needed for CLJS-2041 .






[CLJS-2041] Compiler flag to drop Function.prototype.call invokes Created: 18/May/17  Updated: 20/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2041.patch    

 Description   

Background

If the compiler doesn't know the arity of the function and :static-fns is true, it'll emit code to check if this arity exists or otherwise use .call(null, a0)
Example:

(fn [x] (x 1))
function(x){
x.$cljs$core$IFn$_invoke$arity$1$ ? x.$cljs$core$IFn$_invoke$arity$1$(1) : x.call(null, 1)

Furthermore, any data structures/types that implement IFn will attach .call and .apply to the type's prototype. This (often multi arity) function then calls the appropriate protocol functions depending on the argument.

Problems

The .call is slightly slower.
If the function only has one arity, it'll get called with the .call instead of a faster x(1).

Possible improvement

Since, with :static-fns the compiler checks if the protocol exists on the function/object the .call is never used unless it's a function (NOT a data structure) with exactly one arity.

We can also avoid emitting the attaching of the .call and .apply to any data structures since they'll never be used. (Again, with :static-fns)

Problems & Evaluation

On my local 18k LOC project, I only ran into one problem in a single line of datascript that used:

((.-rschema db) prop)

This generated (even with :static-fns set) db.rschema.call(null,prop) which failed since I removed the .call from the prototype of the PersistentHashMap (the rschema).

Fix was easy:

(let [rschema (.-rschema db)]
  (rschema property))

Which then resulted that the code checked for the protocol at the call site.

Implementation

Q1: Should :static-fns:

  1. No emission of .call and .apply on any IFn protocol implementation? (See add-ifn-methods in core.cljc.
  2. Emission of x.$cljs$core$IFn$_invoke$arity$1$ ? x.$cljs$core$IFn$_invoke$arity$1$(1) : x(1)

Or another compiler option so users can try it out?

Q2: Should

((.-rschema db) prop)

result in checking for IFn protocol?



 Comments   
Comment by A. R [ 18/May/17 2:45 PM ]

That patch is bad. binding doesn't work consecutively.

Comment by Thomas Heller [ 18/May/17 4:47 PM ]

Can you provide a full example?

(fn [x] (x 1))

What is x?

Comment by A. R [ 19/May/17 1:13 AM ]

x can be a few things:

  1. Datatype implementing IFn
  2. A JS function
  3. A CLJS function with exactly 1 arity
  4. A CLJS function with multiple arity
  5. A CLJS function with variadic arity

The call sites emits code that check if the arity exists. So (1) & (4) will never be called through the dispatcher. (2) & (3) will be fine being called normally by f. The only problem is (5) which is very rare.

(defn foo [x] (x 1 2 3 4 5))
(foo =)

Will require the dispatcher generated & working. In my code I never used that pattern so I could even remove all dispatcher functions and everything still worked. Note: The patch doesn't remove the dispatchers, this will still work with the patch. But a more aggressive patch could remove them and throw and error about "ineffcient" invokes. (That's actually how I found CLJS-2042)
(Workaround would be to use apply above).

But to answer your question:

(defn run!
  [proc coll]
  (reduce (fn [_ x] (proc x)) nil coll)
  nil)

Always calls proc with .call if a one arity function is passed (without the patch).

Comment by David Nolen [ 19/May/17 8:50 AM ]

A.R. 5) is probably a deal-breaker. We should avoid statements like "rare" in assessments when we can't possibly know what people are doing in the wild with a 5 year old OSS project.

Comment by David Nolen [ 19/May/17 9:12 AM ]

After brief discussion I agree that after `IFn` was properly implemented `.call` becomes vestigial in theory. However downstream libraries may depend on this detail so a flag is desirable for now.

Patch review. Please keep patches more tightly focused. Remove dropping the generated `.call` code code for now. Let's call the flag *fn-invoke-call*. One of your code gen blocks looks inverted to me with respect to the flag. Please fix these issues and I will re-review.





[CLJS-2042] Variadic invoke calls array_seq inefficiently Created: 18/May/17  Updated: 20/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2042.patch    

 Description   

The compiler emits an inefficient call:

variadic-invoke
       (let [mfa (:max-fixed-arity variadic-invoke)]
        (emits f "(" (comma-sep (take mfa args))
               (when-not (zero? mfa) ",")
               "cljs.core.array_seq([" (comma-sep (drop mfa args)) "], 0))"))

Since array_seq is itself variadic this can be optimized. Should probably be:

cljs.core.prim_seq.cljs$core$IFn$_invoke$arity$2$(...,0)

Or even a {{new IndexedSeq([...],0,nil)}}?



 Comments   
Comment by David Nolen [ 19/May/17 3:21 PM ]

Ok.





[CLJS-2045] sort-by: Avoid re-creating comparator Created: 20/May/17  Updated: 20/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The fn->comparator call should be lifted:

(sort (fn [x y] ((fn->comparator comp) (keyfn x) (keyfn y))) coll)
(let [comparator (fn->comparator comp)]
  (sort (fn [x y] (comparator (keyfn x) (keyfn y))) coll))

Also, fn->comparator is again called on the function in sort, not sure how to avoid that unless we copy the sort code into sort-by.






[CLJS-2043] Remove checks made redundant by CLJS-2040 Created: 20/May/17  Updated: 20/May/17

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

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


 Description   

CLJS-2040 added a check to see if we have analyzer metadata for a given ns. The old checks are still in there but would also be covered by the new check. We should probably remove them.

https://github.com/clojure/clojurescript/commit/77e01a01af9f45c76cfa34aa67bfae154b075544#diff-55b85385d2d0bfb6dc20d59ed982d5c8R953






[CLJS-2038] self calls do not optimize - regression Created: 15/May/17  Updated: 18/May/17

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

Type: Defect Priority: Major
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is a regression of:

https://dev.clojure.org/jira/browse/CLJS-275



 Comments   
Comment by A. R [ 18/May/17 2:19 AM ]

This issue can be solve by changing the defn macro for the "simple" case and carrying the function name over to the function:

(core/list 'def (with-meta name m)
  (cons `fn (cons name fdecl)))

This isn't done in clojure because of: https://dev.clojure.org/jira/browse/CLJ-809

Though I don't think that's and issue in CLJS since we don't have real vars anyways and can't redefine {{def}}s.





[CLJS-1871] A declare with :arglists should generate static function calls Created: 14/Dec/16  Updated: 15/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: performance


 Description   

This is performance enhancement.

  1. Problem
    After a declare the compiler doesn't know which arities the function will be defined with and hence generates code that checks if that arity is defined and then either calls it or uses a slower `xy-fn.call(null, ...)` construct. This is not optimal since it can be slower and also generates slightly more code.

Especially functions which only have one arity are problematic since they will end up being called with `xy-fn.call`.

  1. Affects
    Code that uses a function that was only declared and not def'ed yet. Such as `cons` in `IndexedSeq` or `conj!` in `TransientHashMap`.
    1. Performance
      A preliminary benchmark showed neglible improvements in Chrome v54 but a significant (factor of 2.2) performance benefit in Firefox.
  1. Solution
    Most of the declares should use `(def ^{:declare true, :arglists '([x xs])} cons)` and the compiler should take the `:arglists` into consideration and emit direct function calls instead.


 Comments   
Comment by A. R [ 12/May/17 8:26 AM ]

Similarly, functions that call themselves recursively don't get invoked optimally. Such as:

  • push-tail
  • do-assoc
  • pop-tail
  • tv-push-tail
  • tv-pop-tail

Matters quite a bit for TreeMap kv-reduce + dissoc.

EDIT: Separately addressed: https://dev.clojure.org/jira/browse/CLJS-2038





[CLJS-2037] Throw if overwriting alias in current namespace Created: 14/May/17  Updated: 15/May/17

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: António Nuno Monteiro
Resolution: Unresolved Votes: 0
Labels: None


 Description   

We should make this behavior the same as Clojure, where requiring a different namespace under and existent alias throws



 Comments   
Comment by David Nolen [ 15/May/17 7:59 AM ]

Go for it.





[CLJS-1982] Backtick of a quoted namespace should not change it Created: 16/Mar/17  Updated: 14/May/17

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

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


 Description   

Little inconsistence with Clojure here:

boot.user=> `'my-namespace.test-runner
(quote my-namespace.test-runner)

while in lumo, planck and replumb (notice the slash instead of the dot):

cljs.user=> `(require 'my-namespace.test-runner)
(cljs.core/require (quote my-namespace/test-runner))

this happens in a normal repl as well. Some other examples:

cljs.user=> `'my-namespace.test-runner
(quote my-namespace/test-runner)
cljs.user=> `'my-namespace.test-runner.ars
(quote my-namespace/test-runner.ars) ;; second not changed





[CLJS-2003] munge/demunge in cljs.core has too many calls to str Created: 10/Apr/17  Updated: 14/May/17

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

Type: Enhancement Priority: Trivial
Reporter: Erik Assum Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-2003-remove-redundant-calls-to-str-in-munge-dem.patch    
Patch: Code




[CLJS-2004] Minor fix for test-simple script Created: 10/Apr/17  Updated: 14/May/17

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

Type: Enhancement Priority: Minor
Reporter: Dejan Josifovic Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: clojurescript, script, test
Environment:

Lubuntu 16.10


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

 Description   

On Ubuntu based Linux distributions $[] doesn't work.
Output is: Tested with $[ran+1] out of 4 possible js targets
and should be: Tested with 4 out of 4 possible js targets

As in CLJS-929 (for test script), $(()) will work for ash, dash, bash, and zsh.



 Comments   
Comment by Dejan Josifovic [ 10/Apr/17 2:42 PM ]

Add patch.





[CLJS-2036] Relative path exception thrown when :preloads requires a :foreign-lib Created: 13/May/17  Updated: 13/May/17

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

Type: Defect Priority: Major
Reporter: Oliver George Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 10
[org.clojure/clojurescript "1.9.542"]



 Description   

I see an error when using :preloads and :foreign-libs.

java.lang.IllegalArgumentException:
C:\Temp\canidep\src\uilib.js is not a relative path, compiling:(C:\Temp\canidep\scripts\build.clj:5:1)

Unable to find source-code formatter for language: project.clj. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defproject canidep "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.9.0-alpha16"]
                 [org.clojure/clojurescript "1.9.542"]]
  :jvm-opts ^:replace ["-Xmx1g" "-server"]
  :source-paths ["src" "target/classes"]
  :clean-targets ["out" "release"]
  :target-path "target")
Unable to find source-code formatter for language: scripts/build.clj. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(require '[cljs.build.api :as b])
(b/build "src"
  {:main 'canidep.core
   :output-to "out/canidep.js"
   :output-dir "out"
   :verbose true
   :preloads '[canidep.preload]})
src/canidep/core.cljs
(ns canidep.core
  (:require [cljsjs.uilib]))

(println "Hello world!")
src/canidep/preload.cljs (works)
(ns canidep.preload)

(println "preload works without require")
src/canidep/preload.cljs (fails)
(ns canidep.preload
  (:require [cljsjs.uilib]))

(println "preload fails with require")
src/uilib.ext.js
// Nothing to see
src/uilib.js
// Nothing to see


 Comments   
Comment by Oliver George [ 13/May/17 11:49 PM ]

And the test deps file

src/deps.cljs
{:externs ["uilib.ext.js" ]
 :foreign-libs [{:file     "uilib.js"
                 :provides ["cljsjs.uilib"]}]}
Comment by Oliver George [ 13/May/17 11:53 PM ]

Unable to reproduce on OSX. Seems Windows specific.





[CLJS-1992] declare after def should have no effect Created: 30/Mar/17  Updated: 13/May/17

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

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

Attachments: Text File CLJS-1994.patch    

 Description   

Currently using declare after def will override all the analyzer data of the def with the basically empty data of the declare

(ns test.declare-after-def)

(defn foo
  ([x] x)
  ([x y] x)
  ([x y & more]
    x))

(declare foo)

(foo 1 2 3 4 5)

The effect is that the compiler will no longer emit the optimized invoke for foo

So instead of the desired

test.declare_after_def.foo.cljs$core$IFn$_invoke$arity$variadic((1),(2),cljs.core.array_seq([(3),(4),(5)], 0));

we end up with

(test.declare_after_def.foo.cljs$core$IFn$_invoke$arity$5 ? test.declare_after_def.foo.cljs$core$IFn$_invoke$arity$5((1),(2),(3),(4),(5)) : test.declare_after_def.foo.call(null,(1),(2),(3),(4),(5)));

Note that this always takes the "slow" path via .call since test.declare_after_def.foo.cljs$core$IFn$_invoke$arity$5 does not exist.



 Comments   
Comment by Thomas Heller [ 30/Mar/17 4:56 PM ]

The patch re-uses the already defined :redef-in-file warning type which I think is close enough, however in the spirit of better errors a new warning type might be better?

Comment by David Nolen [ 12/May/17 3:55 PM ]

This triggers a bunch of warnings not covered in the patch when running tests. Have you seen this?

Comment by Thomas Heller [ 13/May/17 3:49 AM ]

I changed the ticket a bit since the declare after def seems to be ok in Clojure and doesn't produce a warning there.

The goal is now to just prevent the declare from replacing the analyzer data of a def.

Currently investigating some strange analyzer behaviour that analyzes things twice.

[:redef-meta true]
[:a {:file /Users/zilence/code/oss/clojurescript/src/main/cljs/cljs/core.cljs, :line 945, :column 16, :end-line 945, :end-column 25, :tag boolean, :arglists (quote ([c x])), :doc Evaluates x and tests if it is an instance of the type
  c. Returns true or false}]
[:b {:file cljs/core.cljs, :line 945, :column 16, :end-line 945, :end-column 25, :tag boolean, :arglists (quote ([c x])), :doc Evaluates x and tests if it is an instance of the type
  c. Returns true or false}]

Only :file changes the metadata otherwise remains the same. This triggers a :redef-in-file warning.

Comment by Thomas Heller [ 13/May/17 4:14 AM ]

The new patch prevents a declare from replacing the analyzer data in the compiler env.

The question remains why certain things are analyzed twice but the :redef-in-file warning was fixed by moving the *file-defs* swap.





[CLJS-2021] Passing a non-vector to subvec returns an unusable object Created: 02/May/17  Updated: 12/May/17

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

Type: Defect Priority: Minor
Reporter: Frank Wang Assignee: David Nolen
Resolution: Unresolved Votes: 5
Labels: None

Attachments: Text File CLJS-2021.patch    

 Description   

For (subvec v start end), v can be any data type so long as (and (<= end (count v)) (<= start (count v))) is true. If v does not satisfy IVector, performing vector operations on the returned object will result in unexpected behaviour.

For example:

cljs.user=> (def subvec-list (subvec '(:foo) 0 1))
#'cljs.user/subvec-list
cljs.user=> (conj subvec-list :bar)
Error: No protocol method IVector.-assoc-n defined for type cljs.core/List: (:foo)

cljs.user=> (def subvec-nil (subvec nil 0 0))
#'cljs.user/subvec-nil
cljs.user=> subvec-nil
[]
cljs.user=> (conj subvec-nil :bar)
Error: No protocol method IVector.-assoc-n defined for type null:

cljs.user=> (def subvec-nil' (assoc subvec-nil 0 :foo))
#'cljs.user/subvec-nil'
cljs.user=> subvec-nil'
#object[Error Error: No protocol method IIndexed.-nth defined for type cljs.core/PersistentHashMap: {0 :foo}] cljs.user=> (def subvec-map (subvec {:foo :bar} 0 1))

#'cljs.user/subvec-map
cljs.user=> (assoc subvec-map :some-key :some-val)
Error: Subvec's key for assoc must be a number.
cljs.user=> (assoc subvec-map 0 :some-val)
#object[Error Error: No protocol method IIndexed.-nth defined for type cljs.core/PersistentArrayMap: {:foo :bar, 0 :some-val}]

This behaviour is inconsistent with JVM Clojure, where subvec will throw if (not (vector? v)).



 Comments   
Comment by Frank Wang [ 02/May/17 1:20 PM ]

A workaround for this is to call vec on the data structure before passing it to subvec.





[CLJS-2012] find on PHM with nil entry always returns nil entry Created: 19/Apr/17  Updated: 11/May/17

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

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

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

 Description   

The following always returns the nil entry:

(find (hash-map :a 1 :b 2 nil 3) :a) 
; [nil 3]


 Comments   
Comment by Francis Avila [ 19/Apr/17 10:11 AM ]

The way -find is called, it is guaranteed that (contains? coll k) was called and returned true first; -find is never responsible for membership testing. So the has-nil? is unnecessary. This would be a sufficient impl/patch:

(-find [coll k]
  (if (nil? k)
    [nil nil-val]
    (.inode-find root 0 (hash k) k nil)))

Now, whether this should be the contract for -find is a different story. We could potentially avoid double-lookup if -find were also responsible for membership testing: collections where testing membership also happens to retrieve the value could avoid doing it twice.

Comment by Thomas Mulvaney [ 19/Apr/17 10:19 AM ]

Yes, that is a much nicer patch. The current double lookup implementation of find is probably at its worst with PersistentTreeMaps which I think are O(log2n) rather than log32 like PHMs.

Comment by Thomas Mulvaney [ 11/May/17 7:10 AM ]

From what I understand IFind.-find takes the place of the IAssociative.entryAt method in Clojure. In Clojure from what I can tell, find makes a call to entryAt rather than doing a double lookup. Indeed, there is a commented out -entry-at method in Clojurescripts declaration of the IAssociative protocol. Does following the behaviour of entryAt in Clojure makes sense?





[CLJS-2031] Setting a "value" property optimized away by Closure compiler only with ClojureScript Created: 09/May/17  Updated: 10/May/17

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

Type: Defect Priority: Major
Reporter: Jussi Nieminen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I'm compiling the following code with the latest ClojureScript compiler:

(ns repro.core)

(enable-console-print!)

(defn clear-data []
  (do
    (set! (.-value (.querySelector js/document "#data")) "")
    nil))

(.addEventListener (.querySelector js/document "#data") "keyup"
                   (fn [event]
                     (when (= (.-keyCode event) 13)
                       (clear-data))))

... which then generates a function called repro.core.clear_data, which takes care of resetting the data input field of the html page. All works fine without optimizations, but with advanced optimizations Closure decides to inline and drop the setting of the .value field. In the end I have something like this (Q.b only does comparison on its parameters):

document.querySelector("#data").addEventListener("keyup",function(a){return Q.b(a.keyCode,13),null});

The nil at the end of the "do" special form represents other stuff that gets done (this is a dumped down repro from my original code). The important thing is, that setting the property happens in the middle of the function, and the result of that operation is not returned.

Closure docs do mention that setting a property without matching reads will be considered side-effect free and thus gets dropped, but I've thought that basic property names are excluded from this. This is at least somewhat confirmed by playing with the online compiler, which does not rename a "value" property.

The code used in the link above is a de-ClojureScriptified version of the output from the ClojureScript compiler (removed the classes and goog stuff). So somehow the additional code that ClojureScript includes seems to confuse the Closure compiler's optimizations.

I'm not sure if this really is a bug, or just working as expected (if a little confusing). But decided to report it anyway just in case.



 Comments   
Comment by Thomas Heller [ 09/May/17 2:11 PM ]

I investigated something similar recently. If you compile with :closure-warnings {:check-types :warning} the set! will not be removed by Closure, but you'll get a bunch of warnings about other things.

I'm not sure why but :check-types has some effects on code removal. It is enabled by default for the Closure web app and CLI but disabled by CLJS.

Comment by Jussi Nieminen [ 10/May/17 2:26 AM ]

I can confirm that adding the above to my build config makes it work. The resulting optimized JavaScript now looks like this (linebreaks and indentation added for clarity):

document.querySelector("#data").addEventListener("keyup", function(a){
  Q.b(a.keyCode,13)&&(document.querySelector("#data").value="");
  return null
});

I do get 196 warnings for this simple piece of code though...

Comment by Thomas Heller [ 10/May/17 2:54 AM ]

You can vote for https://dev.clojure.org/jira/browse/CLJS-2019 which takes care of some of the warnings. The other part is a WIP but I can compile my work projects with :check-types enabled and get only correct warnings.

We should probably still figure out if this is intended by Closure or a bug.

Comment by A. R [ 10/May/17 2:58 AM ]

Looks like a bug in GCC:

https://github.com/google/closure-compiler/issues/2365

Adding :closure-warnings {:check-types :warning} does fix it. Presumable that's also the configuration of the online compiler.





[CLJS-2030] Case with grouped keyword test emit result-expr multiple times Created: 09/May/17  Updated: 09/May/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

For case with N grouped keywords the result expression is emitted N times.

Example:

(macroexpand '(case x
                (:a :b :c :d :e) very-very-big-expr
                0))
=>
(let*
 [G__1148530 (if (cljs.core/keyword? x) (.-fqn x) nil)]
 (case*
  G__1148530
  [["a"] ["b"] ["c"] ["d"] ["e"]]
  [very-very-big-expr very-very-big-expr very-very-big-expr very-very-big-expr very-very-big-expr]
  0))

Proposed patch:

(every? core/keyword? tests)
(core/let [no-default (if (odd? (count clauses)) (butlast clauses) clauses)
                 kw-str #(.substring (core/str %) 1)
                 tests (mapv #(if (seq? %) (mapv kw-str %) [(kw-str %)]) (take-nth 2 no-default))
                 thens (vec (take-nth 2 (drop 1 no-default)))]
        `(let [~esym ~e
               ~esym (if (keyword? ~esym) (.-fqn ~esym) nil)]
           (case* ~esym ~tests ~thens ~default)))

Also fixes 2029 aka 1518.






[CLJS-1734] :import in ns silently discards imported classes with the same name Created: 11/Aug/16  Updated: 08/May/17

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Robin Hermansson
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   
(ns some.ns
  (:import goog.fx.Transition.EventType
           goog.history.EventType))

Both classes are named EventType and the second will effectively remove the first one without warning or error.



 Comments   
Comment by Robin Hermansson [ 07/May/17 8:03 AM ]

I would like to work on this issue.

Comment by David Nolen [ 08/May/17 7:14 PM ]

Robin, go for it





[CLJS-2011] :modules cannot rely on code motion or dead code removal Created: 19/Apr/17  Updated: 06/May/17

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

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


 Description   

Currently the :modules compiler option relies on code motion and dead code removal by the Closure Compiler. Many constructs used by CLJS are not moved/removed by Closure so the result will very likely be sub-optimal.

Summary

:modules currently relies on code motion and the user specifying every namespace that should go into a module.

This is unlikely to produce good results and several things are not correctly identified as pure by Closure and therefore are not moved.

Namespaces that are not referenced by any of the configured :entries are still included in the cljs_base.js module. If these files contain any kind of side-effects they are not removed as well.

Problematic issues include defmethod, anything with ^:export, construction of hash maps and more.

The :optimize-constants implicit option of :advanced is also not optimal and every keyword ends up in the cljs_base.js module.

Examples

Given this demo repo: https://github.com/thheller/cljs-issues/tree/master/modules

With the following ns structure:

  • foo.main requires foo.a, contains kw :foo.main/main
  • foo.a contains kw :foo.a/a
  • foo.b contains kw :foo.b/b and is not required by anything.

Built with these options:

(cljs/build
  "src"
  {:modules
   {:main
    {:entries ['foo.main]
     :output-to "out/main.js"}}
   :output-dir "out"
   :verbose true
   :optimizations :advanced})

The following build output:

Building module :cljs-base
Building module :main
  adding entry (foo.main)
  module :main depends on :cljs-base
Adding remaining namespaces to :cljs-base
  adding entry (goog)
  adding entry [goog.string goog.string.Unicode]
  adding entry [goog.object]
  adding entry [goog.math.Integer]
  adding entry [goog.string.StringBuffer]
  adding entry [goog.debug.Error]
  adding entry [goog.dom.NodeType]
  adding entry [goog.asserts goog.asserts.AssertionError]
  adding entry [goog.array]
  adding entry [goog.reflect]
  adding entry [goog.math.Long]
  adding entry (cljs.core)
  adding entry (cljs.core.constants)
  adding entry (foo.b)
  adding entry (foo.a)
  • foo.b was never referenced and should not have been included in the build.
  • foo.a was only used by foo.main but ended up in the cljs_base.js module since it is a transitive dependency the user did not specify. It should have been in the :main module before going into Closure.
  • main.js only contains something like console.log("main loaded",Fe); where Fe is the keyword which was constructed in the cljs_base.js although it was only used in this module.

A similar example at https://github.com/kommen/cljs-issues/tree/modules-foreign-libs/modules adds a dependency from foo.a to cljsjs.plotly

Build output:

Building module :cljs-base
Building module :main
  adding entry (foo.main)
  module :main depends on :cljs-base
Adding remaining namespaces to :cljs-base
  adding entry (goog)
  adding entry [goog.string goog.string.Unicode]
  adding entry [goog.object]
  adding entry [goog.math.Integer]
  adding entry [goog.string.StringBuffer]
  adding entry [goog.debug.Error]
  adding entry [goog.dom.NodeType]
  adding entry [goog.asserts goog.asserts.AssertionError]
  adding entry [goog.array]
  adding entry [goog.reflect]
  adding entry [goog.math.Long]
  adding entry (cljs.core)
  adding entry (cljs.core.constants)
  adding entry [cljsjs.plotly]
  adding entry (foo.a)
  adding entry (foo.b)
  • cljsjs.plotly was only used in foo.a, which itself is only used in foo.main, but ended up in cljs_base.js


 Comments   
Comment by David Nolen [ 28/Apr/17 3:59 PM ]

I don't see how this is a problem at all. This is just expected?

Comment by Thomas Heller [ 28/Apr/17 5:59 PM ]

This is not expected at all.

I assume you read the README of the demo project which highlighted some of the problems? I really don't know how to explain this better.

In shadow-build I collect the dependencies of all defined :entries and then move them as close to the edges as possible. If a dependency is only used in one :module it will be in that :module and not the base module. In my work project I define ONE entry namespace for one module and that causes 55 other files to be moved there as well. You really shouldn't expect a user to specify all 56 namespaces to get a proper split.

These files contain defmethod, cljs.spec/def and other side-effecty things that will not be moved by code-motion. They would remain in the base module. The solution is to add the files to the Closure JSModule before calling compile, which also allows :modules to work reasonably well in :simple and :whitespace.

Files that aren't required anywhere should not be passed into Closure as it cannot be guaranteed that they will be removed.

Comment by Dieter Komendera [ 03/May/17 7:57 AM ]

I'm also running into issues where :modules doesn't work as expected for me.

I've pushed a fork of Thomas' demo repo here showing that a dependency which I would expect to be moved into the main module, but instead it is put into the cljs_base module.

https://github.com/kommen/cljs-issues/tree/modules-foreign-libs/modules

Comment by David Nolen [ 05/May/17 12:21 PM ]

Thomas, I'm not going to look at information not in a ticket sorry. If there are details you want to be considered put them into the issue. This issue is danger of being closed unless corrected. Thanks.

Comment by Francis Avila [ 05/May/17 6:27 PM ]

I have updated the ticket description to include text from the READMEs mentioned in the two linked repositories. The code to reproduce the builds is too large to attach, so see linked repos for details.

Comment by Thomas Heller [ 06/May/17 2:45 AM ]

Francis Avila: Thanks for updating the description!

David Nolen: Sorry for keeping the description in the github repo, didn't think that this would be a problem. I mentioned these issues when you implemented :modules but that got lost in chat archives.

My only interest here was to report it so "someone" would hopefully pick it up. I can't take this any further as I disagree with some other related implementation choices.





[CLJS-1901] Investigate new Google Closure source mapping support Created: 24/Jan/17  Updated: 05/May/17

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

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


 Description   

Google Closure now contains comprehensive support for (at least from the command line) for source map merging and inline source map generation. We should investigate how reusable this functionality actually is.



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 3:30 PM ]

Investigated it a bit, just sharing what I learned so far:

1. historically there used to be a hidden flag `--source_map_input` which could be used to produce source-map-aware error reporting, not source map composition as name would suggest[1]
2. mid 2016, a patch landed[2], which enhanced this for full source map composition
3. by the end of 2016, the feature seems to be public and enabled in command-line tool by default[3][5]
4. as of today, the official source-maps wiki page[4] has not been updated to reflect this latest development

[1] https://github.com/google/closure-compiler/issues/1360#issuecomment-170716968
[2] https://github.com/google/closure-compiler/pull/1971
[3] https://github.com/google/closure-compiler/pull/2008
[4] https://github.com/google/closure-compiler/wiki/Source-Maps
[5] https://github.com/google/closure-compiler/pull/2129

Comment by Antonin Hildebrand [ 24/Jan/17 3:53 PM ]

Closure compiler also newly understands inlined source maps using data URLs in input Javascript files[1].

1. parsing of inline source maps is enabled by default unless `--parse_inline_source_maps=false` is passed, it is independent on `--source_map_input` flag
2. information from `--source_map_input` and inlined source-maps is merged, inlined maps override `--source_map_input`, the last inlined map wins in case of multiple //# sourceMappingURL=<data URL> present [2]

[1] https://github.com/google/closure-compiler/pull/1982
[2] https://github.com/google/closure-compiler/pull/1982#issuecomment-243249065

Comment by Thomas Heller [ 02/May/17 5:23 AM ]

FWIW I added support for this in shadow-build a while ago. It does not need inline source maps to work.

The code can be found here: https://github.com/thheller/shadow-build/blob/master/src/main/shadow/cljs/closure.clj

The relevant bits are .addInputSourceMap on Compiler and .setApplyInputSourceMaps on CompilerOptions.

If everything is properly configured the warnings displayed by Closure will contain an "Originally at:" location which points to the CLJS file.

Closure will also use the input source maps when generating source maps for :advanced builds, so the manual merge done by CLJS at the moment becomes unnecessary. The source maps also appear to be more accurate. Before input source maps I had a few issues where source maps were off by a few lines, but that may have been due to my incorrect source map handling in shadow-build.





[CLJS-1902] Add support for compiler option :inline-source-maps Created: 24/Jan/17  Updated: 01/May/17

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

Type: Enhancement Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Patch: Code and Test

 Description   

1. refactor `emit-source-map` and break it into multiple functions
2. fix logic for relative path computation (see `strip-prefix-path`)
3. add support for `:inline-source-maps` option
4. add tests

Related: CLJS-1402, CLJS-1901



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:09 PM ]

Patches:
https://github.com/darwin/clojurescript/commit/de1da63072c8049b9812d41cce750e5a972c0b0b.patch
https://github.com/darwin/clojurescript/commit/2389fb7839a18299d842d381b135a669a7091869.patch
https://github.com/darwin/clojurescript/commit/3ba594d8c7215ef8c8276737b7e5d8008c6b3a98.patch

Comment by Antonin Hildebrand [ 24/Jan/17 8:27 PM ]

Full review: https://github.com/clojure/clojurescript/compare/darwin:inline-source-maps~3...darwin:inline-source-maps

Also please note that the first patch testing original functionality fails in one test because there was a bug in timestamp formatting in :source-map-url case:
https://github.com/clojure/clojurescript/compare/master...darwin:inline-source-maps#diff-55b85385d2d0bfb6dc20d59ed982d5c8L1239

Comment by Antonin Hildebrand [ 25/Jan/17 10:25 AM ]

Today when testing Dirac I realised we need to embed sources contents as well.

Additional patch:
https://github.com/darwin/clojurescript/commit/c1df38f14a33d02fe2d421f80db0b421b17286bb.patch

New review URL: https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps

Tested in DevTools and works like a charm.

Comment by Dusan Maliarik [ 06/Feb/17 10:00 AM ]

This would be helpful for us as well.

Comment by Andrea Richiardi [ 01/May/17 3:49 PM ]

I have run across this one as well by following this tutorial.

Without either this patch or Dirac's complicated setup it is not currently possible to use node --inspect and debug correctly. The symptom I see on our side is that source maps are detected but for some reason Chrome DevTools does not show them in the Tree View.

The content of one of it is:

{"version":3,"file":"\/Users\/user\/cqrs-engine-cljs\/out\/cqrs\/event_store.js","sources":["event_store.cljs"], ...




[CLJS-2018] User supplied externs not loaded with user specified compiler state Created: 25/Apr/17  Updated: 26/Apr/17

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

Type: Defect Priority: Major
Reporter: Jonathan Henry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2018.patch    

 Description   

User supplied externs for use with warn-on-infer are only loaded in the 2-arity versions of cljs.closure/build, cljs.build.api/build and cljs.build.api/watch when compiler is nil.

Note: This only affects the warnings that are generated by the analyzer with warn-on-infer; the externs are correctly passed to gclosure.



 Comments   
Comment by Jonathan Henry [ 25/Apr/17 7:34 PM ]

This patch moves the loading of externs from cljs.env/default-compiler-env to cljs.closure/build.

Comment by Jonathan Henry [ 26/Apr/17 12:20 PM ]

Ignore this patch, I just realized this makes it so the built-in externs are no longer loaded for the compiler and analyzer API.





[CLJS-2016] Support inheritance annotations in externs Created: 25/Apr/17  Updated: 25/Apr/17

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

Type: Enhancement Priority: Major
Reporter: Jonathan Henry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Closure externs may contain @extends annotations to specify base classes. The base classes' attributes should be propagated to subclasses in `:cljs.analyzer/externs`.






[CLJS-1444] Node.js shim requires `node` invocation to be in the same directory as shim script Created: 10/Sep/15  Updated: 24/Apr/17

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

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

Attachments: Text File 0001-In-bootstrapping-code-use-__dirname-to-calculate-pat.patch    

 Description   

Don't have time to provide a proper repro now but the basic issue can be illustrated by this:

~/c/boot-cljs-example (master=) node target/main.js
module.js:338
    throw err;
          ^
Error: Cannot find module '/Users/martin/code/boot-cljs-example/out/goog/bootstrap/nodejs.js'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/martin/code/boot-cljs-example/target/main.js:6:1)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
~/c/boot-cljs-example (master=) cd target/
~/c/b/target (master=) node main.js
Starting...

This is compiled with boot because that was what I had at hand right now. The compiled shim looks like this:

var path = require("path");
try {
    require("source-map-support").install();
} catch(err) {
}
require(path.join(path.resolve("."),"out","goog","bootstrap","nodejs.js"));
require(path.join(path.resolve("."),"out","cljs_deps.js"));
goog.global.CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"nodejs"};
goog.require("boot.cljs.main");
goog.require("cljs.nodejscli");

The problem here is that path.resolve(".") will return the directory the node command was invoked in and not the directory of the shim. (See the "Cannot find module..." error above)

A solution could be to use __dirname which always resolves to the directory of the current file. This might result in some breakage for existing setups.



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

I have a proposed solution but I fear that supporting "run from anywhere" adds essential complexity to the resulting code. My thought process:

1. Relative paths are desirable to produce "context free code." If the user chooses absolute paths, then that behavior is respected and there's nothing to handle (no "path algebra") .

2. When dealing with relative paths, the whole system needs to establish a "frame of reference", a root path. The ClojureScript compiler assumes the path from which it is compiling to be that frame of reference, which usually coincides with the top root of the project. Though arbitrary, it is the only choice that makes sense.

3. The frame of reference is not explicit anywhere in the code, since it is defined as ".". If it were explicit, it would reveal context, as in "/home/some-user/their-folder/this-project/".

4. When we approach the code from another reference point (executing the script from another directory), we first need to find the original compiler path (reference point,) and then resolve all paths from there. The compiler uses `cljs.closure/path-relative-to` for this purpose.

Path algebra:
compiler-path = __dirname - output-to

Node.js

var compiler-path = __dirname.replace(output-to, "")
path.resolve (compiler-path, output-dir, "goog", "base.js")
path.resolve (compiler-path, output-dir, "cljs_deps.js")

which assumes that if output-to was given relatively, then output-dir is also relative. If they are not in sync, more work needs to be done to keep them that way.

It's not up to me to decide if the extra complexity is worth the use-case. I actually hope there is a simpler solution to solve this that I'm not seeing.

Comment by Karol Majta [ 14/Mar/16 10:43 AM ]

I find this behavior really weird and would opt for switching to __dirname. I am also not sure i fully understand consequences of such switch (I have little cljs experience, speaking more from the perspective of nodejs user). My point is: current behavior renders clojurescript hard to use for commandline and desktop applications (namely electron).

For command line and desktop applications assumptions about CWD cannot be made. For now i run my applications through a bootstrap script JS script:

process.chdir(__dirname);
require('./target/out');

I am lucky that my code does not have to use the real CWD, but it's a hack more than a real solution.

Speaking from nodejs perspective:

1. Using absolute paths is considered a bad practice anyway.
2. Nodejs programs that don't use external packages (don't depend on node_modules) can be run from any CWD
3. Nodejs programs that do depend on node_modules will break if run from a different directory than the one containing node_modules, but this is expected behavior.

Comment by J. Pablo Fernández [ 26/Jan/17 5:42 AM ]

I just run into this problem when trying to develop an Electron application. The way it's working right now is essentially unpackageable. I think it would be nice to have this behavior even as an option and I'm happy to work on a patch.

Comment by J. Pablo Fernández [ 26/Jan/17 5:57 AM ]

As far as I can see, this is the relevant code:

https://github.com/clojure/clojurescript/blob/cdaeff298e0f1d410aa5a7b6860232270d287084/src/main/clojure/cljs/closure.clj#L1410-L1411

Comment by J. Pablo Fernández [ 26/Jan/17 6:15 AM ]

A potential workaround seems to use :simple optimization.

Comment by Matt Lee [ 21/Apr/17 12:02 PM ]

I have also just hit this issue in an electron app that I'm building. @pupeno did you get anywhere with your offer to work on a patch? I too would be happy to look in to a patch for this. Although I think I'll need some pointers to get started.

Comment by Matt Lee [ 22/Apr/17 5:36 AM ]

From a quick experiment this morning it looks like replacing path.resolve(".") with __dirname at https://github.com/clojure/clojurescript/blob/aa5f001300e9aebd976cb180f5b7ccb37fcb6898/src/main/clojure/cljs/closure.clj#L1460-L1461 works for a couple of simple electron and node apps. By work I specifically mean that the error doesn't occur even when the app is started from a current working directory that is different to the compilation directory, i.e. the code is path independent.

I'll attach a patch for this once I've done some more complete testing. Before then any feedback on this approach would be appreciated.

Comment by Matt Lee [ 23/Apr/17 1:51 AM ]

Attaching patch for the suggested fix of using __dirname instead of "." in the generated script. I have tested this with nodejs 6.10.0 and electron 1.6.5.

Comment by Matt Lee [ 24/Apr/17 2:34 AM ]

Also see: CLJS-1990





[CLJS-1990] Clojurescript programs targeting nodejs should support global installation Created: 28/Mar/17  Updated: 24/Apr/17

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

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

Attachments: Text File 0001-CLJS-1990-Use-module-relative-__dirname-for-bootstra.patch    

 Description   

The top-level entry point in a :target :nodejs application uses $CWD relative paths to load the bootstrapping. See "path.resolve('.')" here: https://github.com/clojure/clojurescript/blob/296d0a69340e832b92ed742b3cd0304a06bea27f/src/main/clojure/cljs/closure.clj#L1460 for example.

This works fine for a local build, but is problematic when we try to globally install clojurescript (such as via 'npm install -g') because it requires the caller $CWD to be something that is likely to be unnatural (e.g. /usr/lib/node_modules/$pkg). Suggested fix is to replace "path.resolve('.')" with "__dirname".



 Comments   
Comment by Matt Lee [ 24/Apr/17 2:34 AM ]

Also see: CLJS-1444





[CLJS-2001] Add map-entry? predicate Created: 06/Apr/17  Updated: 19/Apr/17

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

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

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

 Description   

map-entry? has existed in Clojure since 1.8 would be nice to have it in ClojureScript.



 Comments   
Comment by Thomas Mulvaney [ 06/Apr/17 6:00 AM ]

The attached patch looks more like the first implementation of `map-entry?` as per CLJ-1831.

This is because ClojureScript returns PersistentVectors when iterating over PAM and PHM maps.

Comment by David Nolen [ 07/Apr/17 11:06 AM ]

This patch is no good. 2 element vectors are not MapEntry.

Comment by Francis Avila [ 07/Apr/17 7:22 PM ]

Given that Clojure still returns MapEntry (CLJ-1831 was backed out later) and CLJS returns vectors, it is probably impossible for this predicate to be portable. If we can't consider count-2 vectors map-entry?=true, then the only possible cljs impl is (defn map-entry? [x] false). Given this, perhaps the best solution is not to have map-entry? in cljs, to discourage people from using it in portable code.

Comment by David Nolen [ 12/Apr/17 1:03 PM ]

I'm fine with adding a MapEntry type which implements all the vector protocols and returning that instead. That work should be a separate issue though and then we can come back to this one.

Comment by Thomas Mulvaney [ 19/Apr/17 8:00 AM ]

This came about as I was porting some Clojure code. I was probably misusing/abusing map-entry? anyway. The code could be rewritten to check if something is a map first and then do the appropriate thing on the sequence of entries rather than doing the check from "with in" the collection.

As mentioned, a 2 element vector != MapEntry. So, I've opened an issue to track adding a MapEntry type: CLJS-2013





[CLJS-1998] Printing an Object with a null prototype throws an error Created: 04/Apr/17  Updated: 13/Apr/17

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

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


 Description   

ClojureScript doesn't handle printing objects with a null prototype:

(prn (.create js/Object nil)) ;;throws Uncaught TypeError: Cannot read property 'cljs$lang$ctorStr' of undefined
(str (.create js/Object nil)) ;;throws Uncaught TypeError: Cannot convert object to primitive value

For the first case, it looks like pr-writer-impl's last case needs a separate check for a nil object constructor. I've not yet investigated the second case.



 Comments   
Comment by David Nolen [ 07/Apr/17 11:09 AM ]

Is this a common JS pattern?

Comment by Jonathan Boston [ 07/Apr/17 11:40 AM ]

I assume it isn't since I only recently ran into it. But PDFjs uses it regularly, which is where the problem came to light.

It appears to be used for objects that are strictly for wrapping other data. Perhaps it's a performance optimization?

I'm away from a computer for the next few days, but can provide links/more info then as necessary.

Comment by David Nolen [ 07/Apr/17 12:03 PM ]

I'm happy to see a fix for the first case and I'd like hear more information about the second one.

Comment by Jonathan Boston [ 13/Apr/17 2:05 AM ]

So the second case is interesting and I'm not sure of the fix implications. The following plain javascript highlights the problem:

Object.create(null) + "";
"" + Object.create(null);
[Object.create(null)].join(""); //str implementation uses this

//all three calls give the following error
Uncaught TypeError: Cannot convert object to primitive value

The str implementation passes the value through to array.join, so we'd need to check for null prototypes for every str call.

Adding an extra conditional to printing out to the console seems harmless enough, but adding an extra conditional for every str call seems like it might not be worth it.

I'm happy to do a fix for whatever you suggest.





[CLJS-1593] Self-host: Munged minus macro Created: 25/Feb/16  Updated: 11/Apr/17

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: bootstrap

Attachments: Text File 0001-CLJS-1593-only-return-a-var-when-we-re-looking-for-a.patch    
Patch: Code

 Description   

In bootstrap, the macro form of cljs.core/- is evidently available as _ so, for example

(_ 7 3)
works.

Repro:

cljs.user=> (require 'cljs.js)
nil
cljs.user=> (cljs.js/eval-str (cljs.js/empty-state)
  "(_ 7 3)" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value 4}


 Comments   
Comment by Erik Assum [ 07/Apr/17 4:29 PM ]

So, in self-hosted (at least) the munged names are available, you can see this for `+` as well

cljs.user=> (_PLUS_ 4 4)
8
(in-ns 'cljs.js)
nil
cljs.js=> (compile-str (empty-state) "(+ 4 4)" nil identity)
{:value "((4) + (4));\n"}
cljs.js=> (compile-str (empty-state) "(_PLUS_ 4 4)" nil identity)
{:value "((4) + (4));\n"}
cljs.js=>

With a special `ns-interns**` as

(defn ns-interns**
  "Bootstrap only."
  [sym]
  (let [ns-obj (find-ns-obj sym)
        ns     (Namespace. ns-obj sym)]
    (letfn [(step [ret k]
              (let [var-sym (symbol k #_(demunge k))] ;; not demunging the key
                (assoc ret
                  var-sym (Var. #(gobject/get ns-obj k)
                            (symbol (str sym) (str var-sym)) {:ns ns}))))]
      (reduce step {} (js-keys ns-obj)))))

you see that `-` is stored under `_`, e.g. the munged name in the ns-object

cljs.core=> (get (ns-interns** 'cljs.core$macros) '_)
#'cljs.core$macros/_
cljs.core=>
Comment by Mike Fikes [ 11/Apr/17 8:00 AM ]

Interestingly, the patch works for cljs.core/- but it doesn't seem to completely work for new symbols defined in cljs.user, if I'm interpreting this correctly:

cljs.user=> (require 'cljs.js)
true
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval st '(def foo-bar 17) {:eval cljs.js/js-eval :context :expr} identity)
{:value 17}
cljs.user=> (cljs.js/eval st 'foo_bar {:eval cljs.js/js-eval :context :expr} identity)
WARNING: Use of undeclared Var cljs.user/foo_bar
{:value 17}
Comment by Erik Assum [ 11/Apr/17 3:05 PM ]

Yep

cljs.user=> (def foo-bar 17)
#'cljs.user/foo-bar
cljs.user=> foo_bar
            ^
WARNING: Use of undeclared Var cljs.user/foo_bar at line 1
17
cljs.user=>

Don't remember the details now, but the way I got to fixing the minus was by the code-path which
tries to figure out macros, IIRC.

I guess I'll have to look at the code-path which resolves normal symbols as well.





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

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

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


 Description   

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

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

In Clojure, this does what I want:

["ab" "ab"]

In ClojureScript, I get:

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

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



 Comments   
Comment by Erik Assum [ 10/Apr/17 11:12 AM ]

Might not be the answer you want, but Clojurescript uses js' split implementation.
Testing this in the browser you get

> "ab; ab;".split(/(; )|(;$)/)
< ["ab", "; ", undefined, "ab", undefined, ";", ""] (7)
>

from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split

If separator is a regular expression that contains capturing parentheses, then each time separator is matched, the results (including any undefined results) of the capturing parentheses are spliced into the output array. However, not all browsers support this capability.

Which means that to avoid this, you should use non-capturing groups:

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

Which incidentally can be simplified to

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

Which produces the result you're after in both clojure and clojurescript.





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

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

Attachments: Text File 0001-CLJS-1009-allow-remote-url-in-deps.cljs-foreign-libs.patch    

 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



 Comments   
Comment by Nathan Dao [ 08/Apr/17 6:43 PM ]

js_deps.cljs deliberately only allows upstream foreign-libs (defined in deps.cljs file) to be local file. Yet, this issue is still open, so I (hastily ) assume the local file enforcement was not intentional.

The patch should add back support for using url as foreign-libs in deps.cljs and remove unused codes in load-foreign-library*.





[CLJS-1997] Outward function type hint propagation Created: 03/Apr/17  Updated: 07/Apr/17

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

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

Attachments: Text File CLJS-1997.patch    

 Description   

New feature: Detect the type of a function and arrange so that it is available for outward type hinting.

For reference, a small amount of discussion from Jan 2016 in the #cljs-dev Slack:

mfikes: Curious if "outward" ^boolean propagation has been debated / rejected. Example: If simple analysis detects that ^boolean can be applied as in for functions like (defn f [x] (not x)). Perhaps automatically propagating type hints in this direction would be unwelcome by users?

dnolen: @mfikes: we have already have outward propagation for numerics which broke because of changes for code motion

dnolen: absolutely nothing against that



 Comments   
Comment by Mike Fikes [ 03/Apr/17 7:38 PM ]

The attached patch will handle cases like

(defn foo? [x] (or (string? x) (number? x)))

and

(def baz (fn ([x] 1) ([x y] 2)))

but it doesn't attempt to address the case of multi-arity defn s, as these are implemented by dispatching to multiple single-arity functions that could have different return types.

It adds a :inferred-ret-tag to the AST only because the arguments to or on line 18 in the patch are not reversed (only to allow explicit hints to override inferred hints, but perhaps that is overcomplicating things and the inferred tag could be dropped directly into :ret-tag.)

Bronsa helpfully pointed out in Slack that outward type hint propagation is avoided in Clojure because it breaks things like:

(defn foo [] 1)
(defn bar [] (String/valueOf (foo)))
(defn foo [] "foo")
(bar)

for which an analogy in ClojureScript would be:

(defn foo? [] true)
(defn bar [] (if (foo?) :t :f))
(defn foo? [] "")
(bar)

where the patch causes it to yield :f.

This could be viewed as a REPL-only concern (perhaps addressed by another compiler flag along the lines of :static-fns).

But, arguably the problem already exists today, with type hinting in code like the following affecting the compilation of bar:

(def foo? true)
(defn bar [] (if foo? :t :f))
(def foo? "")
(bar)
Comment by David Nolen [ 07/Apr/17 11:15 AM ]

Thanks will think about it





[CLJS-1981] ./script/bootstrap fails on FreeBSD 11.0 Created: 15/Mar/17  Updated: 07/Apr/17

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

Type: Defect Priority: Minor
Reporter: Duncan Bayne Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap, bug
Environment:

$ git rev-list HEAD | head -n 1
f7d08ba3f837f3e2d20ebdaf487221b18bb640c7

$ uname -a
FreeBSD x220 11.0-RELEASE-p1 FreeBSD 11.0-RELEASE-p1 #0 r306420: Thu Sep 29 01:43:23 UTC 2016 root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64

$ java -version
openjdk version "1.8.0_112"
OpenJDK Runtime Environment (build 1.8.0_112-b16)
OpenJDK 64-Bit Server VM (build 25.112-b16, mixed mode)



 Description   

When I run ./script/bootstrap, it fails when trying to invoke unzip. I think it's expecting GNU/Linux unzip, or something similar:

$ ./script/bootstrap
Usage: unzip [-aCcfjLlnopqtuvyZ1] [-d dir] [-x pattern] zipfile
The 'unzip' utility is missing, or not on your system path.



 Comments   
Comment by David Nolen [ 17/Mar/17 1:51 PM ]

I'm assuming you can install unzip no? If not then please provide a patch that generalize the bootstrap script. Thanks.

Comment by Duncan Bayne [ 06/Apr/17 2:08 AM ]

Yes, unzip is already installed. I'll attempt to generalize it.

If that fails - that is, if there is no suitable set of common arguments to GNU unzip and BSD unzip - I'll detect the unzip type and send the correct commands.

Comment by David Nolen [ 07/Apr/17 11:08 AM ]

This just doesn't seem like a problem. Just install unzip, just like you need to install Java etc.





[CLJS-2002] Don't throw when no *print-fn* is set Created: 07/Apr/17  Updated: 07/Apr/17

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


 Description   

Currently calling (enable-console-print!) causes a bunch of code to be retained in :advanced mode even if you never print.

While that is not ideal it doesn't cause runtime errors. Not calling it and trying to print however will throw an exception which will potentially break your app.

No *print-fn* fn set for evaluation environment

So we end up in a no-win situation for :advanced builds where a "forgotten" prn may break your app in production or "maybe" bloating your file size by retaining all the print-related things.

I think the no-print-fn condition should never throw, maybe just try to write a warning using console.log. Or just dropping the prn altogether.






[CLJS-2000] Don't log deprecation warnings on recursive calls to the same function with a different arity Created: 05/Apr/17  Updated: 06/Apr/17

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

Type: Defect Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: warning


 Description   

If a function has two arity's where one calls the other, then if that function is marked with `^:deprecated`, then compile warnings about deprecations will always be emitted while that function exists. E.g.

(defn ^:deprecated test-deprecated
  ([]
    (test-deprecated nil))
  ([a]
    nil))

produces these logs:

WARNING: my.test/test-deprecated is deprecated. at line 3 src/my/test/error.cljs

I think that only outside references to a deprecated function should warn here. Otherwise, it's impossible to deprecate a multi-arity function and still get clean compiles.






[CLJS-1995] Possible conflict with automatic aliases for JS modules Created: 02/Apr/17  Updated: 02/Apr/17

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

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


 Description   

https://clojurescript.org/guides/javascript-modules

The hello-es6 example uses a directory to apply :module-type to every file in that directory.

{:file "src" :module-type :es6}

This leads to src/js/hello.js being aliased to the js.hello ns which .cljs files can then :require.

Given a directory structure like this:

{:file "lib-a" :module-type :es6}
{:file "lib-b" :module-type :es6}

.
├── lib-a
│   └── js
│       └── hello.js
├── lib-b
│   └── js
│       └── hello.js

Leads to lib-b silently replacing lib-a as they both claim the js.hello name.

The same issue is present in closure-compliant libs but they typically follow some kind of manual namespacing (ie. goog.string, cljs.core, ...) which ES6/JS libs do not do (and do not even support given their use of relative imports vs absolute imports)

Not sure how to handle this but at the very least there should be some kind of warning that there is a conflicting alias.

Demo here: https://github.com/thheller/hello-es6-conflict






[CLJS-1991] ClojureScript compiler generates invalid JavaScript code when name contains emoji Created: 30/Mar/17  Updated: 31/Mar/17

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

Type: Defect Priority: Trivial
Reporter: Thai Pangsakulyanont Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

ClojureScript Compiler 1.9.495 (note: I selected 1.9.293 because 1.9.495, the currently released version, is not available)


Attachments: Text File CLJS-1991-Munge-non-identifier-unicode-characters.patch    

 Description   

Clojure (for JVM) handles code that contains emoji just fine, as shown in this example REPL session:

user=> (let [🖌 1] 🖌)
1

However, in ClojureScript, it generates an invalid JavaScript code:

app:cljs.user=> (let [🖌 1] 🖌)
#object[SyntaxError SyntaxError: Invalid or unexpected token]

This is because 🖌 is not a valid JavaScript name.

If this name should not be acceptable in ClojureScript as well, I think the compiler should raise an error during compilation instead of producing an invalid JavaScript code. However, it would be better such names are supported, because 1) Clojure works with such name and 2) it would allow writing code more expressively.

Actual result: Invalid JavaScript code is generated:

var 🖌_38518 = (1);

Expected result: Invalid JavaScript identifiers that's allowed in Clojure should be escaped:

var _1F58C__38518 = (1);


 Comments   
Comment by Thai Pangsakulyanont [ 31/Mar/17 8:24 AM ]

I added a simple patch that fixes this problem. Also a test is added to verify the fix. I have already signed the Clojure CA.


To give an example of how emojis might be useful in ClojureScript, I am creating a web page using ClojureScript and I want to define a color scheme. I could use the “artist palette” emoji to refer to the color palette.

(def 🎨 {:red    "#A4303F"
         :yellow "#FFECCC"
         :green  "#C8D6AF"
         :blue   "#254E70"})

This allows the code to be used like this:

[:div {:style {:color (🎨 :red)}} "Error!"]

One could also take advantage of the colors to make the code easier to understand:

(def ❤️ "#A4303F")
(def 💛 "#FFECCC")
(def 💚 "#C8D6AF")
(def 💙 "#254E70")

Current workaround: It is possible to use an emoji when referring to other namespaces, like this:

(ns mysite.core
  (:require [mysite.theme.colors :as 🎨]))

In this case, 🎨 can be used without generating a syntax error, because after compilation they will be replaced by mysite.theme.colors





[CLJS-1989] s/fdef expansion side effect fails when load cached source Created: 28/Mar/17  Updated: 28/Mar/17

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


 Description   

s/fdef has an expansion-time side effect that causes it to not operate properly if cached code is loaded.

To repro, have src/foo/core.cljs:

(ns foo.core
 (:require [clojure.spec :as s]))

(defn bar [x])

(s/fdef bar :args (s/cat :x number?))

Then:

$ java -cp cljs.jar:src clojure.main -m cljs.repl.nashorn
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.spec.test :as st] 'foo.core)
true
cljs.user=> (st/instrument)
[foo.core/bar]
cljs.user=> :cljs/quit

Now this has been cached. If you exit and try again, (st/instrument) fails:

$ java -cp cljs.jar:src clojure.main -m cljs.repl.nashorn
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.spec.test :as st] 'foo.core)
true
cljs.user=> (st/instrument)
[]


 Comments   
Comment by Thomas Heller [ 28/Mar/17 11:34 AM ]

s/def is also affected. It uses the cljs.spec/registry-ref atom which has a CLJ side (in cljs/spec.cljc) as well as a CLJS side (in cljs/spec.cljs). The CLJ is not populated when using the cache.





[CLJS-1830] sorted-map-by returns values for non-existing keys Created: 22/Oct/16  Updated: 24/Mar/17

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

Type: Defect Priority: Minor
Reporter: Eugene Pakhomov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Oracle Java 1.8.0_91
Apache Maven 3.3.9



 Description   

Run the following in a REPL:

(def a (sorted-map-by > 1 :a 2 :b))
(get a "a")

Expected: nil or exception

Actual: :a



 Comments   
Comment by Erik Assum [ 23/Mar/17 11:42 AM ]

So, the type of a here is `PersistentTreeMap`
It defines `get` through `-lookup`, which in turn has the following code (around line 7679)

(-lookup [coll k not-found]
    (let [n (.entry-at coll k)]
      (if-not (nil? n)
        (.-val n)
        not-found)))

Calling `(.entry-at a "a")` gives
`[1 :a]`, and `(.-val [1 :a])` gives `:a`

So the bug is somewhere in

(entry-at [coll k]
    (loop [t tree]
      (if-not (nil? t)
        (let [c (comp k (.-key t))]
          (cond (zero? c) t
                (neg? c)  (recur (.-left t))
                :else     (recur (.-right t)))))))
Comment by António Nuno Monteiro [ 23/Mar/17 11:46 AM ]

The problem is the comparator. This actually produces an error in Clojure, but JS will accept comparing "a" to a number

Comment by Erik Assum [ 24/Mar/17 7:36 AM ]
(def c (cljs.core/fn->comparator <))
#'foo/c
foo=> (c 1 "a")
0
foo=> (< 1 "a")
      ⬆
WARNING: cljs.core/<, all arguments must be numbers, got [number string] instead. at line 1
false
foo=>

Jupp so the problem is using `<` as a comparator since it's happy comparing strings and ints.

So this issue could be closed as "working as intended"?





[CLJS-1986] Support for ES6 generators Created: 19/Mar/17  Updated: 20/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: James Laver Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File generators.patch    
Patch: Code

 Description   

The attached patch adds support for ES6 generators (supported in node for a while now).

Generator functions can be declared through the :generator metadata that this patch adds support for. There is a new special token, `yield`.



 Comments   
Comment by António Nuno Monteiro [ 19/Mar/17 12:06 PM ]

I don't see how this patch is desirable. The ClojureScript compiler emits ES3 compatible code, which doesn't include generators.

In general, I think it's best to raise issues like this in the mailing list or the Clojurians slack before wasting time doing work that's not going to be accpeted.

Comment by James Laver [ 19/Mar/17 12:25 PM ]

Why is it that we emit ES3 compatible code in particular (I wasn't aware)? Google closure compiler has had support for compiling generators to ES3 for some time now https://github.com/google/closure-compiler/wiki/ECMAScript6

Comment by David Nolen [ 20/Mar/17 2:38 PM ]

I agree that this issues like this need some discussion first. Feel free to start one on the mailing list / Slack or IRC.





[CLJS-1977] defrecord should use murmur hashing like Clojure Created: 13/Mar/17  Updated: 17/Mar/17

Status: Reopened
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: None


 Description   

Due probably to an oversight, defrecord uses the older hash-imap method of hashing instead of the newer murmur hashing used by Clojure. Clojure does the equivalent of this:

(bit-xor (hash "full.class.name.of.Record") (hash-unordered-coll the-record))

This would also allow us to remove the internal functions hash-iset (which is already unused) and hash-imap (which is only used by records).






[CLJS-1975] Perf: Compare f with nil in Delay impl Created: 11/Mar/17  Updated: 11/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

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

 Description   

When a Delay has been realized, f is set to nil. We can avoid truth_ and not calls and directly compare with nil for a minor perf boost.

In script/noderepljs, this leads to these

(simple-benchmark [x (delay 1)] @x 1e9)
(simple-benchmark [x (delay 1)] (realized? x) 1e9)
(simple-benchmark [x (doto (delay 1) deref)] (realized? x) 1e9)

speeding up by 6%, 11% and 9%, respectively.






[CLJS-1974] Remove cljs.core/fixture1 and fixture2 Created: 11/Mar/17  Updated: 11/Mar/17

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

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

There are a couple of vars in cljs.core named fixture1 and fixture2 that appear to perhaps be leftovers from when tests were assertions included in the cljs.core namespace.

https://github.com/clojure/clojurescript/commit/43861f7bc0b7b4fb534167b0773e49ccb68e6b1e#diff-a98a037c6c098dd3707e861df3c2f5acR1893

These can probably be safely removed.






[CLJS-1970] Cannot infer target type for list/vector expressions Created: 08/Mar/17  Updated: 10/Mar/17

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

Type: Defect Priority: Minor
Reporter: Daniel Janus Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Ubuntu 16.04.2, Oracle JRE 8



 Description   

With

(set! *warn-on-infer* true)
enabled, attempting to compile functions like:

(defn foo [] (list))
(defn bar [] (vector))

results in a warning:

WARNING: Cannot infer target type in expression (. cljs.core/List -EMPTY) at line 2427 src/cljs/discann/core.cljs

WARNING: Cannot infer target type in expression (. cljs.core/PersistentVector -EMPTY) at line 2435 src/cljs/discann/core.cljs

The line number reported is totally unrelated to the line of code where the problematic fn appears.

Affects 1.9.456 and 1.9.494.






[CLJS-1972] cljs.nodejs/enable-util-print! identical to enable-console-print Created: 10/Mar/17  Updated: 10/Mar/17

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


 Description   

cljs.nodejs/enable-util-print! is identical to the cljs.core/enable-console-print! and should probably rather re-use than having duplicate code.






[CLJS-1971] Typos in docstring of require macro Created: 08/Mar/17  Updated: 08/Mar/17

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

Type: Defect Priority: Trivial
Reporter: Shogo Ohta Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

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

 Description   

The example code in the docstring looks not right:

cljs.user=> (doc require)
-------------------------
cljs.core/require
([& args])

...

Example:

  The following would load the library clojure.string :as string.

  (require '[clojure/string :as string])
nil
cljs.user=>





[CLJS-1969] Docstring for condp indicates IllegalArgumentException Created: 07/Mar/17  Updated: 07/Mar/17

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring

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

 Description   

The docstring for condp indicates IllegalArgumentException is thrown. Instead it should indicate Error (as does case's docstring, for example.)






[CLJS-1966] cljs.test assumes the output directory is '/out/' when determining the filename for a failed or errored test result. Created: 06/Mar/17  Updated: 06/Mar/17

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

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


 Description   

If your output directory is does not contain '/out/' in the path the code for getting the filename on failure for test results will return nonsense.

You can see here where we we make the assumption that '/out/' is used.
https://github.com/clojure/clojurescript/blob/2f8a1529955acc943ac8082ab5848b2cba54bc4d/src/main/cljs/cljs/test.cljs#L379






[CLJS-1965] letfn collisions across namespaces Created: 02/Mar/17  Updated: 02/Mar/17

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

Type: Defect Priority: Minor
Reporter: Jeremy R Sellars Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Affects 1.9.495 and earlier. Probably only affects browsers (not nodejs).


Attachments: Text File cljs-1965_wrap_letfn_statements.patch    

 Description   

If you `letfn` a fn with the same name in two namespaces, the wrong fn is used.

one.cljs
(ns hello-world.one)

(letfn [(answer [] "1")]
  (defn get-answer []
    (answer)))
two.cljs
(ns hello-world.two)

(letfn [(answer [] "2")]
  (defn get-answer []
    (answer)))
core.cljs
(ns hello-world.core
  (:require [hello-world.one]
            [hello-world.two]))

(println "one =>" (hello-world.one/get-answer))  ; one => 1
(println "two =>" (hello-world.two/get-answer))  ; two => 1      WHAT?!?

This issue seems to exist both on top-level `letfn`s and non-top-level `(let [] (letfn [...]))`.



 Comments   
Comment by Jeremy R Sellars [ 02/Mar/17 4:21 PM ]

This patch wraps `letfn` :expr and :statement forms in a function declaration (formerly, only :expr forms were wrapped).

I only did minimal testing. It fixes the code from the description.

Note: This is my first crack at the compiler and it is entirely possible I have not understood the entire problem.





[CLJS-1963] cljs.analyzer/load-core is called for every analyzed form Created: 01/Mar/17  Updated: 01/Mar/17

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

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


 Description   

In cljs.analyzer/analyze-form the load-core fn is called. load-core guards against doing its work multiple times. It then always calls (intern-macros 'cljs.core), which also checks whether it was called before. This ends up doing the checks very often. load-core should probably be called in a less frequent manner.

Performance impact is very minimal but I did a quick test in my work project and load-core is called 416671 times there (without cache) when 1 would be enough.






[CLJS-1962] Quick start guide unnecessarily uses rlwrap to run commands Created: 28/Feb/17  Updated: 28/Feb/17

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

Type: Defect Priority: Trivial
Reporter: Jeff Younker Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation
Environment:

Quick start guild



 Description   

At least one example command in the quick start guide uses rlwrap. The command rlwrap is unnecessary to run the command, and it is not installed on all systems (e.g. windows or OSX). This creates an unnecessary stumbling block on these systems, so rlwrap should be removed from the examples in the guide.






[CLJS-1959] under :nodejs target we should provide __dirname and __filename constants Created: 27/Feb/17  Updated: 27/Feb/17

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

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





[CLJS-1958] Macro :arglists include &form and &env Created: 27/Feb/17  Updated: 27/Feb/17

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

Type: Defect Priority: Trivial
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Minor issue, but nice to fix especially because of tooling. In lumo:

cljs.user=> (:arglists (get-in @lumo.repl/st [:cljs.analyzer/namespaces 'cljs.core$macros :defs 'when]))
([&form &env test & body])

I would like to filter out the first two arguments that are of course not needed.
A simple patch is here.

I can provide a patch once I figure out where is the right place to apply it. Of course if I am not doing it wrong.

Thanks!



 Comments   
Comment by Andrea Richiardi [ 27/Feb/17 12:26 PM ]

On second thought, if we put this filter in `load-analysis-cache!`, probably we will pay to much of a price for just a display/ide problem. The best thing I can think of is to filter the transit/json file before loading. So probably this is better to be inside `lumo`.

Comment by Andrea Richiardi [ 27/Feb/17 12:44 PM ]

After conversation with António Nuno Monteiro: this inconsistency might be solved with CLJS-1461 so this one here can be closed I guess.





[CLJS-1955] data_readers.cljc can't reference handlers in user code Created: 27/Feb/17  Updated: 27/Feb/17

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

Type: Defect Priority: Major
Reporter: Dustin Getz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

clojurescript 1.9.456-493



 Description   

data_readers.cljc:

{a/UserIdentity foo.core/user-identity}
(ns foo.core)
(defn user-identity [i] i)
(assert (= 1 #a/UserIdentity 1))

`CompilerException java.lang.IllegalStateException: Attempting to call unbound fn: #'foo.core/user-identity

Using a #' in data_readers.cljc results in a different error:
`java.lang.ClassCastException: clojure.lang.Cons cannot be cast to clojure.lang.Named`






[CLJS-1921] Certain files are compiled twice, leading to "Can't redefine a constant ..." errors Created: 30/Jan/17  Updated: 24/Feb/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: Viktor Magyari
Resolution: Unresolved Votes: 0
Labels: compiler
Environment:

windows, linux



 Description   

Repro:

Create a folder with the following files:

cljs.jar (the quick start uberjar)
src
  foo.cljc
  core.cljc

The contents shall be:

;; foo.cljc
(ns foo)

(def ^:const a 10)

;; core.cljc
(ns core
  (:require [foo]))

(def ^:const a 10)

In this folder, call java -cp src;cljs.jar clojure.main. In the repl, enter:

(require '[cljs.build.api :refer [build]])
(build "src" {:output-to "out/main.js" :output-dir "out" :verbose true})

If some conditions are met (see [1] and the explanations below), you'll see the compilation fail with "Can't redefine a constant at <...>".

NOTE: the easiest way to reproduce is to sort the value returned by cljs.closure/add-dependency-sources by a function of our choosing, (comp first :provides) for example. This can ensure that the last element will be a dependency which appears twice (see explanations below). However, in this case we have to use a hand-crafted classpath, which includes our patched clojurescript jar (+ its dependencies, + the src folder).

Cause:

NOTE: My understanding of the compiler internals (cljs.js-deps and cljs.closure in particular) is very superficial, but those familiar with it can probably follow along.

The journey starts at cljs.closure/add-dependency-sources, invoked in cljs.closure/build. The function takes some dependencies, converts them to a set, and adds some dependencies to it. Things to note here:

  • the :file entry in each dependency is nil [0]
  • the order of (seq (set inputs)) is "random" (or unspecified rather) [1]
  • the :source-file entry in each dependency is a java.io.File instance
  • the :source-file entry in each additional dependency from cljs.closure/find-cljs-dependencies is a java.net.URL instance
  • there are some dependencies (in the repro case at least) which were in inputs and were added as well with cljs.closure/find-cljs-dependencies (so they appear twice in the list)

The fact that some dependencies are java.io.File, some are java.net.URL instances may be the reason why some items appear multiple times in the set, but I haven't confirmed this.

Next, the computed dependencies are fed into cljs.js-deps/dependency-order, where they are passed to cljs.js-deps/build-index. This function reduces over each dependency (OUTER reduce), and reduces over each provided namespace (:provides entry, INNER reduce).

Here's the crucial part: the INNER reduce associates each namespace in :provides with the dependency, and the OUTER reduce then associates the :file entry with the dependency. But, since the :file entries are nil (see [0]), the value mapped to nil is overridden with each step in the OUTER reduce. Now, let X be a dependency which appeared twice (or at least twice, does not matter) in inputs. If X happens to be the last element of the dependencies (depends on [1]), then in the final map (when the OUTER reduce finishes) X will be mapped to nil. Therefore, in the final index, X will be present twice (once mapped to nil, once mapped to the namespace it provides).

Afterwards, these dependencies are compiled, and when the stars align (= sometimes it happens, sometimes it doesn't, both with parallel and non-parallel build) the compilation will fail with "Can't redefine a constant at <...>". This is most likely due to the fact that certain dependency (X), which has a ^:const var, was compiled twice (and some namespaces are compiled twice, this can be observed when :verbose is set to true).

I could reproduce the error on both windows (locally), and on linux (on a travis-ci server).



 Comments   
Comment by David Nolen [ 24/Feb/17 2:44 PM ]

Please verify that CLJS-1948 does or does not resolve this problem.





[CLJS-1933] Support CLJS browserless remote REPL from nodejs Created: 09/Feb/17  Updated: 09/Feb/17

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

Type: Enhancement Priority: Minor
Reporter: Mike Longworth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, remote, repl
Environment:

Dev machine OSX - build and Cursive editing and REPL
Remote: RaspberryPI - nodejs
compiler out-files shared between devices with OSXFUSE.



 Description   

I would like to develop clojurescript for a remote nodejs target compiling cljs and running a REPL on my development machine.
https://github.com/clojure/clojurescript/wiki/Remote-REPL suggests a way of doing this however:

!) I haven't managed to get this working.
2) I don't like that the solution relies identical absolute file paths for the compiler output, better to have matching relative paths.

I made a post to request help with this but haven't managed to resolve all the issues:
https://groups.google.com/forum/#!topic/clojurescript/Y4ajOcej8Qo

I have made some progress since the post:
1) I dug into the cljs.repl.node source and found I can stop the node hang I reported by specifying repl-env :debug-port, and get:

Clojure 1.8.0
Debugger listening on port 5002
ClojureScript Node.js REPL server listening on 5001
TypeError: Cannot read property 'nameToPath' of undefined
at Object.goog.require (repl:2:49)
at repl:1:-56
at Object.exports.runInThisContext (vm.js:54:17)
at Domain.<anonymous> ([stdin]:50:34)
at Domain.run (domain.js:221:14)
at Socket.<anonymous> ([stdin]:49:25)
at emitOne (events.js:77:13)
at Socket.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:146:16)
at Socket.Readable.push (_stream_readable.js:110:10)
TypeError: goog.provide is not a function
at repl:1:-56
at Object.exports.runInThisContext (vm.js:54:17)
at Domain.<anonymous> ([stdin]:50:34)
at Domain.run (domain.js:221:14)
at Socket.<anonymous> ([stdin]:49:25)
at emitOne (events.js:77:13)
at Socket.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:146:16)
at Socket.Readable.push (_stream_readable.js:110:10)
at TCP.onread (net.js:523:20)
To quit, type: :cljs/quit

This looks like some kind of path problem but I haven't managed to resolve it.

I did some investigations with my original relative-path setup to try and identify the issues:
1) I eliminated absolute paths from the compile output by disabling analysis caching.
2) I ran wireshark on the REPL port and found that absolute paths were being sent by the REPL, this currently makes the relative path option unworkable.

I have many gaps in my knowledge of the REPL operation at the moment and I don't know what the best approach is to getting a good solution for a browserless remote repl setup.



 Comments   
Comment by David Nolen [ 09/Feb/17 12:33 PM ]

It's probably going to be easier to discuss this issue in IRC or Slack first. There's just too many different issues piled into this one. Thanks.





[CLJS-1926] Changes to namespace metadata are not properly transferred to *ns* dynamic var Created: 02/Feb/17  Updated: 02/Feb/17

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

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


 Description   

A CLJS macro may want to access the metadata of the current ns via (meta *ns*).

Changes to the ns metadata are not reflected back the *ns* var when re-compiling a namespace, the metadata of the first compile will remain. This is due to the analyzer always calling create-ns but never updating the meta. It should probably be updated inside parse 'ns [1]. Clojure always resets the metadata via the ns macro.

One potential conflict is when a .clj and a .cljs file exist for the same namespace and both provide different metadata. Both platforms reseting the meta is probably not ideal, maybe we should vary-meta merge instead?

[1] https://github.com/clojure/clojurescript/blob/94b4e9cdc845c1345d28f8e1a339189bd3de6971/src/main/clojure/cljs/analyzer.cljc#L2312






[CLJS-1924] The compiler cannot infer the target type of "(. RecordName -prototype)" expressions Created: 01/Feb/17  Updated: 01/Feb/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler, extern


 Description   

Repro:

Place

(set! *warn-on-infer* true)

(defrecord Foo [])

anywhere in your source files, compile with :infern-externs true.

Expected:

Multiple warnings like:

  • WARNING: Cannot infer target type in expression (. Foo -prototype)
  • WARNING: Cannot infer target type in expression (. other__8838__auto__ -constructor)
  • WARNING: Cannot infer target type in expression (. user/Foo -getBasis)

There are also warnings for (. cljs.core/List -EMPTY), but this might be unrelated.






[CLJS-1918] case needs a type hint for keywords case when using *warn-on-infer* Created: 30/Jan/17  Updated: 30/Jan/17

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

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





[CLJS-1917] `case` doesn't handle matching against lists Created: 28/Jan/17  Updated: 28/Jan/17

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following works in Clojure but not ClojureScript

(let [foo '(:a :b)]
  (case foo
    '(:a :b) :works))





[CLJS-1864] timestamped source maps broken with Node Created: 29/Nov/16  Updated: 28/Jan/17

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

Type: Defect Priority: Minor
Reporter: Dmitr Sotnikov Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I'm using Figwheel with Node, and noticed a bug with timestamped source maps. When the :source-map-timestamp compiler flag is set, the sourceMappingURL is set to source.js.map?timestamp.

This works fine in the browser, but breaks in Node where files are loaded from the filesystem. It looks like a simple workaround would be to check if :target is :node and output something like source.js.timestamp.map instead, and use it directly as the value of sourceMappingURL.

Here's a change I made locally in cljs.compiler/emit-source-map that allows source maps to be resolved on Node when using timestamps:

emit-source-map
(defn emit-source-map [src dest sm-data opts]
     (let [timestamp (System/currentTimeMillis)
           filename (str (.getPath ^File dest)
                         (when (and
                                 (true? (:source-map-timestamp opts))
                                 (= (:target opts) :nodejs))
                           (str "." timestamp))
                         ".map")
           sm-file  (io/file filename)]
       (if-let [smap (:source-map-asset-path opts)]
         (emits "\n//# sourceMappingURL=" smap
                (string/replace (util/path sm-file)
                                (str (util/path (io/file (:output-dir opts))))
                                "")
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str
                    (if-not (string/index-of smap "?") "?" "&")
                    "rel=" timestamp)
                  ""))
         (emits "\n//# sourceMappingURL="
                (or (:source-map-url opts) (.getName sm-file))
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str "?rel=" timestamp)
                  "")))
       (spit sm-file
             (sm/encode {(url-path src) (:source-map sm-data)}
                        {:lines                   (+ (:gen-line sm-data) 2)
                         :file                    (url-path dest)
                         :source-map-path         (:source-map-path opts)
                         :source-map-timestamp    (:source-map-timestamp opts)
                         :source-map-pretty-print (:source-map-pretty-print opts)
                         :relpaths                {(util/path src)
                                                   (util/ns->relpath (first (:provides opts)) (:ext opts))}}))))


 Comments   
Comment by David Nolen [ 30/Nov/16 8:32 AM ]

Does Node.js have source map caching issues? The timestamp feature was created for caching issues present in web browsers.

Comment by Dmitr Sotnikov [ 30/Nov/16 8:39 AM ]

I tried it with :source-map-timestamp set to false, and source maps got out of sync when Cljs sources were reloaded.

Comment by David Nolen [ 30/Nov/16 10:01 AM ]

Okay. This issue will require more investigation into Node.js source mapping support before pursuing anything. As the behavior is understood, information should be added here.

Comment by Dmitr Sotnikov [ 30/Nov/16 2:56 PM ]

Sounds like a plan.

Comment by David Nolen [ 30/Nov/16 7:25 PM ]

OK I took a look at the implementation of source-map-support, it does indeed cache the source map. However the proposed idea here isn't comprehensive enough. We need to change all the places where :source-map-timestamp is used in the source code. Patch is welcome.

Comment by Dmitr Sotnikov [ 30/Nov/16 7:28 PM ]

Yeah, I noticed the key is used in a few places. I can definitely take a look at making a patch in the near future if the approach looks good to you.

Comment by Dmitr Sotnikov [ 28/Jan/17 12:39 PM ]

It looks like the approach of adding a timestamp introduces some problems. Generating unique file names would mean that old files have to be cleaned up somehow, since the new files will no longer overwrite them. Having to keep track of that isn't ideal. Perhaps it would be better to see if there's a way to prevent Node from caching the source maps.





[CLJS-1914] Investigate directly requiring CommonJS Node module into a ClojureScript namespace Created: 28/Jan/17  Updated: 28/Jan/17

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

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


 Comments   
Comment by David Nolen [ 28/Jan/17 10:20 AM ]

We would need to check that it exists. We would also need to parse out CommonJS requires to sort out the foreign libs inputs.





[CLJS-1913] Investigate slow reading / compilation of CLJC files Created: 27/Jan/17  Updated: 27/Jan/17

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1908] Improve error messages by using pr-str instead of str when printing objects Created: 26/Jan/17  Updated: 26/Jan/17

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   

Many error messages from ClojureScript include the invalid argument like this:

(throw (js/Error. (str "Doesn't support name: " x)))

If x is nil, then the error message produces is "Doesn't support name: " which is a bit mystifying to debug. If x was wrapped with pr-str then the error message would be the much more understandable: "Doesn't support name: nil".

If there's interest in this, then I can prepare a patch which wraps these kinds of errors with pr-str.






[CLJS-1907] Improve error message from cljs.reader/read-string when reading keyword with number first (e.g. :0seconds) Created: 26/Jan/17  Updated: 26/Jan/17

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

Type: Enhancement Priority: Trivial
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: errormsgs

Attachments: Text File CLJS-1907.patch    

 Description   

In read-keyword, if the keyword doesn't match symbol-pattern then re-matches* returns nil, and then the aget for token throws an error about null not being an object. The provided patch checks if the keyword matches, and if not throws an error with a descriptive error message that the keyword is not valid.

This hit our team when we were trying to deserialise some EDN, and we had keywords with leading numbers in them.

(defn read-keyword
  [reader initch]
  (let [token (read-token reader (read-char reader))
        a (re-matches* symbol-pattern token) ;; returns nil when token is `0s`
        token (aget a 0) ;; throws from aget on nil
        ns (aget a 1)
        name (aget a 2)]
    (if (or (and (not (undefined? ns))
                 (identical? (. ns (substring (- (.-length ns) 2) (.-length ns))) ":/"))
            (identical? (aget name (dec (.-length name))) ":")
            (not (== (.indexOf token "::" 1) -1)))
      (reader-error reader "Invalid token: " token)
      (if (and (not (nil? ns)) (> (.-length ns) 0))
        (keyword (.substring ns 0 (.indexOf ns "/")) name)
        (keyword token)))))


 Comments   
Comment by Daniel Compton [ 26/Jan/17 6:05 PM ]

I don't love using the _ side effect in the let binding, but the alternative makes the code a bit uglier and less readable.





[CLJS-1841] Check lib folder before compiling with cljsc, check for compilation success before running tests Created: 06/Nov/16  Updated: 25/Jan/17

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

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

Attachments: Text File CLJS-1841.patch     Text File CLJS-1841v2.patch    
Patch: Code

 Description   

If script/bootstrap hasn't been run, then running script/test returns the slightly cryptic error message:

Error: Could not find or load main class clojure.main

This patch checks if the lib folder exists and has files in it. If not it prompts the user to run script/bootstrap and exits.

This patch also adds a check for compilation success before running tests against the various VM engines.



 Comments   
Comment by Daniel Compton [ 25/Jan/17 5:57 PM ]

Add v2 patch.





[CLJS-1904] Reload support for JS Modules Created: 25/Jan/17  Updated: 25/Jan/17

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

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


 Description   

Process modules currently works by changing transforming compiler opts. This means the resulting compiler opts cannot be used to run JS module processing again. This issue is now clear in cljs/repl.cljc. The other issue is that JS module processing is the only time that ClojureScript is involved in the compilation of JS sources - prior to this we never to need to trigger JS compilation ourselves. Thus require + :reload doesn't work on JS namespaces.






[CLJS-1792] Can't load clojure.spec.test when clojure.test.check is unavailable Created: 23/Sep/16  Updated: 25/Jan/17

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

Type: Defect Priority: Minor
Reporter: Arne Brasseur Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:
Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
[org.clojure/clojure "1.9.0-alpha12"]
[org.clojure/clojurescript "1.9.229" :scope "provided"]


 Description   

Requiring clojure.spec.test results in an error, because it's looking for clojure.test.check.

(ns foo.bar
  (:require [clojure.spec.test]))
Caused by: clojure.lang.ExceptionInfo: No such namespace: clojure.test.check, could not locate clojure/test/check.cljs, clojure/test/check.cljc, or Closure namespace "clojure.test.check" in file file:/home/arne/.m2/repository/org/clojure/clojurescript/1.9.229/clojurescript-1.9.229.jar!/cljs/spec/test.cljs {:tag :cljs/analysis-error}

This problem goes away when adding org.clojure/test.check as a dependency.

This is not an issue in Clojure. An exception is only raised when calling a function that relies on test.check.



 Comments   
Comment by David Nolen [ 30/Sep/16 11:41 AM ]

This is not a bug per se, we can't do what Clojure does here. How to best handle is something to consider. Present a good idea and submit a patch.

Comment by Andrea Richiardi [ 24/Jan/17 4:46 PM ]

I went through cljs.spec.test and I have noticed that basically the [clojure.test.check :as stc] dependencies is there only for using the namespace for stc/ret and others. We could get rid of it and use the ns explicitely.

There is another require, which is [clojure.test.check.properties]. My guess is that it is there because of some side-effect (maybe it registers some spec?). I need some enlightenment here but I could work on a cut-off





[CLJS-1402] Source Mapping Closure Error Logger Created: 08/Aug/15  Updated: 24/Jan/17

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

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


 Description   

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



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:06 PM ]

I believe this will be fixed by CLJS-1901 either using `--source_map_input` or inlining source-maps into generated js files (CLJS-1902).





[CLJS-1898] JSValue should support metadata to fix source maps contents edge case Created: 23/Jan/17  Updated: 24/Jan/17

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

Type: Defect Priority: Trivial
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Dirac DevTools REPL relies on proper source-maps[1]. When I type "#js {}" into the REPL prompt, evaluation works, but generated javascript code snippet has wrong source map. Associated source-map's sourcesContent is something like #object[cljs.tagged_literals.JSValue 0x34d692f3 "cljs.tagged_literals.JSValue@34d692f3"]. And it is not what user entered. And this is not a valid cljs source I could parse again for code-completions.

The problem is somewhat subtle. When generating source-map info, the REPL code given a form tries to look at :source metadata[2] and if not present, it simply prints the form using pr. We get Clojure-style printing because JSValue does not implement ClojureScript printing as reported in CLJS-733.

Instead of implementing the rejected idea from CLJS-733. I focused on the first-case code path with :source metadata. It turned out that :source gets added if given form supports metadata protocols. Easiest idea was to change JSValue from deftype to defrecord.

I confirm that this fixed the REPL issue in Dirac. Now I'm getting back the original source code as entered by user.

Note that similar issue is present with #uuid and #inst tags as well. Those mostly work because their printer is compatible with both Clojure and ClojureScript. But fundamentally user might get back different source-map source than entered into the REPL. For example whitespace will be missing. This might cause confusion because line/column information reported by reader which might not match this artificial source-map content generated by printing.

I believe a fix would be to wrap UUID and Date forms created by tagged literals into something similar to JSValue and work with this wrapped value instead. Same with #queue. This would make cljs.tagged-literal code consistent, because every form originated from data readers would be inside a wrapper. It would enable arbitrary metadata on wrappers.

As a side note:
I noticed that reader's metadata get lost. They are not transplanted from form produced by the reader to generated tagged literal forms.
My second goal was to copy reader's metadata. Unfortunately this wasn't possible with current implementation of tools.reader. I can get reader metadata from passed form, but that is not the full picture, it only describes content after the tag was already consumed. I would need metadata from tag itself, passed into data-reader call somehow[3].

I don't know about any real-world problem caused by missing reader metadata on tagged literals, but I think ideally it should be fixed. For example when implementing error-reporting in cljs-oops, I had to handle this edge case[4], because line/column metadata was missing in some edge cases.

[1] https://github.com/binaryage/dirac/releases/tag/v0.8.0
[2] https://github.com/clojure/clojurescript/blob/960bb9b778190aa7359acb2f74cc61d452cef2ae/src/main/clojure/cljs/repl.cljc#L476
[3] https://github.com/clojure/tools.reader/blob/3f36a18a6c5d53a4fc718822131ee75625fd44dc/src/main/cljs/cljs/tools/reader.cljs#L834
[4] https://github.com/binaryage/cljs-oops/blob/d530a3cdf8cbab39bd2699c36caded4414224c50/src/lib/oops/reporting.clj#L29



 Comments   
Comment by Antonin Hildebrand [ 23/Jan/17 5:58 PM ]

I just noticed that some tests broke because of the defrecord change. My original implementation was using deftype with clojure.lang.IObj implementation. But it had a shortcoming of reusing val as the target for holding metadata. Which might not be IObj in general case. And also a separate implementation would have to be done for bootstrapped. Also I didn't want to add another field into deftype. I will leave it open for your suggestion how to properly implement this.

Comment by Antonin Hildebrand [ 24/Jan/17 2:28 PM ]

Another stab at it - this time I rely on the fact that val field must be a vector or a map. So it can carry metadata for JSValue without changing adding new fields to deftype.

Tests passing.





[CLJS-1900] Source maps for processed JavaScript modules Created: 24/Jan/17  Updated: 24/Jan/17

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

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


 Description   

Currently we don't emit source maps for JS modules converted into Google Closure namespaces.






[CLJS-1899] Local bindings conflict with global JS namespace Created: 24/Jan/17  Updated: 24/Jan/17

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

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


 Description   

Not sure if it's a bug or expected behaviour, but this:

(defn test-fn []
  (let [href     js/location.href
        location "123"]
    href))

gets compiled to this (not in advanced mode):

cognician.chat.ui.pages.insights.test_fn = (function cognician$chat$ui$pages$insights$test_fn(){
var href = location.href;
var location = "123";
return href;
});

and local location var shadows global I'm trying to access in location.href.

That sort of thing is expected and one should pay attention and work around stuff like this in JS, but in CLJS it's very confusing because nothing hints what am I doing wrong and why that code fails. I remember one of ClojureScript goals was to fix JS semantics, so maybe there's a way this might be addressed? At least throw a warning, maybe?



 Comments   
Comment by Thomas Heller [ 24/Jan/17 5:04 AM ]

This came up recently on the #cljs-dev slack channel. There is definitely a bug somewhere.

(let [href     js/location.href
      location "123"]
  href)

produces

var href_51444 = location.href;
var location_51445 = "123"; // << correct

So it works at the top level, but when inside a defn (and others) we get

(ns test)
(defn test-fn []
  (let [href     js/location.href
        location "123"]
    href))
test.test_fn = (function test$test_fn(){
var href = location.href;
var location = "123"; // << incorrect
return href;
});
Comment by David Nolen [ 24/Jan/17 7:27 AM ]

Taking a quick look it seems that maybe we aren't checking `:js-globals` consistently and often only looking at locals? Also now that externs inference is a thing we should probably compute `:js-globals` from all known externs instead of the obviously incomplete list we currently have in place.





[CLJS-1896] Externs file validation Created: 23/Jan/17  Updated: 23/Jan/17

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

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


 Description   

Invalid externs file will fail silently and not provide the expected externs inference warnings. We should try to catch such issues when we parse the file and throw an exception.






[CLJS-1886] RangedIterator should only be created from cljs.core.PersistentVector instances Created: 10/Jan/17  Updated: 22/Jan/17

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

Type: Defect Priority: Minor
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

RangedIterator uses cljs.core.PersistentVector internals and thus should not be created from other types of vectors. subvec does not follow this rule.



 Comments   
Comment by ewen grosjean [ 10/Jan/17 5:21 AM ]

This fixes a regression introduced by http://dev.clojure.org/jira/browse/CLJS-1855.

Comment by David Nolen [ 11/Jan/17 7:08 AM ]

Does this patch match the implementation approach in Clojure? Thanks.

Comment by ewen grosjean [ 11/Jan/17 11:08 AM ]

Yes, the Clojure approach is to check the type of the wrapped vector and to use a ranged iterator on instances of APersistentVector. It falls back to a more general iterator when the wrapped vector is not an instance of APersistentVector.
https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/jvm/clojure/lang/APersistentVector.java#L565

Comment by David Nolen [ 16/Jan/17 4:51 PM ]

Hrm that's not really the same since Clojure is checking for an interface. We probably want to replicate this behavior by using a marker protocol and testing for it with `implements?`.

Comment by ewen grosjean [ 17/Jan/17 5:39 AM ]

Clojurescript's ranged iterator goes quite deep into the persistent vector implementation details. Shouldn't marker protocol be used to mark abstract things ?

Comment by David Nolen [ 17/Jan/17 5:56 AM ]

That's not how we use marker protocols in ClojureScript so I'm not really concerned about that. This is something for expert implementers anyhow.

Comment by ewen grosjean [ 22/Jan/17 1:48 PM ]

Clojure checks for APersistentVector because the Clojure ranged iterator implementation only uses methods from APersistentVector.

The Clojurescript ranged iterator goes deeper into the persistent vector implementation details.

I can make a marker protocol named "APersistentVector", but if we want to follow the Clojure approach, then we probably want both PersistentVector and SubVec to implement it, just like in Clojure.

But this won't work because the Clojurescript ranged iterator implementation does not work on SubVec instances.

Two other possibilities are to not make SubVec to implement the marker protocol or to change the ranged iterator implementation.

Comment by ewen grosjean [ 22/Jan/17 2:29 PM ]

New patch attached. It introduces a new marker protocol named APersistentVector and makes PersistentVector to implement it





[CLJS-1891] UUID.toString can return non-string Created: 18/Jan/17  Updated: 20/Jan/17

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

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Summary

An incorrectly-constructed UUID can cause TypeError from str.

Detail

The cljs.core/uuid constructor stores its argument in the uuid field of the UUID deftype. See core.cljs line 10400.

It is assumed that this field is the string representation of the UUID, so it is also the return value of returned from UUID.toString. See core.cljs line 10377.

In correct code, the argument to cljs.core/uuid will never be anything but a string, so this is safe. But incorrect code can call cljs.core/uuid on any type, such as an Object. This leads to the following error when attempting to convert the UUID back into a string:

cljs.user=> (str (uuid #js{:foo "bar"}))
#object[TypeError TypeError: Cannot convert object to primitive value]
   Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (jar:file:~/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2782:22)
   cljs$core$str (jar:file:~/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2775:1)
nil

The error is thrown from arity-1 of cljs.core/str, which coerces its argument to a string by wrapping it in a JavaScript array and calling Array.join; see core.cljs line 2819

Presumably the implementation of Array.join is calling toString on the object. toString returns something which is not a string and cannot be coerced into a string, leading to the TypeError. This can be demonstrated more directly with:

cljs.user=> (str #js{:toString (fn [] #js{})})
#object[TypeError TypeError: Cannot convert object to primitive value]
   Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (jar:file:/Users/stuart/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2782:22)
   cljs$core$str (jar:file:/Users/stuart/.m2/repository/org/clojure/clojurescript/1.9.225/clojurescript-1.9.225.jar!/cljs/core.cljs:2775:1)
nil

Although the ECMAscript specifications are vague on the subject, it seems safe to say that it is incorrect for toString to return anything which is not a string.

Related

CLJS-1599, "UUIDs are not equal for upper/lower case strings," also relates to construction of UUIDs.



 Comments   
Comment by David Nolen [ 20/Jan/17 11:47 AM ]

Is just throwing a non-string value satisfactory?





[CLJS-1889] A lone ampersand `&` can be used to name a var, but throws when invoked. `(&)` Created: 15/Jan/17  Updated: 16/Jan/17

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

Type: Defect Priority: Trivial
Reporter: Aleksander Madland Stapnes Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I can use the symbol & to name something, but if I try to invoke it, the following exception is thrown:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:src/amp/core.cljs {:file #object[java.io.File 0x5e63cad "src/amp/core.cljs"]}, compiling/home/madstap/code/ampersand/build.clj:3:1)
at clojure.lang.Compiler.load(Compiler.java:7391)
at clojure.lang.Compiler.loadFile(Compiler.java:7317)
at clojure.main$load_script.invokeStatic(main.clj:275)
at clojure.main$script_opt.invokeStatic(main.clj:335)
at clojure.main$script_opt.invoke(main.clj:330)
at clojure.main$main.invokeStatic(main.clj:421)
at clojure.main$main.doInvoke(main.clj:384)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.lang.Var.invoke(Var.java:379)
at clojure.lang.AFn.applyToHelper(AFn.java:154)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:src/amp/core.cljs {:file #object[java.io.File 0x5e63cad "src/amp/core.cljs"]}
at clojure.core$ex_info.invokeStatic(core.clj:4617)
at cljs.compiler$compile_file$fn__3147.invoke(compiler.cljc:1406)
at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1376)
at cljs.closure$compile_file.invokeStatic(closure.clj:430)
at cljs.closure$fn__4204.invokeStatic(closure.clj:497)
at cljs.closure$fn__4204.invoke(closure.clj:493)
at cljs.closure$fn_4146$G4139_4153.invoke(closure.clj:389)
at cljs.closure$compile_sources$iter_43154319$fn_4320.invoke(closure.clj:829)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:688)
at clojure.core$next__4341.invokeStatic(core.clj:64)
at clojure.core$dorun.invokeStatic(core.clj:3033)
at clojure.core$doall.invokeStatic(core.clj:3039)
at cljs.closure$compile_sources.invokeStatic(closure.clj:826)
at cljs.closure$build.invokeStatic(closure.clj:1942)
at cljs.build.api$build.invokeStatic(api.clj:198)
at cljs.build.api$build.invoke(api.clj:187)
at cljs.build.api$build.invokeStatic(api.clj:190)
at cljs.build.api$build.invoke(api.clj:187)
at user$eval24.invokeStatic(build.clj:3)
at user$eval24.invoke(build.clj:3)
at clojure.lang.Compiler.eval(Compiler.java:6927)
at clojure.lang.Compiler.load(Compiler.java:7379)
... 11 more
Caused by: clojure.lang.ExceptionInfo: No method in multimethod 'parse' for dispatch value: & at line 5 src/amp/core.cljs {:file "src/amp/core.cljs", :line 5, :column 1, :tag :cljs/analysis-error}
at clojure.core$ex_info.invokeStatic(core.clj:4617)
at cljs.analyzer$error.invokeStatic(analyzer.cljc:628)
at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2871)
at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:2892)
at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3011)
at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3056)
at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3073)
at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1255)
at cljs.compiler$compile_file_STAR_$fn__3124.invoke(compiler.cljc:1325)
at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1159)
at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1316)
at cljs.compiler$compile_file$fn__3147.invoke(compiler.cljc:1396)
... 34 more
Caused by: java.lang.IllegalArgumentException: No method in multimethod 'parse' for dispatch value: &
at clojure.lang.MultiFn.getFn(MultiFn.java:156)
at clojure.lang.MultiFn.invoke(MultiFn.java:251)
at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:2867)
at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:2870)
... 43 more



 Comments   
Comment by Aleksander Madland Stapnes [ 16/Jan/17 1:59 AM ]

Better explanation as I can't seem to edit the description: https://gist.github.com/madstap/c77581185afa7fea8bbf2556f2d9fafe





[CLJS-1888] Seqs of PHMs and PAMs do not handle metadata correctly Created: 13/Jan/17  Updated: 13/Jan/17

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: metadata

Attachments: Text File CLJS-1888.patch    

 Description   

Metadata on parent seq ends up being passed to the next seq. Calling `empty` on a seq also ends up carrying metadata.

Examples:

(def s (with-meta (seq {:a 1 :b 2}) {:some :meta}))

(meta s) => {:some :meta} ;; Good
(meta (rest s))  => {:some :meta} ;; Bad, expected nil
(meta (next s))  => {:some :meta} ;; Bad, expected nil
(meta (empty s)) => {:some :meta} ;; Bad, expected nil





[CLJS-1885] assoc should return an array-map when passed a nil collection Created: 09/Jan/17  Updated: 09/Jan/17

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

Type: Enhancement Priority: Major
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

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




[CLJS-1884] Give a chance to MetaFn to be removed by closure under :advanced optimization Created: 09/Jan/17  Updated: 09/Jan/17

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

Type: Enhancement Priority: Major
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Unresolved Votes: 0
Labels: None

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




[CLJS-1574] CLJS string equivalence is very slow in Chrome Created: 16/Feb/16  Updated: 03/Jan/17

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

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

Google Chrome 48.0.2564.109 on Mac OS X 10.11.3
Safari 9.0.3 (11601.4.4) on Mac OS X 10.11.3



 Description   

Clojurescript's equivalence for strings in Google Chrome is ~1000 times slower than equivalent javascript functionality, and ~1000 times slower than the same function in Safari.

Google Chrome
js equiv:  0.005 seconds
cljs equiv:  1.898 seconds
Safari
js equiv:  0.005 seconds
cljs equiv:  0.006 seconds
(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(deftest test-js-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (js* "~{} == ~{}" str1 str2)
                  "js equivalence")
        end   (now)
        ]
    (println "js equiv: " (delta end start))))

(deftest test-cljs-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (= str1 str2)
                  "cljs equivalence")
        end   (now)
        ]
    (println "cljs equiv: " (delta end start))))


 Comments   
Comment by Stephen Nelson [ 16/Feb/16 6:06 PM ]

This bug only occurs when cljs.pprint has been required.

Comment by Stephen Nelson [ 16/Feb/16 6:38 PM ]

After a whole lot of binary search, here's a minimal reproduction. When cljs.pprint is loaded it constructs write-option-table. It seems that constructing a hash map with the keys :added and :ns causes a call to (= :added :ns), which is sufficient to cause string equality to become extremely slow.

(ns hello-world.core)

(enable-console-print!)

(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(defn test [] 
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (= str1 str2)
        end   (now)
        ]
      (println "cljs equiv: " (delta end start))))

(test)

(= :added :ns)

(test)
Comment by Peter Schuck [ 17/Feb/16 4:50 PM ]

Is the ClojureScript compiled with options :optimizations :advanced or :static-fns true? Compiling ClojureScript without those options results in call indirection for all function calls which might explain the slowdown. See http://swannodette.github.io/2015/03/16/optimizing-clojurescript-function-invocation/ for more information.

Comment by Stephen Nelson [ 17/Feb/16 9:06 PM ]

This happens with :advanced, :simple, and without optimisations. Stepping through the generated javascript seems to indicated that the slow down comes from the VM's internal function dispatch. Regardless, I don't think that the extra function calls related to dynamic dispatch in clojurescript could add minutes of overhead per call. Note that the test case above only uses 128k of string data, the case where I encountered this issue first used ~512K and took about 5 minutes to complete a single function call.

Comment by Francis Avila [ 17/Feb/16 9:14 PM ]

I have reproduced this in Chrome for Linux, on :none and :advanced optimization levels using different test code. I verified the result of the compare so the JIT won't optimize it away and I used performance.mark() and performance.measure() for timing, although none of this should have mattered.

Every subsequent string compare after the first -equiv-invoking use of equal is significantly slower for no reason I can see. There are no intermediate GCs or anything to suggest that it should be slower--it just takes longer! The only thing I can think of is maybe the keyword-equals triggers a deopt because it makes the equal-function megamorphic, but the code is run so few times that there should not be jit optimizations kicking it at all. Also, the keyword-compare itself remains fast.

I suspect a Chrome/v8 bug. Possibly a different internal string representation kicks in for some reason which has a slower compare? This is only an issue for largish, non-constant strings, and the slowdown is proportional to string size. I'm going to try and reproduce this with pure JS.

Comment by Francis Avila [ 18/Feb/16 12:33 AM ]

All you need to reproduce this is to use the strict equality operator in a function body non-monomorphically. Subsequent executions of the function with strings (at least) which have not been compared before the polymorphic call will be very slow.

If you replace strict equality (triple-equal) with normal equality (double-equal), this issue goes away.

This is clearly a Chrome/v8 bug, but I'm not sure where to report it.

Minimal pure-javascript reproduction:

function str(size) {
  var s = "";
  for (var i = 0; i < size; i++) s += "a";
  return s;
}

function eq(x, y) {
  performance.mark("start");
  x === y; // No slowdown if use == instead
  performance.mark("end");
}

function print_measures() {
  performance.getEntriesByType("measure")
  .forEach(entry => console.log(entry.name, entry.duration));
}

var s1 = str(64 * 1024);
var s2 = str(64 * 1024);
var s3 = str(64 * 1024);

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(0, 0);
performance.measure("eq(0, 0)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

print_measures();

Results with Chrome 48.0.2564.109 (64-bit) on a slow iMac with OS X 10.11.3

eq(s1, s2)   4.465000000000003     // fast string compare
eq(0, 0)     0.009999999999990905  // break monomorphism of eq()
eq(s1, s3) 259.665                 // Now string compare is slow
eq(s1, s2)   0.019999999999924967  // Repeated call still fast
eq(s1, s3) 232.52499999999998      // ... but not from after the polymorphic invoke
Comment by Francis Avila [ 22/Feb/16 3:14 AM ]

Issue added to v8: https://bugs.chromium.org/p/v8/issues/detail?id=4773

Comment by Francis Avila [ 03/Jan/17 9:23 AM ]

Update: This bug was fixed upstream today, so it should start showing up in releases eventually.





[CLJS-1881] Improve cljs.core/distinct perf by using transient map Created: 25/Dec/16  Updated: 29/Dec/16

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

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

Attachments: Text File cljs-1881-transient-in-distinct.patch    
Patch: Code

 Description   

Current implementation of cljs.core/distinct uses persistent set. This patch improves performance by ~10%-33% by using transient map instead. Mirror Clojure task CLJ-2090

10 elements
(reduce + 0 (distinct coll))     12.360220502805724 µs => 9.504153281757874 µs (-23%)
(transduce (distinct) + 0 coll)  7.689213711227641 µs => 5.3549045227207 µs (-30%)
100 elements
(reduce + 0 (distinct coll))     136.43424283765356 µs => 106.66990187713321 µs (-21%)
(transduce (distinct) + 0 coll)  73.05427319211107 µs => 48.737280701754386 µs (-33%)
1000 elements
(reduce + 0 (distinct coll))     1.1207102908277415 ms => 919.8952205882359 µs (-17%)
(transduce (distinct) + 0 coll)  677.2834912043312 µs => 482.79681467181547 µs (-28%)
10000 elements
(reduce + 0 (distinct coll))     4.777295238095228 ms => 4.3203448275862115 ms (-9%)
(transduce (distinct) + 0 coll)  2.889020114942531 ms => 2.44890487804879 ms (-15%)

Benchmarking code: https://gist.github.com/tonsky/258c3d715e6a485522f7ba5e663624fd






[CLJS-324] cljs.core/format Created: 24/Jun/12  Updated: 27/Dec/16

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

Comment by Ikuru Kanuma [ 27/Dec/16 6:52 PM ]

Would much appreciate a DCE compliant implementation of this, as I happen to be maintaining a library that broke because of this removal.
Has anyone worked on this since?

Comment by Gary Fredericks [ 27/Dec/16 7:31 PM ]

I thought about trying to port java.util.Formatter just for fun.

I can't figure out what you mean by "DCE compliant".

Comment by Ikuru Kanuma [ 27/Dec/16 8:33 PM ]

Oh, DCE = dead code elimination.
I thought it was a common abbrev. from this mail:
https://groups.google.com/forum/#!topic/clojure-dev/URrnaU6KWQk

I would definitely send my cheers if you do decide to put in the time!





[CLJS-1876] Faster PersistentVector, Subvec and ChunkedSeq reduce. Created: 19/Dec/16  Updated: 19/Dec/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: performance

Attachments: File benchmarks.cljs     Text File CLJS-1876.patch    
Patch: Code

 Description   

This patch improved the performance of the 2-arity `-reduce` of PersistentVectors as well as the 2 and 3 arity versions of `-reduce` for `Subvecs` and `ChunkedSeqs`.

For large (> 32) `Subvecs` and `ChunkedSeqs` saw a 7x speed up in V8 and up to 11x in JavascriptCore. Smaller versions saw no change.

The 2-arity version of PersistentVector `-reduce` was also improved ~4x and ~7x in V8 and JavascriptCore respectively.

In the benchmarks below all runs where (N <= 32) were run 1,000,000 times. For the larger collections 100,000 iterations were made.

PersistentVector 3-arity reduce (no code was changed)

N master (v8) patched (v8) master (jsc) patched (jsc)
0 44 44 17 19
1 121 102 29 32
2 151 133 35 37
4 219 199 50 50
8 360 336 79 79
16 606 588 130 131
32 1124 1109 243 250
100 329 338 75 76
1000 3235 3317 704 725
10000 32960 33575 7365 7316

Persistent Vector 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 65 41 29 12
1 96 58 49 42
2 147 66 87 45
4 246 85 113 44
8 446 142 202 69
16 829 276 362 98
32 1627 506 693 132
100 534 154 236 41
1000 5442 1568 2321 329
10000 58396 15386 26162 3406

ChunkedSeq 3-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 57 69 93 88
1 54 60 31 26
2 68 63 27 28
4 94 91 37 39
8 146 152 59 58
16 272 266 121 107
32 479 526 170 174
100 1186 165 459 39
1000 11760 1539 4547 334
10000 121986 16080 48639 3384

ChunkedSeq 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 62 63 96 97
1 23 31 16 19
2 35 40 20 17
4 68 70 26 29
8 116 120 49 47
16 232 223 83 89
32 467 444 156 158
100 1123 164 417 39
1000 12328 1659 4516 327
10000 126570 15940 47435 3330

Subvec 3-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 67 67 51 34
1 185 71 100 35
2 296 84 160 44
4 514 100 259 52
8 958 156 405 77
16 1878 295 733 138
32 3668 565 1433 139
100 1164 165 462 36
1000 12596 1600 4798 337
10000 122919 16108 48093 3511

Subvec 2-arity reduce

N master (v8) patched (v8) master (jsc) patched (jsc)
0 73 49 35 22
1 169 58 75 48
2 289 70 117 54
4 544 103 197 56
8 961 159 378 74
16 1852 288 697 103
32 3644 514 1425 145
100 1245 147 441 39
1000 12065 1537 4556 333
10000 122519 15600 46324 3370

The source code for the benchmarks is attached.






[CLJS-1866] RangedIterator performance tweaks Created: 08/Dec/16  Updated: 19/Dec/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Thomas Mulvaney
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-1866.patch     Text File CLJS-1866-updated.patch     Text File CLJS-1866-updated.patch    
Patch: Code

 Description   

The attached patch simplifies and speeds up the RangedIterator.

The benchmarks were run using the following function to test vector iteration:

(defn consume-iterator
  [v]
  (let [iter (-iterator v)]
    (loop []
      (when (.hasNext iter)
        (.next iter)
        (recur)))))

A series of "simple-benchmarks" were setup as follows:

(simple-benchmark [v (into [] (range N))] (consume-iterator v) I)

Where 'N' and 'I' were values from the 'Vector Size' and 'Iterations' columns of the table below .

Vector Size Iterations V8 Speed [msec] (master) V8 Speed [msec] (patch) JSC Speed [msec] (master) JSC Speed [msec] (patch)
1 100,000 15 11 13 7
2 100,000 14 10 7 4
4 100,000 18 10 9 5
8 100,000 27 12 14 6
16 100,000 43 17 19 9
32 100,000 74 24 37 15
100 100,000 217 59 105 45
1000 100,000 2008 524 1032 392
10,000 100,000 20390 5856 10249 4178
100,000 10,000 20334 5324 10324 4387

Javascript engine versions used:

  • V8 version 5.1.281.47
  • JSC version Unknown

The RangedIterator constructor function `ranged-iterator` was also made private.



 Comments   
Comment by David Nolen [ 16/Dec/16 2:04 PM ]

Let's get a patch with the performance change without altering the constructor first. Thanks.

Comment by Thomas Mulvaney [ 17/Dec/16 7:15 AM ]

Rebased and constructor no longer private.

Comment by David Nolen [ 17/Dec/16 9:59 AM ]

Sorry for not being clear. Leave the fields of the deftype alone even if we aren't using them for now. We want the performance enhancements without changing the API at all.

Comment by Thomas Mulvaney [ 19/Dec/16 5:58 AM ]

Thanks that makes sense. Fixed in this patch.





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

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

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


 Description   

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

Work in progress: https://github.com/frenchy64/clojurescript/tree/reflect






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

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

Type: Defect Priority: Minor
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None


 Description   

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

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

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


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

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


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

From @ykomatsu on Github:

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

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

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

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

Comment by Dusan Maliarik [ 09/Dec/16 2:15 PM ]

Would anyone from the team please look at the patch? Thank you

Comment by David Nolen [ 09/Dec/16 6:22 PM ]

Please attach a patch to the ticket for review. Linking out of JIRA is not desirable. Thanks.





[CLJS-1865] Google Closure Compiler in JavaScript Created: 06/Dec/16  Updated: 06/Dec/16

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

Type: Enhancement Priority: Trivial
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Google released its JavaScript version of the Closure Compiler – "this allows Closure Compiler to run entirely in JS. Java is not required":

Switching to the JS compiler means JS devs coming to ClojureScript will be able to use the tools they're familiar with a simplify the onboarding docs on the website.

NB: I discovered this while experimenting with using ClojureScript with Polymer:






[CLJS-1863] :reload/:reload-all issue with .cljc runtime/macro nses Created: 29/Nov/16  Updated: 29/Nov/16

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

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


 Description   

Brandon Bloom discovered an issue where a ns that is used both from runtime and for macros (where the macros are self-required) won't respect `(require ... :reload/:reload-all)`.



 Comments   
Comment by David Nolen [ 29/Nov/16 3:59 PM ]

Whatever we do needs to be copied over into a bootstrapped, but that's a separate issue.





[CLJS-1862] allow NodeJS's NODE_MODULES to be set as a REPL option Created: 28/Nov/16  Updated: 29/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Marc Daya Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: nodejs, repl

Attachments: Text File 65.patch    
Patch: Code

 Description   

The NodeJS REPL that ships with ClojureScript seems to assume that all NodeJS modules are installed globally, or that node's NODE_PATH environment variable is set for the process that starts the REPL (e.g. CIDER). Allowing this to be set as a REPL option make it possible for modules to be installed and made available to the REPL by build tooling, eliminating manual steps by the user and improving repeatability.



 Comments   
Comment by David Nolen [ 28/Nov/16 4:26 PM ]

Thanks. Have you submitted your Clojure CA yet?

Comment by Marc Daya [ 29/Nov/16 2:02 PM ]

It has just been filed.





[CLJS-901] Protocolize compiler access to the file system Created: 03/Dec/14  Updated: 18/Nov/16

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: 3
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-1854] Self-host: Reload ns with const Created: 16/Nov/16  Updated: 16/Nov/16

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

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


 Description   

Bootstrapped ClojureScript fails to allow you to reload a namespace containing a constant.

To reproduce, evaluate the following forms in a REPL:

(require 'cljs.js)

(def st (cljs.js/empty-state))

(cljs.js/eval st
  '(require (quote foo.core))
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 1)"}))}
  prn)

(cljs.js/eval st
  '(require (quote foo.core) :reload)
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 2)"}))}
  prn)

The expectation is that the :reload directive in the last require will allow the namespace to be loaded with the const def being re-defined.

Instead, you get the following in the eval callback:

{:error #error {:message "Could not eval foo.core", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't redefine a constant at line 1 ", :data {:file nil, :line 1, :column 15, :tag :cljs/analysis-error}}}}

Note: This has probably been a defect in bootstrapped ClojureScript for quite a while (maybe forever). In particular, it is not a regression introduced with the new require capability (CLJS-1346).

FWIW, Planck has been working around this (and violating public API), manipulating cljs.js/*loaded* via its require REPL special, essentially purging portions of the analysis cache when reloading: https://github.com/mfikes/planck/blob/1.17/planck-cljs/src/planck/repl.cljs#L329-L348






[CLJS-1852] explain-data problems don't provide resolved symbols under :pred Created: 15/Nov/16  Updated: 15/Nov/16

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

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


 Description   

Same as http://dev.clojure.org/jira/browse/CLJ-2059 which has a patch.






[CLJS-1846] Range issues Created: 10/Nov/16  Updated: 15/Nov/16

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

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

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

 Description   

Problem
There are a number of bugs with Range which occur when the step size is 0 or where negative.

Examples

cljs.user=> (count (range 10 0 0))
-Infinity  ;Expect Infinity

cljs.user=> (nth (range 10 0 -1) -1)
11 ; Expected IndexOutOfBounds

cljs.user=> (take 5 (sequence identity (range 0 10 0)))
() ; Expected (0 0 0 0 0)

cljs.user=> (into [] (take 5) (range 0 10 0))
[] ; Expected [0 0 0 0 0]


 Comments   
Comment by David Nolen [ 10/Nov/16 4:37 PM ]

This patch is headed in the right direction but it needs to be more vigilant about performance. I'm more than happy to talk it over via IRC or Slack. Thanks!

Comment by Thomas Mulvaney [ 15/Nov/16 8:24 AM ]

Updated patch with performance tweaks.

  • Added the ^boolean annotation to the `some-range?` helper.
  • Removed calls to methods of Range where possible.
  • Improved 2-arity reduces performance over master significantly by replacing the call to ci-reduce.




[CLJS-1764] Double warning for undeclared Var Created: 26/Aug/16  Updated: 14/Nov/16

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

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

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

 Description   

A regression occurred where an undeclared Var in a {{require}}d file causes two diagnostics:

$ more src/foo/core.cljs
(ns foo.core)

(def x 2)

abc
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.227.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52749
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.227"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.211.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56704
To quit, type: :cljs/quit
cljs.user=>  *clojurescript-version*
"1.9.211"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit


 Comments   
Comment by David Nolen [ 16/Sep/16 2:04 PM ]

If somebody wants to do a git bisect to sort this one out, that would be awesome

Comment by António Nuno Monteiro [ 07/Nov/16 9:34 AM ]

Only seems to happen at the REPL

Comment by António Nuno Monteiro [ 13/Nov/16 3:47 PM ]

Patch with fix.

This only happened when `require`ing at the REPL. Required namespaces ended up being analyzed twice, once in `cljs.repl` and once in `cljs.closure`. The patch adds wraps compiling these NSes in `cljs.closure` in a `cljs.analyzer/no-warn`.

Comment by David Nolen [ 14/Nov/16 9:24 AM ]

How will this not effect non REPL cases?

Comment by António Nuno Monteiro [ 14/Nov/16 9:29 AM ]

I just now realized that it will probably affect those cases as well, although the `add-dependencies` function seems to (currently) only be used in `cljs.repl`. What other approach should I try? Restrict the cases where we

*analyze-deps*
at the REPL?

Comment by Thomas Heller [ 14/Nov/16 9:51 AM ]

FWIW I don't think this is related to the REPL at all.

I have been seeing doubled warnings for a while now in shadow-build but never bothered to find you why.

abc

(defn x [y] xyz)

Will always warn twice about "xzy" but only once for "abc", doesn't matter if a REPL is involved or not.





[CLJS-1822] Use `:file-min` when processing JS modules with advanced optimizations Created: 15/Oct/16  Updated: 11/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Currently the `:file-min` option is ignored if a foreign lib is a JS module. Certain libraries produce 2 different artifacts, one which has development time checks, and a production-ready bundle which doesn't.

This patch proposes that `:file-min` in a JS module be fed to the Google Closure Compiler (instead of `:file`) when processing JS modules in `simple` or `advanced` compilation. This way, the development bundle of a JS module can be used with `:optimizations :none`, while the production-ready bundle can be used when compiling projects for production use.



 Comments   
Comment by António Nuno Monteiro [ 05/Nov/16 12:05 PM ]

Attached patch with fix and test.





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

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
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-1127] validate compiled file written to disk Created: 16/Mar/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
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: 08/Nov/16

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

Type: Enhancement Priority: Minor
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-1070] top-level boolean inference does not work Created: 28/Feb/15  Updated: 08/Nov/16

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

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


 Description   

Problem for using boolean Closure defines



 Comments   
Comment by Francis Avila [ 30/Mar/15 12:02 PM ]

I am unsure if this is the same issue, but forms like ^boolean (js/isFinite n) also do not seem to analyze correctly: if, and, and or will still emit a call to truth_.





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

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

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

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



 Description   

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

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

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

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



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

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

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

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

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

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

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

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

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

to be true and emit a "return".

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

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

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

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

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

This issue occurs for me even without a let.

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

gives me

#object[SyntaxError SyntaxError: Unexpected token return]




[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
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-773] Use unchecked-*-int functions for real 32-bit math Created: 26/Feb/14  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
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-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 08/Nov/16

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

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


 Description   

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

Example:

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

Whereas foo.getBarRight expands to something like

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

foo.getBarWrong on the other hand expands to

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





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

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

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


 Description   

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






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

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
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-375] loop doesn't seem to preserve tag information as evidenced by extra cljs.core.truth_ calls Created: 06/Sep/12  Updated: 08/Nov/16

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

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





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

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

Type: Enhancement Priority: Minor
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-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 08/Nov/16

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

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

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    

 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJS-1207] Emit a warning if multiple resources found for a ClojureScript namespace Created: 15/Apr/15  Updated: 08/Nov/16

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

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


 Description   

We should emit a simple warning if a namespace doesn't not appear to be unique on the classpath.






[CLJS-1195] generic reusable command line argument parsing for REPLs Created: 10/Apr/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Jason Courcoux
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

REPLs are more or less started in the same way and all the builtin ones provide a -main entry point. We should supply reusable command line argument parsing that any REPL can use to get standard command line driven start.



 Comments   
Comment by Jason Courcoux [ 30/Sep/16 3:27 AM ]

Just wanted to capture my initial thoughts in case I'm going down the wrong road, or overthinking it and someone wants to point me in a different direction. I can see the following options for parsing the command line arguments - in no particular order:

1) Reuse a third party such as clojure/tools.cli

  • Less to maintain within the ClojueScript codebase itself.
  • Supports GNU option parsing conventions
  • Extra dependency - Guessing this is a a definite no for various reasons, but don't want to assume anything.
  • Is it over complicated for our needs here?

2) Reuse something in the java platform - looks like there is a class sun.tools.jar.CommandLine which has very basic functionality for parsing command line arguments.

  • Already in the Java platform, although I believe this is probably only in the JDK so probably no good for this use case.
  • Very limited support - would be easier to replicate the functionality in clojure code.

3) Use the clojure reader to just read in clojure data

  • Nice and simple, and reusing something that already exists
  • Arguments would be in the same format as they are now
  • No validation of parameters passed in.

4) Custom parsing of arguments - wondering if we could do something with clojure spec and allow repls to pass a spec which could be used to infer how to parse/validate the data (e.g. for port number is it an int or string).

  • Leveraging spec gives repls a mechanism to specify constraints, and can get clear errors out
  • Can be more flexible in the arguments accepted - i.e. --port "9000" and --port 9000 could both be valid
  • I've not done much with spec so although I think this sounds feasible I'm not 100%

I think I'm going to explore option 4, and I'll update as I go.

Comment by David Nolen [ 30/Sep/16 6:09 AM ]

Thanks for writing this up. 1) tools.cli is not a bad idea but do we need it. 3) seems Clojure-y - we just want typical CLI support. 4) Clojure 1.9 is alpha we don't want a dependency on this.

My original thought was to just replicate what clojure.main does - I don't see why we need anything more.

Comment by Jason Courcoux [ 30/Sep/16 9:45 AM ]

Thanks for the quick response. I've had a look at clojure.main, and as far as I can tell it doesn't do anything in the way of generic parsing of arguments - The main function dispatches based on some known options (repl/main/help etc) and passes the rest of the arguments through - in each case it just binds the arguments to command-line-args which may or may not get parsed/accessed at a later point either during startup, or from the repl session - neither of these seem to be what this Jira is asking for, unless I've misunderstood.

Just so I'm 100% on what's being asked here - this ticket is for parsing repl environment options, i.e. for the browser repl the options would be host/port/working-dir/serve-static etc, and the parsing would need to handle strings/int/boolean values etc.

I'm conscious you're probably very busy, I'm almost certainly missing something, and don't want to take up too much of your time, so if you tell me it's there in clojure.main I'll keep digging until I find it.

Comment by David Nolen [ 30/Sep/16 10:48 AM ]

We're not at all interested in exposing all the options via command line flags. The first step is simply mirroring Clojure's REPL options that make sense. For all the CLJS REPL specific stuff a flag which takes string of EDN or an EDN config file is fine.





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

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

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





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

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
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-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 08/Nov/16

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

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


 Description   

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



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

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





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

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

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

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

 Description   

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



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

A patch that caches upstream dependencies in the compiler env.

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

Type: Defect Priority: Minor
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-1136] Initial require fails to fully load added symbols Created: 17/Mar/15  Updated: 08/Nov/16

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

Type: Defect Priority: 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-1134] Lift protocols from cljs.closure into cljs.protocols ns Created: 17/Mar/15  Updated: 08/Nov/16

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

Type: Task Priority: Minor
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-1133] REPL require results in warnings to be emitted twice Created: 17/Mar/15  Updated: 08/Nov/16

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

Type: Defect Priority: 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-1129] :modules tutorial for wiki Created: 16/Mar/15  Updated: 08/Nov/16

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

Type: Task Priority: 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: 08/Nov/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
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-1415] Handling JSDoc param name [x] optional syntax Created: 10/Aug/15  Updated: 08/Nov/16

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

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





[CLJS-1412] Add JSDoc type information to individual IFn methods Created: 10/Aug/15  Updated: 08/Nov/16

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

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


 Description   

Propagate user supplied docstring type information to the various fn arities so that more code may be checked.






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

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

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


 Description   

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






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

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

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


 Description   

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

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

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

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

Analysis

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

Record literals

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

EDIT: Record literals can also go into the constant pool

cljs.reader

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

Related Tickets



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

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

I'm all for this enhancement.





[CLJS-1315] Warning on Google Closure enum property access with / Created: 18/Jun/15  Updated: 08/Nov/16

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

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


 Description   

Edge case in / usage, EventType/CLICK does not trigger a warning. Foo/bar always means that Foo is a namespace, it cannot be used for the static field access pattern common in Java as there's no reflection information in JavaScript to determine this.






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

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

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

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

 Description   

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



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

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

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

Thanks will review.

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

cljs-1300.patch no longer applies on master





[CLJS-1286] REPL environment should be able to provide advice if source mapping fails Created: 23/May/15  Updated: 08/Nov/16

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

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


 Description   

For example browser REPL will often need users to supply :host-port, :host, and :asset-path in order to correctly parse files from stacktraces.






[CLJS-1271] Missing warning when assigning namespaces via def Created: 17/May/15  Updated: 08/Nov/16

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

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


 Description   

Currently you can assign a Closure namespace to a var without getting a warning.

Minimal sample:

(ns import-names.core
  (:import [goog debug]))

(def debug goog.debug)


 Comments   
Comment by David Nolen [ 29/May/15 12:30 PM ]

The example case is a bit complected. Besides importing a name that matches a def you are also assigning a google closure namespace to a local. This will likely cause problems on its own. We need more information.

Comment by Sebastian Bensusan [ 29/May/15 12:46 PM ]

We should check that :require ed and :import ed namespaces are not used as values and then warn about it.





[CLJS-1238] Setting *main-cli-fn* when using :target :nodejs shouldn't be manditory Created: 01/May/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Jeremy Shoemaker Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File nodejs-main-cli-fn.patch    
Patch: Code

 Description   

Currently, when you use :target :nodejs in the build options for ClojureScript, the resulting code requires you to set *main-cli-fn* to a function.

This prevents someone from writing a library that can be used by JavaScript developers because it forces code execution on require. It also makes writing a CLI tool that can be distributed using NPM less straightforward. I ran into this issue trying to create a Leiningen template for writing CLI tools that could be installed using npm install or npm link. I had a wrapper script to take care of the CLI use-case, and intended to write the ClojureScript module in a more library oriented way, but ran into issues. I could work around this by not using the wrapper script, but it got me thinking about the more general library issue.

I don't see any reason why you should be forced to set *main-cli-fn* and so I'm suggesting making it optional.

Attached is a patch that makes it optional but retains the check for whether the value it is set to is a function in the case where it is set.

This is my first time submitting a change to a project using a git patch and not a pull request, so let me know if I've made the patch wrong.



 Comments   
Comment by Jeremy Shoemaker [ 01/May/15 7:27 PM ]

I just noticed the priority defaulted to "Major". I don't know if I'd say it's major, so feel free to bump it down if that doesn't seem appropriate.

Comment by Ning Sun [ 18/Feb/16 4:08 AM ]

+1.

I was working on a clojurescript library and going to build it as a node library. Currently blocked by this.

Comment by Mike Fikes [ 20/Feb/16 8:07 AM ]

Patch no longer applies.





[CLJS-1237] ns-unmap doesn't work on refers from cljs.core Created: 01/May/15  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ns-unmap

Attachments: Text File 0001-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch     Text File 0002-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch    
Patch: Code

 Description   

In ClojureScript, using ns-unmap on a symbol from cljs.core doesn't exclude it from the current namespace. Note that both a function and a macro still exist, even after unmapping:

To quit, type: :cljs/quit  
cljs.user=> (ns-unmap 'cljs.user 'when) ;; macro
true  
cljs.user=> (ns-unmap 'cljs.user 'not)  ;; function
true  
cljs.user=> (when 1 2)  
2  
cljs.user=> (not false)  
true  

This differs from the behavior of Clojure's ns-unmap. Note the appropriate errors when attempting to use unmapped symbols:

Clojure 1.7.0-beta1
user=> (ns-unmap 'user 'when) ;; macro
nil
user=> (ns-unmap 'user 'not)  ;; function
nil
user=> (when 1 2)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: when in this context, compiling:(NO_SOURCE_PATH:11:1) 
user=> (not false)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: not in this context, compiling:(NO_SOURCE_PATH:12:1) 

Somehow ClojureScript's ns-unmap needs to add the symbol to the current namespace's :excludes set. Note that the def special form does this already (after it displays a warning).

We have two solutions. 0001 extends the ns form's :merge behavior to support :excludes, and then uses this in ns-unmap. If the enhancement to ns isn't wanted, patch 0002 changes ns-unmap to update :excludes directly.



 Comments   
Comment by David Nolen [ 05/May/15 7:23 AM ]

The second patch is preferred. However it seems the second patch is too permissive. The :excludes logic should only be applied if the symbol identifies a core macro or fn.

Comment by Chouser [ 05/May/15 3:46 PM ]

The ns form's own :refer-clojure :exclude accepts arbitrary symbols and adds them to the namespace's :excludes set, which seems like the same permissiveness problem. Do you want a patch that addresses the permissiveness of both ns and ns-unmap in this ticket, or should such a patch go in a new ticket?

Comment by David Nolen [ 05/May/15 4:08 PM ]

New ticket to fix the bug that :exclude doesn't check the symbol list for cljs.core declared vars, and an updated patch here please.





[CLJS-1222] Sequence of a stateful transducer is producing the wrong answer Created: 24/Apr/15  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Lucas Cavalcanti Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, cljs, collections
Environment:

OSX 10.10.3, java 1.8.0-ea-b124



 Description   

I'm producing more than one element on the 1-arity of the transducer, and sequence is only considering the last one.

Here is the transducer and the tests that fail for sequence:

(defn sliding-window [n]
  (fn [rf]
    (let [a #js []]
      (fn
        ([] (rf))
        ([result]
         (loop [] ;; Here I'm emitting more than one element
           (when (not-empty a)
             (rf result (vec (js->clj a)))
             (.shift a)
             (recur))))
        ([result input]
         (.push a input)
         (if (== n (.-length a))
           (let [v (vec (js->clj a))]
             (.shift a)
             (rf result v))
           result))))))

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))


 Comments   
Comment by Lucas Cavalcanti [ 24/Apr/15 11:18 AM ]

I could make it work by recurring on the result.

([result]
  (loop [res result]
    (if (not-empty a)
      (let [v (vec (js->clj a))]
        (.shift a)
        (recur (rf res v)))
      res)))

even so it's weird that the previous version behaves differently on core.async and sequences in cljs and clj

Comment by David Nolen [ 26/Apr/15 4:04 AM ]

Please demonstrate the problem without core.async. Thanks.

Comment by Lucas Cavalcanti [ 26/Apr/15 7:32 PM ]

Hi,

the last test I posted on the ticket, fails in cljs, but not in clj:

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))
Comment by David Nolen [ 27/Apr/15 7:43 AM ]

I've removed the core.async bits from the description to clarify the issue.

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

The implementation of sliding-window above does not appear to be correct, it doesn't return the result. This ticket needs more information.

Comment by Lucas Cavalcanti [ 10/May/15 3:51 PM ]

As I said on http://dev.clojure.org/jira/browse/CLJS-1222?focusedCommentId=38620&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-38620

changing the 1-arity of the sliding-window to that fixes the transducer.

The point of this ticket now is that the behavior of the same (wrong) transducer in clj (both core.async and sequence) and cljs (core.async) is different than cljs sequence.





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

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

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

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

 Description   

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

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

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



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

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

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

OK

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

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

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

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

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

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

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

Sure! Have you submitted your CA yet?

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

Yes, I did yesterday.

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

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

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

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

Experience report:

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

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

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

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

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

Is this the same approach taken by Clojure?

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

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

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

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

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

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

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

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

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

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





[CLJS-1544] cljs.test REPL reload support Created: 13/Jan/16  Updated: 08/Nov/16

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

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


 Description   

When adding a test to a test ns that uses cljs.test and re-loading (via require + :reload) that namespace in the REPL after saving the file - invoking run-tests does not include the newly added test.






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

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

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


 Description   

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






[CLJS-1485] Error when requiring `goog` namespace in a ns declaration Created: 10/Nov/15  Updated: 08/Nov/16

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

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


 Description   

I wanted to use functions from goog namespace although--as I found out later, I didn't have to because goog is already exists in my namespace. So, I put (:require [goog]) in a ns declaration. Then, when I tried to reload that particular namespace by doing :require :reload in a cljs repl, I got:

Error: Namespace "x.x.x" already declared.

Doing :require :reload again in the cljs repl makes the repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)

I tested the steps below using clojurescript 1.7.145 and 1.7.170.

Here are the steps to reproduce which are taken from clojurescript quickstart-browser repl section:

1. Download the standalone clojurescript 1.7.170 jar https://github.com/clojure/clojurescript/releases/download/r1.7.170/cljs.jar

2. Create a directory hello_world and copy the JAR into that directory, then from inside the hello_world directory:

mkdir -p src/hello_world;touch repl.clj;touch index.html;touch src/hello_world/core.cljs

3. repl.clj content

(require 'cljs.repl)
(require 'cljs.build.api)
(require 'cljs.repl.browser)

(cljs.build.api/build "src"
  {:main 'hello-world.core
   :output-to "out/main.js"
   :verbose true})

(cljs.repl/repl (cljs.repl.browser/repl-env)
  :watch "src"
  :output-dir "out")

4. index.html content

<html>
    <body>
        <script type="text/javascript" src="out/main.js"></script>
    </body>
</html>

5. src/hello_world/core.cljs content

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]))

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

(enable-console-print!)

(println "Hello world!")

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

6. run clojurescript repl

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

7. Open http://localhost:9000 in browser (I use google chrome). Open javascript console.

8. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)

10. Look the browser javascript console. Nothing new shown.

11. Quit from the repl using :cljs/quit

12. Add [goog] in ns declaration in src/hello_world/core.cljs so that the content of the file becomes:

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]
            [goog]))

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

(enable-console-print!)

(println "Hello world!")

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

13. Run the clojurescript repl again

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

14. Now refresh the http://localhost:9000 in browser. Make sure the javascript console stays open.

13. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)
;;=> nil

it just returns nil

15. See the javascript console, it shows

Uncaught Error: Namespace "hello_world.core" already declared.

16. Executing this expression again (require '[hello-world.core :as hello] :reload) shows nothing new in the browser's javascript console while the clojurescript repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)





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

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

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


 Description   

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



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

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

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

we should not allow arities above 21 args

Emitting an analyzer warning is what you want?

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

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

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

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





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

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

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


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

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJS-1419] enhance numeric inference, if + number? test on local var should tag local var in the successful branch Created: 12/Aug/15  Updated: 08/Nov/16

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

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


 Comments   
Comment by David Nolen [ 12/Aug/15 6:44 AM ]

One small complication is dealing with and as it has an optimizing case.





[CLJS-1641] Multi-arity defn copies arguments unnecessarily for all cases Created: 16/May/16  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Stephen Brady Assignee: Stephen Brady
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

Background:

Passing js arguments as a parameter to another function is a known performance issue. Copying arguments to an array addresses this, and this approach has been taken to handle args passing for variadic functions in previous patches such as:
https://github.com/clojure/clojurescript/commit/dccc5d0feab9809cb56bd4d7257599a62232ac0d
https://github.com/clojure/clojurescript/commit/f09bbe62e99e11179dec6286fbb46265c12f4737

This commit (https://github.com/clojure/clojurescript/commit/576fb6e054dd50ec458a3c9e4172a5a0002c7aea) introduced a macro path for `defn` forms.

Current Behavior and Impact:

In the case of multi-arity defn forms, the macro path generates an array copy of the arguments variable regardless of whether it is used or necessary. In the case of multiple arities but no variadic arity, copying arguments is unnecessary as arguments will not be passed to the variadic method for the given function. In the case of multiple arities with a variadic case, an args array copy is needed but should be isolated to that case alone; currently, the array copy is performed before checking the arguments length, causing all cases to incur an (unused) args array copy.

Relevant code: https://github.com/clojure/clojurescript/blob/master/src%2Fmain%2Fclojure%2Fcljs%2Fcore.cljc#L2827-L2843

Recommended Change:

  • Do not perform an args array copy before switching on arguments length
  • Perform an args array copy within the variadic dispatch case

Patch forthcoming.



 Comments   
Comment by David Nolen [ 21/May/16 5:03 PM ]

Thanks! Will take a look.

Comment by David Nolen [ 10/Jun/16 1:32 PM ]

This probably needs to be updated in the compiler as well.

Comment by Stephen Brady [ 10/Jun/16 2:43 PM ]

The compiler already isolates the args to array copying behavior in the variadic case. The unnecessary copying is isolated to the defn macro.

These are the only two calls to `emit-arguments-to-array`:

Variadic function: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L743
Multi-arity with variadic case: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L812

Comment by Francis Avila [ 11/Jun/16 8:33 AM ]

From what I can see copy-arguments is only ever used with an immediately-constructed empty array as the dest in the pattern:

`(let [arr# (array)]
  (copy-arguments arr#)
  ;;...
  )

Perhaps it should change to have the exact same behavior as emit-arguments-to-array, i.e. it should take a start index and expand to the name of the destination array. The advantages of this approach are 1) it can preallocate the array to the correct size and 2) your patch no longer needs a slice call--you can avoid allocating two arrays instead of just one. (These two reasons are why I implemented emit-arguments-to-array the way I did.)

I can't think of a way to implement emit-arguments-to-array as a macro without emitting a wrapping js function which messes up the scope, but you could do this:

(defmacro copy-arguments [dest-arr startslice]
  `(loop [i# 0]
     (when (< i# (alength ~dest-arr))
       (aset ~dest-arr i# (aget (js-arguments) (+ i# startslice)))
       (recur (inc i#)))))

And then at the callsite:

`(let [variadic-args-arr# (make-array (- (alength (js-arguments)) ~maxfa))]
  (copy-arguments variadic-args-arr# ~maxfa)
    (let [argseq# (new ^::ana/no-resolve cljs.core/IndexedSeq
                       variadic-args-arr# 0 nil)]
      ;; ...
    ))

However, there are some bugs around arity handling that the above would not solve: CLJS-1678

Comment by Stephen Brady [ 11/Jun/16 8:27 PM ]

Francis, thanks for commenting on this. The patch that I submitted simply moves where/when `copy-arguments` is called. Other than that, I preserved all other aspects of the existing implementation, including how the array is built up and then made into an IndexedSeq. The line diffing in the patch implies that I changed a lot more than I really did. Agreed with your point that `emit-arguments-to-array` is more efficient and precise. Intentionally, I did not try to alter/improve/correct anything in this patch beyond solving the objective in the issue: not unnecessarily copying arguments.

Glad you've reported CLJS-1678 as I've observed this too. This buggy behavior shows up in several places. Beyond what this issue-patch attempts to address, in general, my observation is that we could probably clear out some under-brush that's accumulated as the compiler has matured with regards to arguments handling and code generated for multi-arity and/or variadic functions, apply / applyTo, and implementations of IFn. Seems like there are several opportunities to emit less javascript, create fewer intermediates, and shorten the call chain.

So to reiterate, this patch - despite its superficial appearance - changes very little and just moves the call to copy-arguments to the appropriate place. The benefit is:

For multi-arity functions with no variadic arity, no code for copying the arguments to an array is emitted at all (which in aggregate turns out to be a decent amount). Obviously, at runtime, no array will be created.

For multi-arity functions with a variadic arity, the code for copying the arguments remains but is isolated to the variadic case, and so if the function is called but will dispatch to one of the fixed arities, again, no array will be created.





[CLJS-1843] EDN analysis cache may write unusable data Created: 08/Nov/16  Updated: 08/Nov/16

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

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


 Description   

EDN has a built-in default writer for all objects, this may cause the cache to write something like

#object[Thing "thing-str"]
that cannot be read to construct an actual Thing instance.

This leads to an issue when trying to use the analysis data since it will contain different things when coming from cache or not.

This issue was highlighted by transit since it has no default writer and didn't know how to encode JSValue. [1] Instead of writing something unusable it failed early.

The cache write should rather gracefully fail (and warn) instead of writing unusable data or exploding.

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






[CLJS-1533] Null pointer error on non-existent file in advanced mode Created: 01/Jan/16  Updated: 07/Nov/16

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

Type: Defect Priority: Trivial
Reporter: Glen Mailer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting


 Description   

When building without optimisations, having a foreign-lib point to a non-existent file gives a fairly clear FileNotFoundException

When doing the same build but with advanted optimisations, the much more cryptic error java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil is given

Repo with code that reproduces and full bash output is here: https://gist.github.com/glenjamin/2614f71a50dbbccf9040



 Comments   
Comment by António Nuno Monteiro [ 07/Nov/16 10:09 AM ]

Can't repro in 1.9.293. I get the expected FileNotFoundException.





[CLJS-1646] Internal compiler error when importing Showdown via CommonJS Created: 24/May/16  Updated: 07/Nov/16

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

Type: Defect Priority: Minor
Reporter: Juan Pedro Bolivar Puente Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

cljs 1.8.51, clojure 1.8, lein-cljsbuild 1.1.3



 Description   

Hi!

I am trying to use Showdown 1.4.1 from NPM [1] using a `:commonjs` foreign-lib. Sadly, compilation crashes with the following error:

java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
Node(SCRIPT): node_modules/showdown/dist/showdown.js:1:0
[source unknown]
Parent: NULL
at com.google.javascript.jscomp.ES6ModuleLoader.normalizeInputAddress(ES6ModuleLoader.java:118)
at com.google.javascript.jscomp.ProcessCommonJSModules.inputToModuleName(ProcessCommonJSModules.java:89)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visitScript(ProcessCommonJSModules.java:336)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visit(ProcessCommonJSModules.java:245)
at com.google.javascript.js