<< Back to previous view

[CLJS-1323] possible to reload cljs.core in browser REPL Created: 01/Jul/15  Updated: 01/Jul/15

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

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


 Description   

Accidentally doing so will wreck havoc on the browser REPL as all the core types will get redefined. For some reason cljs.core is the only namespace that is not on the goog.dependencies_.written list, once as the relative path and once again as the absolute path. Uncovered while attempting to test the bootstrap support in browser environments.






[CLJS-1322] Investigate why Babel cannot load into JDK 8 Nashorn Created: 01/Jul/15  Updated: 01/Jul/15

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

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


 Description   

It appears neither the developer browser.js file nor the browser.min.js file can be loaded into Nashorn due to a method size limit exception. The code appears to originate around the lodash include. This may be a red herring but this is a good starting point for investigation.






[CLJS-1316] let does not detect invalid binding vector when it contains destructuring Created: 19/Jun/15  Updated: 19/Jun/15

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

Type: Defect Priority: Major
Reporter: Justin Glenn Smith Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The presence of destructuring in a `let` binding vector prevents detection of basic errors.

cljs.user=> *clojurescript-version*
"0.0-3308"
cljs.user=> (let [{a :a} {:a 0} b] a)
0
cljs.user=> (let [a 0 b] a)
clojure.lang.ExceptionInfo: bindings must be vector of even number of elements at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :tag :cljs/analysis-error}

David Nolen mentions that this looks like something busted with the cljs.core/assert-args macro






[CLJS-1315] Warning on Google Closure enum property access with / Created: 18/Jun/15  Updated: 18/Jun/15

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

Type: Defect Priority: Major
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-1314] Node REPL can't load :libs Created: 17/Jun/15  Updated: 18/Jun/15

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

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

Node REPL with the latest CommonJS loading impl


Attachments: Text File 0001-Illustration-of-fix.patch    

 Description   

I've been trying the CommonJS support and have been failing to get it to work in Node, and likewise so has Maria Neise. This is in the case of invoking build prior to launching the REPL as well as trying a new suggestion in CLJS-1313.

I cranked up the verbosity to see what is going on and ultimately hit upon the idea that

goog.require('module$libs$german');

isn't going to work in Node and instead needs the same alternative logic that is employed for :foreign-libs in cljs.compiler/load-libs, in particular this needs to be emitted.

cljs.core.load_file("out/german.js");

I hacked the code a bit and got the "german" CommonJS module to load in Node and be useable:

To quit, type: :cljs/quit
cljs.user=> (require '[german :refer [hello]])
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
Compiling out/cljs/core.cljs
Using cached cljs.core out/cljs/core.cljs
goog.provide('cljs.user');
goog.require('cljs.core');
goog.require('cljs.repl');
goog.require('cljs.pprint');
cljs.core.load_file("out/german.js");

nil
cljs.user=> (hello)
module$libs$german.hello.call(null)
"Hallo"

The attached patch illustrates the hack to get the above to work. The real fix would be a generalization of this.



 Comments   
Comment by Mike Fikes [ 17/Jun/15 8:44 PM ]

(Sorry for the overly generalized ticket title; it should probably be weakened to just talk about foreign libs that have been converted to native libs.)

Comment by Mike Fikes [ 17/Jun/15 8:59 PM ]

Reproduction steps (note all of this is also in a repo at https://github.com/mfikes/test-commonjs ):

Set up a node_repl.clj:

(require 'cljs.repl)
(require 'cljs.build.api)
(require 'cljs.repl.node)

(def foreign-libs [{:file "libs/greeting.js"
       :provides ["greeting"]
       :module-type :commonjs}
      {:file "libs/german.js"
       :provides ["german"]
       :module-type :commonjs}])

(cljs.build.api/build "src"
  {:main 'foo.bar
   :output-to "out/main.js"
   :verbose true
   :foreign-libs foreign-libs})

(cljs.repl/repl (cljs.repl.node/repl-env)
  :watch "src"
  :output-dir "out"
  :foreign-libs foreign-libs)

Make a libs directory with german.js:

exports.hello = function() {
    return "Hallo";
};

and greeting.js:

var german = require("german");

exports.hello = function(name) {
    return german.hello() + ", " + name;
};

create a src/foo/bar.cljs with

(ns foo.bar
  (:require [greeting :refer [hello]]))

(enable-console-print!)

(println (hello "Welt!"))

Make a QuickStart cljs.jar uberjar and place it at root of tree and then

rlwrap java -cp cljs.jar:src clojure.main node_repl.clj

followed by

(require 'foo.bar)

You should see:

../out/foo/bar.js:6
cljs.core.println.call(null,module$libs$greeting.hello.call(null,"Welt!"));
                                                      ^
TypeError: Cannot read property 'call' of undefined
    at Object.<anonymous> (/Users/mfikes/Projects/test-commonjs/out/foo/bar.js:6:55)
    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 Module.require (module.js:365:17)
    at require (module.js:384:17)
    at global.CLOSURE_IMPORT_SCRIPT (repl:75:16)
    at Object.goog.require (repl:19:60)
    at repl:5:6
Comment by Mike Fikes [ 17/Jun/15 9:29 PM ]

Another comment: My "hack" works, but to be honest, I don't appreciate why the code doesn't work without the hack. And I couldn't defend it if you asked me to. Node can evidently load Closure modules that were produced by CLJS, so why would they fail for Closure modules produced by CommonJS processing? I bet there is more behind this and my hack may be papering over some deeper more fundamental problem.

For reference, in Node, my :js-dependency-index has

{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x37d3d232 "file:/Users/mfikes/Projects/test-commonjs/out/german.js"], :closure-lib true, :lib-path "out/german.js"}

while in Ambly, the same has

{nil {:requires [], :provides ["module$libs$german"], :url #object[java.net.URL 0x34652065 "file:/Volumes/Ambly-81C53995/german.js"], :closure-lib true, :lib-path "/Volumes/Ambly-81C53995/german.js"}

(To clarfy, the above two :js-dependency-index bits are only part of the index... there is also all of the normal goog stuff.)





[CLJS-1313] REPL support for libs compiler opts Created: 17/Jun/15  Updated: 17/Jun/15

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

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

Attachments: Text File CLJS-1313-v1.patch    

 Description   

Today, module processing and the lib dependency index setup occurs only when cljs.closure/build is invoked. This ticket asks for the same setup to occur if either :libs or :foreign-libs options are passed when a REPL is launched (without an explicit build step occurring first, as is done in the Quick Start examples involving cljs.repl/repl).

An example:

(cljs.repl/repl (cljs.repl.node/repl-env)
  :foreign-libs [{:file "libs/greeting.js"
                  :provides ["greeting"]
                  :module-type :commonjs}
                 {:file "libs/german.js"
                  :provides ["german"]
                  :module-type :commonjs}])

The above would be sufficient to cause, for example, CommonJS module processing to occur, and the results to be available within the REPL.

Additionally, the implementation should defer processing to after REPL -setup has been called, in case the REPL establishes an :output-dir for :merge-opts during -setup, thereby ensuring that any module processing output goes to the correct :output-dir.



 Comments   
Comment by Mike Fikes [ 17/Jun/15 6:51 PM ]

The attached CLJS-1313-v1.patch is working fine for Ambly, but it fails for the Node REPL. Interestingly, Maria Neise was finding that CommonJS processing was not quite working properly under Node, even with an explicit build step, when compared to Nashorn. Apart from general discussion that could be had about suitability of such a patch, there is a specific concern with Node that needs to be sorted.

Comment by Mike Fikes [ 17/Jun/15 8:45 PM ]

So, with further investigation, it looks like the issue with using this patch with Node is actually a separate problem with Node: CLJS-1314

Additionally, I can confirm that the patch functions properly for Nashorn.





[CLJS-1312] JS module support: Properly convert UMD pattern Created: 15/Jun/15  Updated: 02/Jul/15  Due: 21/Aug/15

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

Type: Enhancement Priority: Major
Reporter: Maria Neise Assignee: Maria Neise
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Many popular libraries use the following or a similar idiom:

if (typeof define == "function" && define.amd) {
    define(function() {
        return greeting;
    });
} else if (typeof module != "undefined" &&
           module.exports) {
    module.exports = greeting;
} else {
    window["greeting"] = greeting;
}

The ProcessCommonJSModules class converts this to the following:

if (typeof define == "function" && define.amd) {
    define(function() {
        return greeting;
    });
} else {
    if (typeof module != "undefined" && module$greeting) {
        module$greeting = greeting;
    } else {
        window["greeting"] = greeting;
    }
}

module will not be defined, so the greeting object will not be assigned to the new module namespace. We need to check with the Google Closure compiler mailing list if we can submit a patch that replaces module so that the condition succeeds.



 Comments   
Comment by Maria Neise [ 15/Jun/15 1:35 PM ]

Asked about this on Google Closure compiler mailing list: https://groups.google.com/forum/#!topic/closure-compiler-discuss/-M1HBUn35fs





[CLJS-1311] Improve error reporting when converting CommonJS modules Created: 15/Jun/15  Updated: 17/Jun/15  Due: 21/Aug/15

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

Type: Enhancement Priority: Major
Reporter: Maria Neise Assignee: Maria Neise
Resolution: Unresolved Votes: 2
Labels: None


 Description   

When using the Google Closure compiler via the command line and trying to convert a CommonJS module which has trailing commas, the compiler throws a parse error, e.g.:

chance.js:1109: ERROR - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
                        '09' + self.string({ pool: '0123456789', length: 8}),

At the moment, we don't throw any errors when converting a CommonJS module and an error occurs. Instead, just the following gets emitted by the Google Closure compiler:

goog.provide("module$libs$chance");var module$libs$chance={}

We need to change this behaviour to throw an error.



 Comments   
Comment by Kristian Mandrup [ 16/Jun/15 10:53 PM ]

Yes, by default don't convert the CommonJS module, unless perhaps if you set a specific --force compile option.





[CLJS-1305] Node.js REPL load-file regression Created: 09/Jun/15  Updated: 09/Jun/15

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

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


 Description   

Appears to be the same as the one we encountered in browser REPL due to the Closure Library dep bump.






[CLJS-1304] Behavior of clojure.string/replace differs from Clojure Created: 09/Jun/15  Updated: 13/Jun/15

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

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Andrew Rosa
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1304.patch    

 Description   

When using clojure.string/replace with a function as the replacement value, and using a regular expression with capturing groups, the behavior differs between Clojure and ClojureScript. In Clojure, the replacement fn is passed a tuple of the match and capturing group. In ClojureScript, it is only passed the match value.

In Clojure:

=> (println (str/replace "foobar" #"f(o)o" #(do (println %) "X")))
[foo o]
"Xbar"

In ClojureScript:

=> (println (str/replace "foobar" #"f(o)o" #(do (println %) "X")))
foo
Xbar


 Comments   
Comment by Daniel Woelfel [ 11/Jun/15 3:31 AM ]

If you're looking for a workaround, you can still get the match result from the 2nd arg:

=> (println (clojure.string/replace "foobar" #"f(o)o" (fn [& args] (do (println args) "X"))))
(foo o 0 foobar)
Xbar
Comment by Andrew Rosa [ 13/Jun/15 12:40 PM ]

Implementation and tests for the requested behaviour. Despite of the `vec` not being strictly necessary, I stick with it so the behaviour will be identical on both implementations.

Comment by David Nolen [ 13/Jun/15 5:07 PM ]

Is there any reason here to not just copy the Clojure implementation?

Comment by Andrew Rosa [ 13/Jun/15 5:25 PM ]

Hi David,

Clojure's impl was the first place that I look for, but this feature in special is written against the underlaying Java libraries. You can check it here.

Studying the Clojure implementation I found that there are some of re-* family of functions missing on ClojureScript side, and unfortunately they are heavily inspired on Java's Regex structure (return matchers, find and extract groups). MAYBE we can something like them to ClojureScript, but I don't know the real value there. Anyway, I think that port them or not is subject for another ticket - if you want that I investigate more, I could create it.

Comment by Francis Avila [ 13/Jun/15 5:47 PM ]

I made a proposal some time ago to bring clj and cljs re handling closer together: CLJS-776

Comment by David Nolen [ 13/Jun/15 8:51 PM ]

Andrew, was just looking for rationale. What you've done looks OK to me. People should try the patch and give feedback. Thanks!

Comment by Andrew Rosa [ 13/Jun/15 10:41 PM ]

Hi David, I understand and appreciate your concerns I just want to make clear what I've search and done, sorry if I sound rude - maybe I stepped on my language limitations.

Francis, really this re-* thing is tricky to deal with. I will take a closer look onto your ticket and see if I can add any idea.

Thanks everyone!





[CLJS-1303] runtime namespaces support for cljs.tools.reader Created: 09/Jun/15  Updated: 12/Jun/15

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3308
Fix Version/s: Next

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


 Description   

We should just piggieback on goog.dependencies_. This has all of the known namespaces. We should add the usual namespace helpers ns-map etc. but we should throw if COMPILED is false. This stuff can only work under :optimizations :none.






[CLJS-1302] (js/goog.base (js* "this")) no longer works Created: 09/Jun/15  Updated: 09/Jun/15

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

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


 Description   
(defn Foo
  []
  (goog/base (js* "this")))
(goog/inherits Foo js/Object)

(defn Foo
  []
  (this-as this
    (goog/base this)))
(goog/inherits Foo js/Object)





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

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

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


 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.






[CLJS-1299] reader/read-string does not handle \space literal Created: 05/Jun/15  Updated: 05/Jun/15

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

Type: Defect Priority: Major
Reporter: Ray Huang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

OSX



 Description   

I notice that in Clojure Script, it doesn't know how to interpret \space using reader/read-string

In Clojure:
entangle.single=> (pr-str {:foo \space})
"{:foo
space}"

Then, attempting to deserialize in ClojureScript:
entangle.client => (reader/read-string "{:foo
space}")
#<Error: Map literal must contain an even number of forms>
Error: Map literal must contain an even number of forms
at Function.cljs.reader.reader_error.cljs$core$IFn$_invoke$arity$variadic (http://localhost:10000/public/js/out/cljs/reader.js:156:8)
at cljs$reader$reader_error (http://localhost:10000/public/js/out/cljs/reader.js:152:33)
at cljs$reader$read_map (http://localhost:10000/public/js/out/cljs/reader.js:436:26)
at cljs$reader$read (http://localhost:10000/public/js/out/cljs/reader.js:810:34)
at cljs$reader$read_string (http://localhost:10000/public/js/out/cljs/reader.js:837:25)






[CLJS-1298] source-on-disk conditional should include :source-url Created: 05/Jun/15  Updated: 05/Jun/15

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

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

All



 Description   

cljs.closure/source-on-disk is conditional on having the IJavaScript having a :source-url. Currently this conditional is exercised after the call to util/ext. But util/ext has a precondition that will fail on a nil :source-url.

Solution: protect util/ext by checking for :source-url before calling util/ext.

(defn source-on-disk
  "Ensure that the given IJavaScript exists on disk in the output directory.
  Return updated IJavaScript with the new location if necessary."
  [opts js]
  (if (write-js? js)
    (write-javascript opts js)
    ;; always copy original ClojureScript sources to the output directory
    ;; when source maps enabled
    (let [out-file (when-let [ns (and (:source-map opts)
                                      (:source-url js) ;;<--- ADD THIS HERE
                                      (first (:provides js)))]
                     (io/file (io/file (util/output-directory opts))
                       (util/ns->relpath ns (util/ext (:source-url js)))))
          source-url (:source-url js)]
      (when (and out-file source-url
                 (or (not (.exists ^File out-file))
                     (> (.lastModified (io/file source-url))
                        (.lastModified out-file))))
        (spit out-file (slurp source-url)))
      js)))

Or some refactoring of the above.



 Comments   
Comment by David Nolen [ 05/Jun/15 2:20 PM ]

In what cases will an IJavaScript not have a :source-url? Just trying to get some context.

Comment by Bruce Hauman [ 05/Jun/15 2:42 PM ]

Good question. When the sources are already output javascript and they have an output dir relative :url but not a :source-url, this doesn't seem like it should happen.

I'm getting a situation like this

:url #object[java.net.URL 0x5f921081 "file:/Users/brucehauman/workspace/temp/hello-world/resources/goog/uri/utils.js"]
:source-url nil

Comment by David Nolen [ 05/Jun/15 2:58 PM ]

Ok that seems like the real underlying issue. Do you have a small reproducer by chance?

Comment by Bruce Hauman [ 05/Jun/15 3:08 PM ]

It seems like its when the :asset-path is "". And :output-dir is at the root of a resource path like :output-dir "resources".





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

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

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


 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.





[CLJS-1296] browser REPL should queue prints before connection then flush after connection Created: 01/Jun/15  Updated: 01/Jun/15

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

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


 Description   

If the page loads and attempts to print before the connection is made the application will unintuitively error out.






[CLJS-1287] CLJS exceptions should use same format as Clojure 1.7 (#exception {...}) Created: 25/May/15  Updated: 01/Jun/15

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

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


 Description   

Recent Clojure 1.7-RC adds new print format compatible with EDN syntax:

(ex-info "foobar" {:bar :baz})
#error {
 :cause "foobar"
 :data {:bar :baz}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message "foobar"
   :data {:bar :baz}
   :at [clojure.core$ex_info invoke "core.clj" 4591]}]
 ...
}

(try (.indexOf 0 "baz") (catch Throwable e e))
#error {
 :cause "No matching method found: indexOf for class java.lang.Long"
 :via
 [{:type java.lang.IllegalArgumentException
   :message "No matching method found: indexOf for class java.lang.Long"
   :at [clojure.lang.Reflector invokeMatchingMethod "Reflector.java" 53]}]
 ...
}

Unfortunatelly ClojureScript format is different (and sometimes even not compatible with EDN):

(ex-info "foobar" {:bar :baz})
#ExceptionInfo{:message "foobar", :data {:bar :baz}}

(try (.indexOf 0  "baz") (catch js/Error e e))
#<TypeError: Object 0 has no method 'indexOf'>

The request to unify the output of exceptions might sound like "nitpicking".

However please consider applications, which share (lots of) code between server and client.
Any arbitrary difference between Clojure and ClojureScript adds future accidental complexity.

Hint: looking for green light and answer patches welcome

Last thing to consider is how to deal with stacktraces in the patch. Recent CLJS parses have a parser implemented on server side in Clojure. Should the patch move parsing to ClojureScript?



 Comments   
Comment by David Nolen [ 25/May/15 10:14 AM ]

Yes patch welcome. Thanks!





[CLJS-1280] double analysis warnings when using cljs.repl/load-file special fn Created: 21/May/15  Updated: 01/Jun/15

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

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





[CLJS-1247] Add *out* and *err* Created: 04/May/15  Updated: 01/Jun/15

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

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


 Comments   
Comment by David Nolen [ 06/May/15 9:02 AM ]

See CLJS-710





[CLJS-1237] ns-unmap doesn't work on refers from cljs.core Created: 01/May/15  Updated: 01/Jun/15

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

Type: Defect Priority: Major
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-1236] `constructor` needs to munged if used as namespace segment Created: 01/May/15  Updated: 01/Jun/15

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

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


 Description   

Will make goog.require believe that the namespace has already been loaded and throw an exception since all JS objects have the constructor property.



 Comments   
Comment by David Nolen [ 04/May/15 8:19 AM ]

This issue is not quite as simple as it sounds as we actually need a special case for namespace munging. For example we don't want to munge Foo.constructor but we do want to munge ns.foo.constructor. To implement the fix correctly it would require changing all the cases where library names are munged.





[CLJS-1228] cljs.util/topo-sort is polynomial on larger dependency graphs Created: 28/Apr/15  Updated: 01/Jun/15

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

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


 Comments   
Comment by David Nolen [ 08/May/15 7:35 AM ]

This problem was reported due the performance of cljs.analyzer/ns-dependents when used by cljs.compiler/compile-file. However as cljs.compiler/compile-file does not need the topological sorting bit, we should supply a faster alternative to cljs.analyzer/ns-dependents which simply returns a distinct seq which can be poured into a set.





[CLJS-1207] Emit a warning if multiple resources found for a ClojureScript namespace Created: 15/Apr/15  Updated: 01/Jun/15

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

Type: Enhancement Priority: Major
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-1194] data_readers.cljc Created: 10/Apr/15  Updated: 01/Jun/15

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

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


 Description   

Now that conditional reading has landed we can implement support for data_readers.cljc to get both compile time and runtime support.



 Comments   
Comment by David Nolen [ 10/Apr/15 7:45 PM ]

This needs http://dev.clojure.org/jira/browse/CLJ-1699 to be useful.

Comment by Nikita Prokopov [ 19/May/15 7:58 AM ]

CLJ-1699 has landed.

Right now CLJS tries to compile data_readers.cljc as a regular source code file:

Exception in thread "main" java.lang.AssertionError: No ns form found in src/data_readers.cljc, compiling:(/private/var/folders/0h/9vv4g3d955l6ctwwl4k9xjy40000gn/T/form-init3533791126017861878.clj:1:125)
	at clojure.lang.Compiler.load(Compiler.java:7249)
	at clojure.lang.Compiler.loadFile(Compiler.java:7175)
	at clojure.main$load_script.invoke(main.clj:275)
	at clojure.main$init_opt.invoke(main.clj:280)
	at clojure.main$initialize.invoke(main.clj:308)
	at clojure.main$null_opt.invoke(main.clj:343)
	at clojure.main$main.doInvoke(main.clj:421)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Comment by David Nolen [ 19/May/15 8:53 AM ]

This should be addressed first: http://dev.clojure.org/jira/browse/CLJS-1277





[CLJS-1191] Update clojure.walk to the current version on clojure Created: 06/Apr/15  Updated: 01/Jul/15

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

Type: Defect Priority: Major
Reporter: Stuart Mitchell Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: enhancement, performance

Attachments: Text File 50.patch     Text File CLJS-1191.patch     Text File CLJS-1191-tests.patch    
Patch: Code

 Description   

Currently clojure.walk can not handle records

It is using an old version of clojure.walk and clojure has improved the implementation because of

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


src/cljs/clojure/walk.cljs | 4 ++++
1 file changed, 4 insertions

diff --git a/src/cljs/clojure/walk.cljs b/src/cljs/clojure/walk.cljs
index f2ebd8d..541ecea 100644
— a/src/cljs/clojure/walk.cljs
+++ b/src/cljs/clojure/walk.cljs
@@ -43,7 +43,11 @@ the sorting function."}
{:added "1.1"}
[inner outer form]
(cond
+ (list? form) (outer (apply list (map inner form)))
+ (satisfies? IMapEntry form) (outer (vec (map inner form)))
(seq? form) (outer (doall (map inner form)))
+ (satisfies? IRecord form)
+ (outer (reduce (fn [r x] (conj r (inner x))) form form))
(coll? form) (outer (into (empty form) (map inner form)))
:else (outer form)))



 Comments   
Comment by David Nolen [ 07/Apr/15 5:56 AM ]

Please attach the patch as a file. Thanks!

Comment by Stuart Mitchell [ 07/Apr/15 7:18 PM ]

I think this one works

it is a mail formatted patch

Comment by David Nolen [ 08/Apr/15 6:07 AM ]

Please follow the patch conventions described here https://github.com/clojure/clojurescript/wiki/Patches. Thank you.

Comment by David Nolen [ 12/Apr/15 4:53 PM ]

Stuart, I don't see you on the list of contributors. Please submit a CA so I can apply the patch. Thanks!

Comment by Stuart Mitchell [ 13/Apr/15 7:49 PM ]

Hopefully this is the right format

Comment by Stuart Mitchell [ 13/Apr/15 7:50 PM ]

Contributor agreement signed as well

Comment by Sebastian Bensusan [ 01/May/15 1:24 PM ]

Tested the patch on the REPL, works as expected except for walking fns that modify the keys:

> (defrecord Foo [name])
cljs.user/Foo
> (w/prewalk #(if (keyword? %) (str %) %) (Foo. "foo"))
#cljs.user.Foo{:name "foo", ":name" "foo"}

It is not consistent with walking the same fn over a map:

> (w/prewalk #(if (keyword? %) (str %) %) {:name "foo"})

{":name" "foo"}

This behavior was noted in the original Clojure patch as well: http://dev.clojure.org/jira/browse/CLJ-1239 and might be undesirable since it surprises the user.

Comment by Daniel Skarda [ 10/Jun/15 9:59 AM ]

Our project shares many files between server and client (Clojure and ClojureScript). I noticed the same bug today, because tests in CLJ went fine, while same tests in CLJS failed (then I prepared a patch just to find this ticket...)

From my perspective, I would prefer that CLJ and CLJS have the same implementation of clojure.walk.

For example - you implement some clever scheme to avoid kind of surprise you described. If it is not implemented in Clojure in the same way, it will be surprise for people writing code portable between CLJ and CLJS.

So my proposal is to take current Clojure implementation as a master and wait, how clojure.walk implementation will be improved in future releases of Clojure. Then we can port it to ClojureScript.

Comment by Daniel Skarda [ 10/Jun/15 10:10 AM ]

Tests for patch by Stuart

Comment by David Nolen [ 01/Jul/15 1:38 PM ]

Happy to apply these but they need to be rebased to master. Thanks.





[CLJS-1177] A compiler support for non-Closure transforms (JSX, etc) Created: 28/Mar/15  Updated: 01/Jul/15

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Maria Neise
Resolution: Unresolved Votes: 1
Labels: None


 Description   

There are now a variety of JS dialects, JSX, TypeScript etc. which need to be transformed first into regular JS before module processing. We should devise a standard way to deal with this. As we cannot predict what various dialects may arise in the future it's probably best to provide something like :preprocess-module option to cljs.closure/build.



 Comments   
Comment by David Nolen [ 17/Jun/15 5:20 PM ]

We should verify with Babel now that React is moving away from JSX.

Comment by David Nolen [ 01/Jul/15 6:02 PM ]

I went ahead and looked into this a bit. Looks like getting Babel to work will take some time. Babel has difficulties loading into Nashorn due to code size limits that originate from Lodash usage. JSX while deprecated is still the most widely used option so we have some time yet and the interface supplied by both tools is exactly the same. So we should test this feature with JSX and switch to Babel when the issues are resolved.





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

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

Type: Enhancement Priority: Major
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: 01/Jun/15

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

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


 Description   

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






[CLJS-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 01/Jun/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
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-1142] ClojureScript stream socket REPL Created: 18/Mar/15  Updated: 06/May/15

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

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


 Description   

Port relevant bits of CLJ-1671






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

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

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

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

Quick start guide with Node REPL



 Description   

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

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

ClojureScript REPLs do not behave in the same way:

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





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

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

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


 Description   

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






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

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

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


 Description   

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






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

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

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


 Description   

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






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

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

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


 Description   

Needed for shared AOT cache






[CLJS-1075] Generic inline source map support Created: 02/Mar/15  Updated: 02/Mar/15

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

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


 Description   

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






[CLJS-1074] Externs inference Created: 02/Mar/15  Updated: 30/Apr/15

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

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


 Description   

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

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






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

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

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


 Description   

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






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

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

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


 Description   

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






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

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

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

Linux 64bit

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


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

 Description   

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

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

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

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

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



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

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

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

I did bind out to err. Check the patch.

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

Crispin, oops sorry you are correct. Thanks.

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

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

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

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

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

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

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

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





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

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

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


 Description   

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






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

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

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

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



 Description   

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

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

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

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



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

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

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

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

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

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

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

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

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

to be true and emit a "return".

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.





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

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

David, Where exactly should the parallelization happen?





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

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

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

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


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

 Description   

If I evaluate

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

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

(lldb) bt

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

(Full stack trace attached as stacktrace.txt)

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

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



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

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

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

The construct

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

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

In r2014, the emitted JavaScript is:

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

while in r2024 the emitted JavaScript is:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

This JavaScriptCore perf problem is easily reproduced purely with JavaScript:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

For example:

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

Or maybe cheat and emit a ChunkedCons instead?

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

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

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

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

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

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

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

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

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

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

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

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

Activity is occurring with the WebKit ticket:

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

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

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

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

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

Mike this is true only for cljs.core.

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

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





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

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
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-871] .-default property access returns nil Created: 11/Oct/14  Updated: 01/Jun/15

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

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

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

 Description   

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

(deftype Foo [default])

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


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

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

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

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

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

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

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

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

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

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

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

Updated to use new refactorings

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

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





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

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

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


 Description   

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






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

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

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


 Description   

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






[CLJS-836] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 01/Jun/15

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

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

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

 Description   

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



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

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

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

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

Comment by Peter Schuck [ 03/Apr/15 10:10 AM ]

CLJ-1499 has been applied to Clojure. I updated the patch to update the tests.





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

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

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

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


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

 Description   

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



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

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

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

Patch welcome for this.

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

r2173



 Description   

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

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



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

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

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

Bump, this enhancements sound simple & fine.

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

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

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

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

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

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

Be my guest





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 01/Jun/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
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-713] optimized case Created: 04/Dec/13  Updated: 23/Jun/14

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

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

Attachments: Text File 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch     Text File 0001-CLJS-713-first-cut-at-compiling-case-to-switch.patch    

 Description   

With the advent of asm.js many engines will like compile switch statements over integers into jump tables. We should provide a real `case*` ast node that compiles to JS `switch` when possible - i.e. numbers, strings, keywords etc.



 Comments   
Comment by Michał Marczyk [ 18/Feb/14 5:56 PM ]

First cut impl also available here:

https://github.com/michalmarczyk/clojurescript/tree/713-compile-case-to-switch

With this patch applied, case expressions are compiled to switch + some extra bits when all tests are numbers or strings, otherwise old logic is used.

For example, {{(fn [] (let [x 1] (case x 1 :foo (2 3) :bar :quux)))}} gets compiled to

function () {
    var x = 1;
    var G__6469 = x;
    var caseval__6470;
    switch (G__6469) {
      case 1:
        caseval__6470 = new cljs.core.Keyword(null, "foo", "foo", 1014005816);
        break;
      case 2:
      case 3:
        caseval__6470 = new cljs.core.Keyword(null, "bar", "bar", 1014001541);
        break;
      default:
        caseval__6470 = new cljs.core.Keyword(null, "quux", "quux", 1017386809);
    }
    return caseval__6470;
}

The existing test suite passes, but I suppose it wouldn't hurt to add some tests with case in all possible contexts.

Comment by Michał Marczyk [ 18/Feb/14 6:05 PM ]

As a next step, I was planning to arrange things so that numbers/strings are fished out from among the tests and compiled to switch always, with any leftover tests put in an old-style nested-ifs-based case under default:. Does this sound good?

It seems to me that to deal with symbols and keywords in a similar manner we'd have to do one of two things:

1. check symbol? and keyword? at the top, then compile separate switches (the one for keywords would extract the name from the given keyword and use strings in the switch);

2. use hashes for dispatch.

Which one sounds better? Or is there a third way?

Comment by Michał Marczyk [ 18/Feb/14 6:11 PM ]

Of course we'd need to compute hashes statically to go with 2. I'd kind of like it if it were impossible (randomized seed / universal hashing), but currently it isn't.

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

At least on v8, there are surprisingly few cases where a switch statement will be optimized to a jump table. Basically the type of the switched-over value must always (across calls) match the type of every case, and there must be fewer than 128 cases, and integer cases must be 31-bit ints (v8's smi type). So mixing string and number cases in the same switch guarantees the statement will never be compiled. In many cases an equivalent if-else will end up being significantly faster on v8 just because the optimizing jit recognizes them better. There's an oldish bug filed against v8 switch performance. Looking at the many jsperfs of switch statements, it doesn't seem that v8 has improved. Relevant jsperf

Firefox is much better at optimizing switch statements (maybe because of their asm.js/emscripten work) but I don't know what conditions trigger (de)optimization.

I suspect the best approach is probably going to be your option one: if-else dispatch on type if any case is not a number, and then a switch statement covering the values for each of the keyword/string/symbol types present (no nested switch statements, and outlining the nested switches might be necessary). Even with a good hash, to guarantee v8 optimizing-compilation you would need to truncate the hashes into an smi (signed-left-shift once?) inside the case*.

Comment by David Nolen [ 19/Feb/14 12:50 AM ]

There's no need for invention here. We should follow the strategy that Clojure adopts - compile time hash calculation.

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

The problem, as Michal alluded to, is that the hash functions in cljs's runtime environment are not available at compile-time (unlike in Clojure). This might be a good opportunity to clean up that situation or even use identical hash values across Clojure and Clojurescript (i.e. CLJS-754), but that's a much bigger project. Especially considering it will probably not bring much of a speedup over an if-else-if implementation except in very narrow circumstances.

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

Francis Avila I would make no such assumptions about performance without benchmarks. One of the critical uses for case is over keywords. Keyword hashes are computed at compile time, so that's one function call and a jump on some JavaScript engines. This is particularly useful for the performance of records where you want to lookup a field via keyword before checking the extension map.

This ticket should probably wait for CLJS-754 before proceeding.

Comment by Francis Avila [ 22/Feb/14 4:44 AM ]

Record field lookup is a good narrow use case to test. I put together a jsperf to compare if-else (current) vs switch with string cases vs switch with int cases (i.e., hash-compares, assuming perfect hashing).

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

I've merged the case* analyzer and emitter bits by hand into master.

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

I'v merged the rest of the patch into master. If any further optimizations are done it will be around dispatching on hash code a la Clojure.

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

Your keyword-test optimization has a bug: https://github.com/clojure/clojurescript/commit/9872788b3caa86f639633ff14dc0db49f16d3e2a

Test case:

(let [x "a"] (case x :a 1 "a"))
;=> 1
;;; Should be "a"

Github comment suggests two possible fixes.

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

Thanks fix in master.

Comment by Christoffer Sawicki [ 23/Jun/14 3:41 PM ]

case over "chars" is currently not being optimized to switch (in other words: (case c (\a) :a :other) uses if instead of switch).

Given that ClojureScript chars are just strings of length 1, could this perhaps simply be fixed by tweaking https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/core.clj#L1187 ?

Comment by Christoffer Sawicki [ 23/Jun/14 4:11 PM ]

OK, I couldn't resist trying and it seems to be that easy. Would be great if somebody more knowledgeable could look at it and say if it has any side-effects. (See patch with name 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch.)

Comment by David Nolen [ 23/Jun/14 4:15 PM ]

The patch looks good I would have applied it if I hadn't already gone and done it master myself just now

Comment by Christoffer Sawicki [ 23/Jun/14 4:22 PM ]

Hehe. Thanks! Don't forget to update the "case* tests must be numbers or strings" message on line 496 too.

Comment by David Nolen [ 23/Jun/14 4:48 PM ]

The existing docstring is inaccurate - case supports all compile time literals.





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

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
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-705] locals clearing Created: 27/Nov/13  Updated: 29/Nov/13

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

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


 Description   

Without locals clearing ClojureScript is likely to suffer from the same cases as Clojure did for common uses of lazy sequences.



 Comments   
Comment by David Nolen [ 29/Nov/13 3:03 PM ]

For this we'll need to introduce some special private way to set a local to nil, i.e. (_clear_local sym)





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

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

Type: Defect Priority: Major
Reporter: Park Sang Kyu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, bug, patch
Environment:

in windows 7


Attachments: File cljsc.bat.diff     File cljsc-path.bat    
Patch: Code

 Description   

cljsc.bat emit FileNotFoundException when it compile samples of the ClojureScript project in windows like below.

------------------------------------------------
Exception in thread "main" java.io.FileNotFoundException: Could not locate cljs/closure__init.class
or cljs/closure.clj on classpath:
------------------------------------------------

It is caused by lack of a backslash in the end of path of the system environment variable, %CLOJURESCRIPT_HOME% set by a user.
In the case CLASSPATH is set to "C:\\clojure\clojurescriptsrc\clj;C:\\clojure\clojurescriptsrc\cljs" and this make it impossible for javac to find cljs/clojure.clj file.

So it can be solved by adding a backslash to the path of %CLOJURESCRIPT_HOME%.

I attached the patched file, "cljsc-path.bat"



 Comments   
Comment by David Nolen [ 04/Sep/13 11:04 PM ]

Can we please get a proper git diff thanks (and please send in your CA)! Also would be nice to get Windows users to check this out.

Comment by Park Sang Kyu [ 15/Sep/13 3:16 AM ]

git diff

Comment by David Nolen [ 05/Oct/13 11:55 AM ]

Thank you! Have you sent in your CA? http://clojure.org/contributing

Comment by Park Sang Kyu [ 19/Jun/14 10:24 AM ]

Yes i have sent my CA.

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

Excellent, the patch is not correctly formatted. Can we get a new patch that conforms to http://github.com/clojure/clojurescript/wiki/Patches





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

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

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


 Description   

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



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

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

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

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





[CLJS-497] Constant literal optimization Created: 25/Apr/13  Updated: 27/Aug/13

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

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


 Description   

We should optimize constant literals, in particular keywords. This optimization means that we will have to decide whether to make identical? slower by testing for keywords (this means it's probably a bad idea to continue to inline it) or to provide a special keyword-identical? that does the right thing.



 Comments   
Comment by Sean Grove [ 26/Aug/13 11:03 PM ]

This is related to the reified keywords in cljs, see http://dev.clojure.org/jira/browse/CLJS-576

Comment by Sean Grove [ 27/Aug/13 4:24 PM ]

There's another interesting twist while using piggieback + brepl that relates to a missing constants_table.js. Not sure what causes it (haven't found a way to repro), but only happens in a few circumstances, so the repl still mainly works.

The runtime part continues to works fine however.





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

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

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


 Description   

It's worth investigating Selenium, PhantomJS, etc. as solutions to sanity check the Browser REPL when we run the other tests.



 Comments   
Comment by Robert Krahn [ 22/Dec/14 1:22 PM ]

An attempt: https://github.com/clojure/clojurescript/pull/42

Comment by David Nolen [ 24/Dec/14 8:57 AM ]

This looks like an interesting patch, thanks!

Comment by Robert Krahn [ 26/Dec/14 10:57 AM ]

I'll post a patch here, first I'll investigate the load-file issue, though.





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 19/Nov/13

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

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





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

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

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


 Description   

This ticket is related to CLJS-359






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

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

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


 Description   

This ticket is related to CLJS-359






Generated at Thu Jul 02 11:41:32 CDT 2015 using JIRA 4.4#649-r158309.