<< Back to previous view

[CLJS-1865] Google Closure Compiler in JavaScript Created: 06/Dec/16  Updated: 06/Dec/16

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

Type: Enhancement Priority: Trivial
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Google released its JavaScript version of the Closure Compiler – "this allows Closure Compiler to run entirely in JS. Java is not required":

Switching to the JS compiler means JS devs coming to ClojureScript will be able to use the tools they're familiar with a simplify the onboarding docs on the website.

NB: I discovered this while experimenting with using ClojureScript with Polymer:






[CLJS-1718] Foreign lib files should be placed in a location that matches their namespace Created: 29/Jul/16  Updated: 02/Dec/16  Resolved: 02/Dec/16

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

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Affects master



 Description   

Using several foreign libs with the same file name ends up just including one of them, as the files are placed at the root of the `:output-dir`.

A solution for this would be placing those files in a location that matches their `:provides` namespace.



 Comments   
Comment by David Nolen [ 02/Dec/16 5:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/97d2d61e78ce747d02d0e5b2ced706f6fb68ec4e





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

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

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


 Description   

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

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

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

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


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

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

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

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

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

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

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

Sounds like a plan.

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

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

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

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





[CLJS-1863] :reload/:reload-all issue with .cljc runtime/macro nses Created: 29/Nov/16  Updated: 29/Nov/16

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

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


 Description   

Brandon Bloom discovered an issue where a ns that is used both from runtime and for macros (where the macros are self-required) won't respect `(require ... :reload/:reload-all)`.



 Comments   
Comment by David Nolen [ 29/Nov/16 3:59 PM ]

Whatever we do needs to be copied over into a bootstrapped, but that's a separate issue.





[CLJS-1862] allow NodeJS's NODE_MODULES to be set as a REPL option Created: 28/Nov/16  Updated: 29/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Marc Daya Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: nodejs, repl

Attachments: Text File 65.patch    
Patch: Code

 Description   

The NodeJS REPL that ships with ClojureScript seems to assume that all NodeJS modules are installed globally, or that node's NODE_PATH environment variable is set for the process that starts the REPL (e.g. CIDER). Allowing this to be set as a REPL option make it possible for modules to be installed and made available to the REPL by build tooling, eliminating manual steps by the user and improving repeatability.



 Comments   
Comment by David Nolen [ 28/Nov/16 4:26 PM ]

Thanks. Have you submitted your Clojure CA yet?

Comment by Marc Daya [ 29/Nov/16 2:02 PM ]

It has just been filed.





[CLJS-1860] Resolve JS modules referred by their fully-qualified namespace Created: 24/Nov/16  Updated: 28/Nov/16  Resolved: 28/Nov/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

This is part 2 of CLJS-1848. When a JS module is used in a macro, the analyzer should find the correct `full-ns` if the module is fully qualified.

Without this patch, downstream namespaces that use the macro would need to explicitly require the JS module. This shouldn't be necessary if the module has already been required in the namespace that declares the macro.

This patch doesn't introduce any new behavior. Instead, it mimics the current CLJS namespaces behavior for JS modules.



 Comments   
Comment by David Nolen [ 28/Nov/16 8:46 AM ]

fixed https://github.com/clojure/clojurescript/commit/79a20afe360249ab6cb652f4465b7ccd01a923f2





[CLJS-1861] Use usr/bin/env in build scripts for portability Created: 25/Nov/16  Updated: 28/Nov/16  Resolved: 28/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1861.patch    

 Description   

There are a couple of build scripts where

#!/bin/bash
could be converted to
#!/usr/bin/env bash
for additional portability.



 Comments   
Comment by David Nolen [ 28/Nov/16 8:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/170fd767752a4839b25038c86b2d6a6aa3b25ab7





[CLJS-1859] Comparing a map and a record with = produces different results based on argument order Created: 23/Nov/16  Updated: 23/Nov/16

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

Type: Defect Priority: Minor
Reporter: Juan Facorro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac OS X
V8
java version "1.8.0_73"
Java(TM) SE Runtime Environment (build 1.8.0_73-b02)
Java HotSpot(TM) 64-Bit Server VM (build 25.73-b02, mixed mode)



 Description   

The result of comparing a map and a record is different based on the order of the arguments to =:

To quit, type: :cljs/quit 
cljs.user=> (defrecord R []) 
cljs.user/R 
cljs.user=> (= {} (R.)) 
true 
cljs.user=> (= (R.) {}) 
false 

The result is the same for the code above with tags r1.7.228, r1.8.34 and 1.9.293.

This seems to be rooted in the fact that when a map is the first argument, the function used to make the comparison is the implementation of equiv from the map. But when a record is the first argument the implementation used is the one from the record, which checks if the types of both arguments are equal.

In Clojure JVM the implementation of equiv in clojure.lang.APersistentMap checks for the marker interface clojure.lang.MapEquivalence, which avoids this situation.






[CLJS-1858] Should allow `:cache-analysis true` and `cache-analysis-format nil` Created: 21/Nov/16  Updated: 22/Nov/16  Resolved: 22/Nov/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

This was intended but doesn't actually work.



 Comments   
Comment by David Nolen [ 22/Nov/16 1:00 PM ]

fixed https://github.com/clojure/clojurescript/commits/master





[CLJS-1857] Fix self-host tests Created: 20/Nov/16  Updated: 21/Nov/16  Resolved: 21/Nov/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

self-hosted tests are broken in master given the latest externs inference commits.



 Comments   
Comment by Mike Fikes [ 20/Nov/16 1:23 PM ]

+1 LGTM and script/test-self-parity, script/test-self-host, and script/test all pass for me (apart from the expected Nashorn error we have been seeing recently)

Comment by David Nolen [ 21/Nov/16 3:00 PM ]

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





[CLJS-1856] Self-host: load-deps doesn't delegate to itself Created: 17/Nov/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

The first arity of cljs.js/load-deps should be calling the second arity of itself, but it is currently inadvertently making a call to cljs.js/analyze-deps.

Note: This arity is not being used. The arguments being passed along were properly updated with CLJS-1826, but it is now actually inadvertently calling analyze-deps with an arity error. (The real fix is to make it call load-deps).



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

dnolen comment lost in system migration:

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





[CLJS-1855] Subvec should implement IIterable Created: 17/Nov/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

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

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

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

 Description   

Currently calling `iter` on a `Subvec` falls back to `seq-iter`. We can obtain performance equal to `PersistentVector` iteration by using `ranged-iterator`. This also brings Subvecs more inline with the Clojure implementation.

Benchmark:

(simple-benchmark [sv (subvec (into [] (range 1000000)) 0 1000000)]
                  (let [i (iter sv)]
                    (loop []
                      (if (.hasNext i)
                        (do (.next i)
                            (recur)))))
                  100)
Engine Master (ms) Patch (ms) Gain (master/patch)
V8 41979 3550 11x
jsc 29348 2405 12x

`ChunkedSeqs` could gain the same performance boost by implementing IIterable and utilizing `ranged-iterator` however this would deviate from Clojure implementation wise.



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

dnolen comment lost in system migration:

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





[CLJS-1643] Emit more informative error when emitting a type which has no emit multimethod case Created: 21/May/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

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

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

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

 Description   

Users may accidentally splice in Clojure functions and other things in macros that work in Clojure which cannot work in ClojureScript. We may want to include a default warning for the most common/likely error cases.



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 8:27 AM ]

Attached patch with fix and test.

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

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/417350ddabea283ef8f576b8e361a249d9bfb9e7





[CLJS-1816] Basic timing info in verbose output Created: 11/Oct/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: compiler, newbie

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

 Description   

When logging verbose output during compilation of individual namespaces we could include basic timing information to make it easier to spot slow-to-compile namespaces.



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 6:42 AM ]

Attached patch with fix.

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

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/6602f769ed4d52fd67577aacaf9cfe6db05b8ef3





[CLJS-1616] Self-host: improve documentation for compile-str Created: 06/Apr/16  Updated: 18/Nov/16  Resolved: 18/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Yehonathan Sharvit Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

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

 Description   

It's not clear at all how to use the `opts` arguments for compile-str.

In the code - https://github.com/clojure/clojurescript/blob/c3899acf797eb6779c53b313f5606c5018360b83/src/main/cljs/cljs/js.cljs#L660 - we
only have
:load - library resolution function, see load-fn
:source-map - set to true to generate inline source map information

In fact, there is also :verbose and ::def-emits-var

They are not documented.

Are there more options?



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 8:53 AM ]

Attached patch with fix.

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

dnolen comment lost in system migration:

fixed https://github.com/clojure/clojurescript/commit/33a7e5bcac763d40ca684404cf772e9745d264a





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

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

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

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

 Description   

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



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

Proposed Plan

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

An example of its implementation for PersistentArrayMap is:

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

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

A failing test case:

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

Attached patch with proposed fix and tests.

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

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





[CLJS-901] Protocolize compiler access to the file system Created: 03/Dec/14  Updated: 18/Nov/16

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

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


 Description   

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



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

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

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

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

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

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

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

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

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





[CLJS-1854] Self-host: Reload ns with const Created: 16/Nov/16  Updated: 16/Nov/16

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

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


 Description   

Bootstrapped ClojureScript fails to allow you to reload a namespace containing a constant.

To reproduce, evaluate the following forms in a REPL:

(require 'cljs.js)

(def st (cljs.js/empty-state))

(cljs.js/eval st
  '(require (quote foo.core))
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 1)"}))}
  prn)

(cljs.js/eval st
  '(require (quote foo.core) :reload)
  {:context :expr
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def ^:const x 2)"}))}
  prn)

The expectation is that the :reload directive in the last require will allow the namespace to be loaded with the const def being re-defined.

Instead, you get the following in the eval callback:

{:error #error {:message "Could not eval foo.core", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't redefine a constant at line 1 ", :data {:file nil, :line 1, :column 15, :tag :cljs/analysis-error}}}}

Note: This has probably been a defect in bootstrapped ClojureScript for quite a while (maybe forever). In particular, it is not a regression introduced with the new require capability (CLJS-1346).

FWIW, Planck has been working around this (and violating public API), manipulating cljs.js/*loaded* via its require REPL special, essentially purging portions of the analysis cache when reloading: https://github.com/mfikes/planck/blob/1.17/planck-cljs/src/planck/repl.cljs#L329-L348






[CLJS-1758] QuickStart guide fails at browser REPL step with "TypeError: parentElm is null" Created: 18/Aug/16  Updated: 16/Nov/16  Resolved: 18/Aug/16

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

Type: Defect Priority: Minor
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

When I follow http://clojurescript.org/guides/quick-start up to the browser REPL step, I get an error in the browser console and the REPL never connects. The browser error is TypeError: parentElm is null on line 482 of crosspagechannel.js: parentElm.appendChild(iframeElm);.

I have put up a self-contained example to demonstrate the bug: https://github.com/timmc/sscce-CLJS-1758

  • I am using this command to run the REPL: rlwrap java -cp cljs-1.9.216.jar:src clojure.main repl.clj
  • Refreshing the browser does not help.
  • Using Chromium 52.0.2743.116 instead of Firefox ESR 45.3.0 does not help.
$ java -version
java version "1.8.0_31"
Java(TM) SE Runtime Environment (build 1.8.0_31-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.31-b07, mixed mode)

$ uname -a
Linux bc-timmc 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2+deb8u3 (2016-07-02) x86_64 GNU/Linux
(Debian GNU/Linux 8.5)


 Comments   
Comment by Tim McCormack [ 18/Aug/16 11:27 AM ]

CLJS 1.9.89 fails the same way.

Comment by António Nuno Monteiro [ 18/Aug/16 11:51 AM ]

Not a bug. Your index.html file needs to at least have a `<body>` tag.
All html examples in the quickstart provide valid HTML, you just chose not to use it.

Comment by Tim McCormack [ 18/Aug/16 12:41 PM ]

Ah, of course! I compared the other files with the demo, but not that one!

I could have sworn the body element was always created automatically in the DOM whether or not it was declared. TIL.

(And confirmed, it works now.)

ETA: Not sure which resolution to pick, so leaving open.

Comment by HU Ze [ 16/Nov/16 4:17 AM ]

I am facing same problem before, I think you put your <script> in <head> and actually it MUST be put in <body>. To make it simply, just copy and paste from the Quick Start.





[CLJS-1853] Docstrings are included in compiled output Created: 15/Nov/16  Updated: 15/Nov/16

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

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


 Description   

Looking at code compiled with 'advanced':

$cljs$core$cst$0sym$0_STAR_print_DASH_base_STAR_$$, "target/release-browser/cljs/pprint.cljs", 13, 1, !0, 672, 675, $cljs$core$List$EMPTY$$, "The base to use for printing integers and rationals.", $cljs$core$truth_$$($cljs$pprint$STAR_print_base_STAR$$) ? $cljs$pprint$STAR_print_base_STAR$$.$cljs$lang$test$ : null]))]);

...

$cljs$core$cst$0sym$0cljs$0pprint$$, $cljs$core$cst$0sym$0_STAR_print_DASH_right_DASH_margin_STAR_$$, "target/release-browser/cljs/pprint.cljs", 22, 1, !0, 625, 630, $cljs$core$List$EMPTY$$, "Pretty printing will try to avoid anything going beyond this column.\nSet it to nil to have pprint let the line be arbitrarily long. This will ignore all\nnon-mandatory newlines.", $cljs$core$truth_$$($cljs$pprint$STAR_print_right_margin_STAR$$) ? $cljs$pprint$STAR_print_right_margin_STAR$$.$cljs$lang$test$ :

It looks like docstrings aren't stripped from dynamic vars, only from functions.

This is a bit of a waste of space...






[CLJS-1852] explain-data problems don't provide resolved symbols under :pred Created: 15/Nov/16  Updated: 15/Nov/16

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

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


 Description   

Same as http://dev.clojure.org/jira/browse/CLJ-2059 which has a patch.






[CLJS-1846] Range issues Created: 10/Nov/16  Updated: 15/Nov/16

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

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

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

 Description   

Problem
There are a number of bugs with Range which occur when the step size is 0 or where negative.

Examples

cljs.user=> (count (range 10 0 0))
-Infinity  ;Expect Infinity

cljs.user=> (nth (range 10 0 -1) -1)
11 ; Expected IndexOutOfBounds

cljs.user=> (take 5 (sequence identity (range 0 10 0)))
() ; Expected (0 0 0 0 0)

cljs.user=> (into [] (take 5) (range 0 10 0))
[] ; Expected [0 0 0 0 0]


 Comments   
Comment by David Nolen [ 10/Nov/16 4:37 PM ]

This patch is headed in the right direction but it needs to be more vigilant about performance. I'm more than happy to talk it over via IRC or Slack. Thanks!

Comment by Thomas Mulvaney [ 15/Nov/16 8:24 AM ]

Updated patch with performance tweaks.

  • Added the ^boolean annotation to the `some-range?` helper.
  • Removed calls to methods of Range where possible.
  • Improved 2-arity reduces performance over master significantly by replacing the call to ci-reduce.




[CLJS-1651] Self-host: Cannot replace core macro-function Created: 28/May/16  Updated: 14/Nov/16  Resolved: 14/Nov/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

Replace double to multiply by two in self host and you will see that operator-position resolution chooses the core macro:

ClojureScript Node.js REPL server listening on 54425
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval-str st "(defn double [x] (* 2 x))" nil {:eval cljs.js/js-eval :context :expr} identity)
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1
{:ns cljs.user, :value #object[cljs$user$double "function cljs$user$double(x){
return ((2) * x);
}"]}
cljs.user=> (cljs.js/eval-str st "[(double 3) (apply double [3])]" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value [3 6]}

The correct result above would be [6 6].



 Comments   
Comment by António Nuno Monteiro [ 13/Nov/16 11:27 AM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 13/Nov/16 12:07 PM ]

Confirmed that António's patch fixes things downstream in Planck.

Comment by David Nolen [ 14/Nov/16 1:43 PM ]

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





[CLJS-1764] Double warning for undeclared Var Created: 26/Aug/16  Updated: 14/Nov/16

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

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

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

 Description   

A regression occurred where an undeclared Var in a {{require}}d file causes two diagnostics:

$ more src/foo/core.cljs
(ns foo.core)

(def x 2)

abc
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.227.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52749
To quit, type: :cljs/quit
cljs.user=> *clojurescript-version*
"1.9.227"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit
$ rm -rf .cljs_node_repl
$ java -cp cljs-1.9.211.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56704
To quit, type: :cljs/quit
cljs.user=>  *clojurescript-version*
"1.9.211"
cljs.user=> (require 'foo.core)
WARNING: Use of undeclared Var foo.core/abc at line 5 /Users/mfikes/Desktop/src/foo/core.cljs
nil
cljs.user=> :cljs/quit


 Comments   
Comment by David Nolen [ 16/Sep/16 2:04 PM ]

If somebody wants to do a git bisect to sort this one out, that would be awesome

Comment by António Nuno Monteiro [ 07/Nov/16 9:34 AM ]

Only seems to happen at the REPL

Comment by António Nuno Monteiro [ 13/Nov/16 3:47 PM ]

Patch with fix.

This only happened when `require`ing at the REPL. Required namespaces ended up being analyzed twice, once in `cljs.repl` and once in `cljs.closure`. The patch adds wraps compiling these NSes in `cljs.closure` in a `cljs.analyzer/no-warn`.

Comment by David Nolen [ 14/Nov/16 9:24 AM ]

How will this not effect non REPL cases?

Comment by António Nuno Monteiro [ 14/Nov/16 9:29 AM ]

I just now realized that it will probably affect those cases as well, although the `add-dependencies` function seems to (currently) only be used in `cljs.repl`. What other approach should I try? Restrict the cases where we

*analyze-deps*
at the REPL?

Comment by Thomas Heller [ 14/Nov/16 9:51 AM ]

FWIW I don't think this is related to the REPL at all.

I have been seeing doubled warnings for a while now in shadow-build but never bothered to find you why.

abc

(defn x [y] xyz)

Will always warn twice about "xzy" but only once for "abc", doesn't matter if a REPL is involved or not.





[CLJS-1848] Analyzer can't find JS modules during macro-expansion Created: 12/Nov/16  Updated: 13/Nov/16  Resolved: 13/Nov/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1848-1.patch     Text File CLJS-1848-2.patch     Text File CLJS-1848.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 12/Nov/16 4:23 AM ]

Attached patch with fix.

Comment by David Nolen [ 12/Nov/16 11:43 AM ]

Is there some example demonstrating that this needs to be fixed?

Comment by António Nuno Monteiro [ 12/Nov/16 1:23 PM ]

Turned out the problem was related to finding processed JS modules during macro-expansion.

Changed the patch to add a mapping from the module provide to itself, such that macro-expansion can see that a module for that name exists.

Updated the module processing tests accordingly.

Comment by António Nuno Monteiro [ 12/Nov/16 6:04 PM ]

Actually disregard patch CLJS-1848-1.patch, as I've come to realize there's a much cleaner way to solve this issue. I'll provide a new patch shortly.

Comment by António Nuno Monteiro [ 13/Nov/16 4:06 AM ]

Patch CLJS-1848-2.patch contains the appropriate fix and explanation.

Comment by David Nolen [ 13/Nov/16 10:23 AM ]

fixed https://github.com/clojure/clojurescript/commit/1288204b043e00ca39b0c3c5af7fc8ac7eece816





[CLJS-1194] data_readers.cljc Created: 10/Apr/15  Updated: 12/Nov/16  Resolved: 12/Nov/16

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

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

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

 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

Comment by António Nuno Monteiro [ 12/Nov/16 11:06 AM ]

Attached patch with fix and tests.

Comment by David Nolen [ 12/Nov/16 11:55 AM ]

fixed https://github.com/clojure/clojurescript/commit/9484a134bdf039c10ec3c26c8aaa3acd0dcd9875





[CLJS-1851] Only output JS module processing time when `:compiler-stats` is true Created: 12/Nov/16  Updated: 12/Nov/16  Resolved: 12/Nov/16

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 12/Nov/16 6:36 AM ]

Attached patch with fix.

Comment by David Nolen [ 12/Nov/16 11:49 AM ]

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





[CLJS-1850] *unchecked-if* not declared ^:dynamic warning after commit a732f0 Created: 12/Nov/16  Updated: 12/Nov/16  Resolved: 12/Nov/16

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

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

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

 Description   

The following commit introduced a warning when requiring the analyzer in the `:cljs` branch:
https://github.com/clojure/clojurescript/commit/a732f07456c997b113a7c020e627cfe614ec31fc



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

Attached patch with fix.

Comment by David Nolen [ 12/Nov/16 11:48 AM ]

fixed https://github.com/clojure/clojurescript/commit/617ce7d4e33f65352be3d6d4865ace2996d65bcc





[CLJS-1849] Self-host: regression introduced by CLJS-1794 Created: 12/Nov/16  Updated: 12/Nov/16  Resolved: 12/Nov/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Self-hosted tests are erroring due to the fix introduced in CLJS-1794.



 Comments   
Comment by António Nuno Monteiro [ 12/Nov/16 5:15 AM ]

Attached patch with fix.

Comment by David Nolen [ 12/Nov/16 11:47 AM ]

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





[CLJS-1186] add :postamble option to compiler Created: 02/Apr/15  Updated: 11/Nov/16  Resolved: 10/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Michael Bradley Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cljs

Attachments: Text File cljs_1186.patch    

 Description   

Similar to CLJS-723:

1) :postamble's value will be a vector of paths
2) the compiled output is appended with the contents of the files at those paths
3) the generated source map points to the correct/adjusted line numbers



 Comments   
Comment by David Nolen [ 12/Feb/16 4:42 PM ]

Would like to hear more use cases for this one.

Comment by David Nolen [ 10/Aug/16 2:42 PM ]

Not going to do this one.

Comment by Tom Connors [ 11/Nov/16 4:28 PM ]

I've got a use case for this, if you're willing to reconsider. I'd like to set window.React and window.ReactDOM back to whatever they were before the script executed, like jQuery.noConflict and similar. So my preamble file would contain:
(function(){ var oldReact = window.React; and my postamble would contain: window.React = oldReact; }());





[CLJS-1844] Copy over GSoC externs parsing code Created: 08/Nov/16  Updated: 11/Nov/16  Resolved: 11/Nov/16

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

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


 Description   

We should copy over Maria Geller's externs parsing work https://github.com/mneise/clojurescript/commits/CLJS-1074



 Comments   
Comment by David Nolen [ 11/Nov/16 12:52 PM ]

fixed https://github.com/clojure/clojurescript/commit/498c1da144eee7011cf57a392274e9166ff146b7





[CLJS-1822] Use `:file-min` when processing JS modules with advanced optimizations Created: 15/Oct/16  Updated: 11/Nov/16

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

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

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

 Description   

Currently the `:file-min` option is ignored if a foreign lib is a JS module. Certain libraries produce 2 different artifacts, one which has development time checks, and a production-ready bundle which doesn't.

This patch proposes that `:file-min` in a JS module be fed to the Google Closure Compiler (instead of `:file`) when processing JS modules in `simple` or `advanced` compilation. This way, the development bundle of a JS module can be used with `:optimizations :none`, while the production-ready bundle can be used when compiling projects for production use.



 Comments   
Comment by António Nuno Monteiro [ 05/Nov/16 12:05 PM ]

Attached patch with fix and test.





[CLJS-1845] assoc on Subvec doesn't check bounds. Created: 09/Nov/16  Updated: 11/Nov/16  Resolved: 11/Nov/16

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

It is possible to call assoc on a Subvec at index n where n is anywhere in the bounds of the backing vector.

Example

cljs.user=> (assoc (subvec [0 1 2 3 4 5 6 7 8] 0 3) 8 0)
[0 1 2 3 4 5 6 7 0]

Expected behaviour

Throw an error: Index 8 out of bounds [0,3]



 Comments   
Comment by David Nolen [ 11/Nov/16 12:27 PM ]

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





[CLJS-1847] REPL should recognize `clojure.core/load` Created: 11/Nov/16  Updated: 11/Nov/16  Resolved: 11/Nov/16

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: repl

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

 Comments   
Comment by António Nuno Monteiro [ 11/Nov/16 10:39 AM ]

Attached patch with fix.

Comment by David Nolen [ 11/Nov/16 12:23 PM ]

fixed https://github.com/clojure/clojurescript/commit/140eb7a7b6213f7dfb5cc01ea5e95c267d510a8b





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

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

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

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

 Description   

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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






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

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

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


 Description   

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

Example:

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

Whereas foo.getBarRight expands to something like

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

foo.getBarWrong on the other hand expands to

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





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

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

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

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    

 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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

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



 Description   

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

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

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

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



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

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

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

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

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

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

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

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

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

to be true and emit a "return".

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

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

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

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

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

This issue occurs for me even without a let.

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

gives me

#object[SyntaxError SyntaxError: Unexpected token return]




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

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

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





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

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

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

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



 Description   

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

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

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

(ns test-def)

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

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

https://gist.github.com/4185793



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

code tags

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

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





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

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

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

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

 Description   

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

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

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



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

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

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

http://clojure.org/contributing

Comment by lvh [ 26/Jul/15 5:56 PM ]

I got bit by this bug. Working on figuring out if I can sign that agreement.

Comment by lvh [ 27/Jul/15 11:55 AM ]

This is a duplicate of CLJS-794.

Comment by Jake McCrary [ 04/Feb/16 6:58 PM ]

This patch changes string/replace-all to respect flags that were set on regexp passed as an argument.

I originally attached this to CLJS-794 and then noticed there was this older issue. I was unable to figure out how to edit at ticket to mark the patch as having "Code and Test" so I'm adding it to this issue instead.

I've signed a contributors agreement.

Comment by Mike Fikes [ 04/Feb/16 9:31 PM ]

There is a "sticky" flag y that could be conveyed.

http://www.ecma-international.org/ecma-262/6.0/index.html#sec-get-regexp.prototype.sticky

Comment by Jake McCrary [ 06/Feb/16 10:50 AM ]

Reading a bit more about it and looks like both 'u' and 'y' are newly supported in ECMA6. Is there a way to write tests that exercise this functionality? I've got changes locally to get support of 'y' but felt the need to write a test for it (as its a bit more complicated than simply looking for a flag) and am hung up on having EMCA6 support while running the tests.

I'm actually wondering if having 'u' and 'y' in there is a bit premature. Any guidance on whether adding code for 'u' and 'y' should be done (or removing 'u' from my patch) or testing ClojureScript with ECMAScript 6 support?

Comment by Mike Fikes [ 06/Feb/16 12:51 PM ]

One thought: Whatever this is exercising passes on recent versions of V8, JavaScriptCore, SpiderMonkey, Nashorn, and ChakraCore: https://github.com/clojure/clojurescript/blob/628d957f3ecabf8d26d57665abdef3dea765151e/src/test/cljs/cljs/core_test.cljs#L1472

It does seem tricky to write a robust RegExp clone implementation, and if you do some googling you see people dealing with u and y as special cases. The patch seemed OK to me with respect to this, but I'm not a JavaScript expert.





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

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

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

r2173



 Description   

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

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



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

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

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

Bump, this enhancements sound simple & fine.

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

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

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

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

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

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

Be my guest





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

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

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

Attachments: Text File CLJS-794.patch    

 Description   

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

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

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

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

Thanks



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

A patch is welcome for this. Thanks.

Comment by lvh [ 27/Jul/15 11:55 AM ]

This appears to be identical to CLJS-485, which has a patch (by someone who hasn't signed the CLA yet).

Comment by Jake McCrary [ 04/Feb/16 6:43 PM ]

This patch changes string/replace-all to respect flags that were set on regexp passed as an argument.





[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 08/Nov/16

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

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

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

 Description   

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

(deftype Foo [default])

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


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

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

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

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

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

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

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

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

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

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

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

Updated to use new refactorings

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

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





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

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

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

Linux 64bit

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


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

 Description   

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

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

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

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

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



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

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

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

I did bind out to err. Check the patch.

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

Crispin, oops sorry you are correct. Thanks.

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

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

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

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

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

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

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

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





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

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

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


 Description   

Problem for using boolean Closure defines



 Comments   
Comment by Francis Avila [ 30/Mar/15 12:02 PM ]

I am unsure if this is the same issue, but forms like ^boolean (js/isFinite n) also do not seem to analyze correctly: if, and, and or will still emit a call to truth_.





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

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

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


 Description   

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






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

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

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


 Description   

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






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

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

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

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

 Description   

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



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

A patch that caches upstream dependencies in the compiler env.

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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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



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

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





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

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

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


 Description   

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






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

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

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


 Description   

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






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

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

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

Quick Start Browser REPL with :watch off



 Description   

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

Then further down, where the new symbol is introduced

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

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

(def a 1)
(def a 1)

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

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

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





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

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

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


 Description   

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






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

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

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

Quick Start Browser REPL (OS X / Safari)



 Description   

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

To reproduce:

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

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

But:

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

Now, if you :reload

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


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

Prior to step 8:

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

Between steps 9 and 10:

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

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

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

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





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

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

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

Quick start guide with Node REPL



 Description   

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

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

ClojureScript REPLs do not behave in the same way:

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





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

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

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


 Description   

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






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

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

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





[CLJS-1195] generic reusable command line argument parsing for REPLs Created: 10/Apr/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Jason Courcoux
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

REPLs are more or less started in the same way and all the builtin ones provide a -main entry point. We should supply reusable command line argument parsing that any REPL can use to get standard command line driven start.



 Comments   
Comment by Jason Courcoux [ 30/Sep/16 3:27 AM ]

Just wanted to capture my initial thoughts in case I'm going down the wrong road, or overthinking it and someone wants to point me in a different direction. I can see the following options for parsing the command line arguments - in no particular order:

1) Reuse a third party such as clojure/tools.cli

  • Less to maintain within the ClojueScript codebase itself.
  • Supports GNU option parsing conventions
  • Extra dependency - Guessing this is a a definite no for various reasons, but don't want to assume anything.
  • Is it over complicated for our needs here?

2) Reuse something in the java platform - looks like there is a class sun.tools.jar.CommandLine which has very basic functionality for parsing command line arguments.

  • Already in the Java platform, although I believe this is probably only in the JDK so probably no good for this use case.
  • Very limited support - would be easier to replicate the functionality in clojure code.

3) Use the clojure reader to just read in clojure data

  • Nice and simple, and reusing something that already exists
  • Arguments would be in the same format as they are now
  • No validation of parameters passed in.

4) Custom parsing of arguments - wondering if we could do something with clojure spec and allow repls to pass a spec which could be used to infer how to parse/validate the data (e.g. for port number is it an int or string).

  • Leveraging spec gives repls a mechanism to specify constraints, and can get clear errors out
  • Can be more flexible in the arguments accepted - i.e. --port "9000" and --port 9000 could both be valid
  • I've not done much with spec so although I think this sounds feasible I'm not 100%

I think I'm going to explore option 4, and I'll update as I go.

Comment by David Nolen [ 30/Sep/16 6:09 AM ]

Thanks for writing this up. 1) tools.cli is not a bad idea but do we need it. 3) seems Clojure-y - we just want typical CLI support. 4) Clojure 1.9 is alpha we don't want a dependency on this.

My original thought was to just replicate what clojure.main does - I don't see why we need anything more.

Comment by Jason Courcoux [ 30/Sep/16 9:45 AM ]

Thanks for the quick response. I've had a look at clojure.main, and as far as I can tell it doesn't do anything in the way of generic parsing of arguments - The main function dispatches based on some known options (repl/main/help etc) and passes the rest of the arguments through - in each case it just binds the arguments to command-line-args which may or may not get parsed/accessed at a later point either during startup, or from the repl session - neither of these seem to be what this Jira is asking for, unless I've misunderstood.

Just so I'm 100% on what's being asked here - this ticket is for parsing repl environment options, i.e. for the browser repl the options would be host/port/working-dir/serve-static etc, and the parsing would need to handle strings/int/boolean values etc.

I'm conscious you're probably very busy, I'm almost certainly missing something, and don't want to take up too much of your time, so if you tell me it's there in clojure.main I'll keep digging until I find it.

Comment by David Nolen [ 30/Sep/16 10:48 AM ]

We're not at all interested in exposing all the options via command line flags. The first step is simply mirroring Clojure's REPL options that make sense. For all the CLJS REPL specific stuff a flag which takes string of EDN or an EDN config file is fine.





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

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

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


 Description   

We should emit a simple warning if a namespace doesn't not appear to be unique on the classpath.






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

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

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

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

 Description   

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

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

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



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

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

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

OK

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

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

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

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

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

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

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

Sure! Have you submitted your CA yet?

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

Yes, I did yesterday.

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

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

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

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

Experience report:

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

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

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

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

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

Is this the same approach taken by Clojure?

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

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

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

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

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

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

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

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

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

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





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

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

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

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

 Description   

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



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

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

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

Thanks will review.

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

cljs-1300.patch no longer applies on master





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

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

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


 Description   

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

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

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

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

Analysis

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

Record literals

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

EDIT: Record literals can also go into the constant pool

cljs.reader

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

Related Tickets



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

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

I'm all for this enhancement.





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

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

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


 Description   

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






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

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

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


 Description   

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






[CLJS-1222] Sequence of a stateful transducer is producing the wrong answer Created: 24/Apr/15  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Lucas Cavalcanti Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, cljs, collections
Environment:

OSX 10.10.3, java 1.8.0-ea-b124



 Description   

I'm producing more than one element on the 1-arity of the transducer, and sequence is only considering the last one.

Here is the transducer and the tests that fail for sequence:

(defn sliding-window [n]
  (fn [rf]
    (let [a #js []]
      (fn
        ([] (rf))
        ([result]
         (loop [] ;; Here I'm emitting more than one element
           (when (not-empty a)
             (rf result (vec (js->clj a)))
             (.shift a)
             (recur))))
        ([result input]
         (.push a input)
         (if (== n (.-length a))
           (let [v (vec (js->clj a))]
             (.shift a)
             (rf result v))
           result))))))

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))


 Comments   
Comment by Lucas Cavalcanti [ 24/Apr/15 11:18 AM ]

I could make it work by recurring on the result.

([result]
  (loop [res result]
    (if (not-empty a)
      (let [v (vec (js->clj a))]
        (.shift a)
        (recur (rf res v)))
      res)))

even so it's weird that the previous version behaves differently on core.async and sequences in cljs and clj

Comment by David Nolen [ 26/Apr/15 4:04 AM ]

Please demonstrate the problem without core.async. Thanks.

Comment by Lucas Cavalcanti [ 26/Apr/15 7:32 PM ]

Hi,

the last test I posted on the ticket, fails in cljs, but not in clj:

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))
Comment by David Nolen [ 27/Apr/15 7:43 AM ]

I've removed the core.async bits from the description to clarify the issue.

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

The implementation of sliding-window above does not appear to be correct, it doesn't return the result. This ticket needs more information.

Comment by Lucas Cavalcanti [ 10/May/15 3:51 PM ]

As I said on http://dev.clojure.org/jira/browse/CLJS-1222?focusedCommentId=38620&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-38620

changing the 1-arity of the sliding-window to that fixes the transducer.

The point of this ticket now is that the behavior of the same (wrong) transducer in clj (both core.async and sequence) and cljs (core.async) is different than cljs sequence.





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

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

Type: Defect Priority: Minor
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ns-unmap

Attachments: Text File 0001-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch     Text File 0002-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch    
Patch: Code

 Description   

In ClojureScript, using ns-unmap on a symbol from cljs.core doesn't exclude it from the current namespace. Note that both a function and a macro still exist, even after unmapping:

To quit, type: :cljs/quit  
cljs.user=> (ns-unmap 'cljs.user 'when) ;; macro
true  
cljs.user=> (ns-unmap 'cljs.user 'not)  ;; function
true  
cljs.user=> (when 1 2)  
2  
cljs.user=> (not false)  
true  

This differs from the behavior of Clojure's ns-unmap. Note the appropriate errors when attempting to use unmapped symbols:

Clojure 1.7.0-beta1
user=> (ns-unmap 'user 'when) ;; macro
nil
user=> (ns-unmap 'user 'not)  ;; function
nil
user=> (when 1 2)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: when in this context, compiling:(NO_SOURCE_PATH:11:1) 
user=> (not false)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: not in this context, compiling:(NO_SOURCE_PATH:12:1) 

Somehow ClojureScript's ns-unmap needs to add the symbol to the current namespace's :excludes set. Note that the def special form does this already (after it displays a warning).

We have two solutions. 0001 extends the ns form's :merge behavior to support :excludes, and then uses this in ns-unmap. If the enhancement to ns isn't wanted, patch 0002 changes ns-unmap to update :excludes directly.



 Comments   
Comment by David Nolen [ 05/May/15 7:23 AM ]

The second patch is preferred. However it seems the second patch is too permissive. The :excludes logic should only be applied if the symbol identifies a core macro or fn.

Comment by Chouser [ 05/May/15 3:46 PM ]

The ns form's own :refer-clojure :exclude accepts arbitrary symbols and adds them to the namespace's :excludes set, which seems like the same permissiveness problem. Do you want a patch that addresses the permissiveness of both ns and ns-unmap in this ticket, or should such a patch go in a new ticket?

Comment by David Nolen [ 05/May/15 4:08 PM ]

New ticket to fix the bug that :exclude doesn't check the symbol list for cljs.core declared vars, and an updated patch here please.





[CLJS-1238] Setting *main-cli-fn* when using :target :nodejs shouldn't be manditory Created: 01/May/15  Updated: 08/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Jeremy Shoemaker Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File nodejs-main-cli-fn.patch    
Patch: Code

 Description   

Currently, when you use :target :nodejs in the build options for ClojureScript, the resulting code requires you to set *main-cli-fn* to a function.

This prevents someone from writing a library that can be used by JavaScript developers because it forces code execution on require. It also makes writing a CLI tool that can be distributed using NPM less straightforward. I ran into this issue trying to create a Leiningen template for writing CLI tools that could be installed using npm install or npm link. I had a wrapper script to take care of the CLI use-case, and intended to write the ClojureScript module in a more library oriented way, but ran into issues. I could work around this by not using the wrapper script, but it got me thinking about the more general library issue.

I don't see any reason why you should be forced to set *main-cli-fn* and so I'm suggesting making it optional.

Attached is a patch that makes it optional but retains the check for whether the value it is set to is a function in the case where it is set.

This is my first time submitting a change to a project using a git patch and not a pull request, so let me know if I've made the patch wrong.



 Comments   
Comment by Jeremy Shoemaker [ 01/May/15 7:27 PM ]

I just noticed the priority defaulted to "Major". I don't know if I'd say it's major, so feel free to bump it down if that doesn't seem appropriate.

Comment by Ning Sun [ 18/Feb/16 4:08 AM ]

+1.

I was working on a clojurescript library and going to build it as a node library. Currently blocked by this.

Comment by Mike Fikes [ 20/Feb/16 8:07 AM ]

Patch no longer applies.





[CLJS-1271] Missing warning when assigning namespaces via def Created: 17/May/15  Updated: 08/Nov/16

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

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


 Description   

Currently you can assign a Closure namespace to a var without getting a warning.

Minimal sample:

(ns import-names.core
  (:import [goog debug]))

(def debug goog.debug)


 Comments   
Comment by David Nolen [ 29/May/15 12:30 PM ]

The example case is a bit complected. Besides importing a name that matches a def you are also assigning a google closure namespace to a local. This will likely cause problems on its own. We need more information.

Comment by Sebastian Bensusan [ 29/May/15 12:46 PM ]

We should check that :require ed and :import ed namespaces are not used as values and then warn about it.





[CLJS-1286] REPL environment should be able to provide advice if source mapping fails Created: 23/May/15  Updated: 08/Nov/16

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

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


 Description   

For example browser REPL will often need users to supply :host-port, :host, and :asset-path in order to correctly parse files from stacktraces.






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

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

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


 Description   

Edge case in / usage, EventType/CLICK does not trigger a warning. Foo/bar always means that Foo is a namespace, it cannot be used for the static field access pattern common in Java as there's no reflection information in JavaScript to determine this.






[CLJS-1412] Add JSDoc type information to individual IFn methods Created: 10/Aug/15  Updated: 08/Nov/16

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

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


 Description   

Propagate user supplied docstring type information to the various fn arities so that more code may be checked.






[CLJS-1415] Handling JSDoc param name [x] optional syntax Created: 10/Aug/15  Updated: 08/Nov/16

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

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





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

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

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


 Description   

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






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

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

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


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

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

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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

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



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

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

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

we should not allow arities above 21 args

Emitting an analyzer warning is what you want?

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

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

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

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





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

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

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


 Description   

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






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

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

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


 Description   

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






[CLJS-1419] enhance numeric inference, if + number? test on local var should tag local var in the successful branch Created: 12/Aug/15  Updated: 08/Nov/16

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

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


 Comments   
Comment by David Nolen [ 12/Aug/15 6:44 AM ]

One small complication is dealing with and as it has an optimizing case.





[CLJS-1453] cljs.compiler/load-libs does not preserve user expressed require order Created: 17/Sep/15  Updated: 08/Nov/16

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

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


 Description   

Due to putting the requires into a map the original order is lost. This is a problem primarily when order specific side effects are present in the required namespaces.






[CLJS-1485] Error when requiring `goog` namespace in a ns declaration Created: 10/Nov/15  Updated: 08/Nov/16

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

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


 Description   

I wanted to use functions from goog namespace although--as I found out later, I didn't have to because goog is already exists in my namespace. So, I put (:require [goog]) in a ns declaration. Then, when I tried to reload that particular namespace by doing :require :reload in a cljs repl, I got:

Error: Namespace "x.x.x" already declared.

Doing :require :reload again in the cljs repl makes the repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)

I tested the steps below using clojurescript 1.7.145 and 1.7.170.

Here are the steps to reproduce which are taken from clojurescript quickstart-browser repl section:

1. Download the standalone clojurescript 1.7.170 jar https://github.com/clojure/clojurescript/releases/download/r1.7.170/cljs.jar

2. Create a directory hello_world and copy the JAR into that directory, then from inside the hello_world directory:

mkdir -p src/hello_world;touch repl.clj;touch index.html;touch src/hello_world/core.cljs

3. repl.clj content

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

(cljs.build.api/build "src"
  {:main 'hello-world.core
   :output-to "out/main.js"
   :verbose true})

(cljs.repl/repl (cljs.repl.browser/repl-env)
  :watch "src"
  :output-dir "out")

4. index.html content

<html>
    <body>
        <script type="text/javascript" src="out/main.js"></script>
    </body>
</html>

5. src/hello_world/core.cljs content

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]))

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

(enable-console-print!)

(println "Hello world!")

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

6. run clojurescript repl

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

7. Open http://localhost:9000 in browser (I use google chrome). Open javascript console.

8. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)

10. Look the browser javascript console. Nothing new shown.

11. Quit from the repl using :cljs/quit

12. Add [goog] in ns declaration in src/hello_world/core.cljs so that the content of the file becomes:

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]
            [goog]))

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

(enable-console-print!)

(println "Hello world!")

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

13. Run the clojurescript repl again

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

14. Now refresh the http://localhost:9000 in browser. Make sure the javascript console stays open.

13. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)
;;=> nil

it just returns nil

15. See the javascript console, it shows

Uncaught Error: Namespace "hello_world.core" already declared.

16. Executing this expression again (require '[hello-world.core :as hello] :reload) shows nothing new in the browser's javascript console while the clojurescript repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)





[CLJS-1544] cljs.test REPL reload support Created: 13/Jan/16  Updated: 08/Nov/16

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

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


 Description   

When adding a test to a test ns that uses cljs.test and re-loading (via require + :reload) that namespace in the REPL after saving the file - invoking run-tests does not include the newly added test.






[CLJS-1641] Multi-arity defn copies arguments unnecessarily for all cases Created: 16/May/16  Updated: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Stephen Brady Assignee: Stephen Brady
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Background:

Passing js arguments as a parameter to another function is a known performance issue. Copying arguments to an array addresses this, and this approach has been taken to handle args passing for variadic functions in previous patches such as:
https://github.com/clojure/clojurescript/commit/dccc5d0feab9809cb56bd4d7257599a62232ac0d
https://github.com/clojure/clojurescript/commit/f09bbe62e99e11179dec6286fbb46265c12f4737

This commit (https://github.com/clojure/clojurescript/commit/576fb6e054dd50ec458a3c9e4172a5a0002c7aea) introduced a macro path for `defn` forms.

Current Behavior and Impact:

In the case of multi-arity defn forms, the macro path generates an array copy of the arguments variable regardless of whether it is used or necessary. In the case of multiple arities but no variadic arity, copying arguments is unnecessary as arguments will not be passed to the variadic method for the given function. In the case of multiple arities with a variadic case, an args array copy is needed but should be isolated to that case alone; currently, the array copy is performed before checking the arguments length, causing all cases to incur an (unused) args array copy.

Relevant code: https://github.com/clojure/clojurescript/blob/master/src%2Fmain%2Fclojure%2Fcljs%2Fcore.cljc#L2827-L2843

Recommended Change:

  • Do not perform an args array copy before switching on arguments length
  • Perform an args array copy within the variadic dispatch case

Patch forthcoming.



 Comments   
Comment by David Nolen [ 21/May/16 5:03 PM ]

Thanks! Will take a look.

Comment by David Nolen [ 10/Jun/16 1:32 PM ]

This probably needs to be updated in the compiler as well.

Comment by Stephen Brady [ 10/Jun/16 2:43 PM ]

The compiler already isolates the args to array copying behavior in the variadic case. The unnecessary copying is isolated to the defn macro.

These are the only two calls to `emit-arguments-to-array`:

Variadic function: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L743
Multi-arity with variadic case: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L812

Comment by Francis Avila [ 11/Jun/16 8:33 AM ]

From what I can see copy-arguments is only ever used with an immediately-constructed empty array as the dest in the pattern:

`(let [arr# (array)]
  (copy-arguments arr#)
  ;;...
  )

Perhaps it should change to have the exact same behavior as emit-arguments-to-array, i.e. it should take a start index and expand to the name of the destination array. The advantages of this approach are 1) it can preallocate the array to the correct size and 2) your patch no longer needs a slice call--you can avoid allocating two arrays instead of just one. (These two reasons are why I implemented emit-arguments-to-array the way I did.)

I can't think of a way to implement emit-arguments-to-array as a macro without emitting a wrapping js function which messes up the scope, but you could do this:

(defmacro copy-arguments [dest-arr startslice]
  `(loop [i# 0]
     (when (< i# (alength ~dest-arr))
       (aset ~dest-arr i# (aget (js-arguments) (+ i# startslice)))
       (recur (inc i#)))))

And then at the callsite:

`(let [variadic-args-arr# (make-array (- (alength (js-arguments)) ~maxfa))]
  (copy-arguments variadic-args-arr# ~maxfa)
    (let [argseq# (new ^::ana/no-resolve cljs.core/IndexedSeq
                       variadic-args-arr# 0 nil)]
      ;; ...
    ))

However, there are some bugs around arity handling that the above would not solve: CLJS-1678

Comment by Stephen Brady [ 11/Jun/16 8:27 PM ]

Francis, thanks for commenting on this. The patch that I submitted simply moves where/when `copy-arguments` is called. Other than that, I preserved all other aspects of the existing implementation, including how the array is built up and then made into an IndexedSeq. The line diffing in the patch implies that I changed a lot more than I really did. Agreed with your point that `emit-arguments-to-array` is more efficient and precise. Intentionally, I did not try to alter/improve/correct anything in this patch beyond solving the objective in the issue: not unnecessarily copying arguments.

Glad you've reported CLJS-1678 as I've observed this too. This buggy behavior shows up in several places. Beyond what this issue-patch attempts to address, in general, my observation is that we could probably clear out some under-brush that's accumulated as the compiler has matured with regards to arguments handling and code generated for multi-arity and/or variadic functions, apply / applyTo, and implementations of IFn. Seems like there are several opportunities to emit less javascript, create fewer intermediates, and shorten the call chain.

So to reiterate, this patch - despite its superficial appearance - changes very little and just moves the call to copy-arguments to the appropriate place. The benefit is:

For multi-arity functions with no variadic arity, no code for copying the arguments to an array is emitted at all (which in aggregate turns out to be a decent amount). Obviously, at runtime, no array will be created.

For multi-arity functions with a variadic arity, the code for copying the arguments remains but is isolated to the variadic case, and so if the function is called but will dispatch to one of the fixed arities, again, no array will be created.





[CLJS-1795] Support more options in the `:closure-warnings` compiler option Created: 28/Sep/16  Updated: 08/Nov/16

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

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


 Description   

Support more options in the `:closure-warnings` compiler option as per https://github.com/google/closure-compiler/wiki/Warnings#warnings-categories






[CLJS-1745] refer-clojure doesn't pull in previously excluded vars Created: 14/Aug/16  Updated: 08/Nov/16  Resolved: 08/Nov/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Observe this behavior in Clojure:

user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
CompilerException java.lang.RuntimeException: Unable to resolve symbol: juxt in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> partial
CompilerException java.lang.RuntimeException: Unable to resolve symbol: partial in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
#object[clojure.core$juxt 0x2f62ea70 "clojure.core$juxt@2f62ea70"]
foo.core=> partial
#object[clojure.core$partial 0x38aa816f "clojure.core$partial@38aa816f"]

Compare with ClojureScript:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 49402
To quit, type: :cljs/quit
cljs.user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
nil
foo.core=> partial
nil
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
nil
foo.core=> partial
nil


 Comments   
Comment by António Nuno Monteiro [ 07/Nov/16 12:57 PM ]

Attached patch with fix.

Comment by David Nolen [ 08/Nov/16 1:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/2fb9d1914a932adc833bd82044551205d5793738





[CLJS-1666] Flag to optionally disable transit analysis cache encoding Created: 03/Jun/16  Updated: 08/Nov/16  Resolved: 08/Nov/16

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

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

Attachments: Text File CLJS-1666-1.patch     Text File CLJS-1666.patch    
Patch: Code

 Description   

We need a flag to explicitly disable the transit encoding - this is to work around code that creates unencodeable forms for one reason or another. EDN had become more lax about encoding random Objects which allowed this stuff to go under the radar in the past.



 Comments   
Comment by Wilker Lúcio da Silva [ 06/Jun/16 9:10 PM ]

I'm having a compilation issue with ClojureScript 1.9.36, I'm writing an app that uses untangled and this file https://github.com/untangled-web/untangled-client/blob/f42088c84b059562a48455a71daa6e4ea08d286c/src/untangled/client/data_fetch.cljs fails to compile with Caused by: java.lang.RuntimeException: java.lang.Exception: Not supported: class cljs.tagged_literals.JSValue this issue doesn't happen with 1.9.14

Comment by David Nolen [ 06/Jun/16 10:06 PM ]

We should investigate whether just handling JSValue + other cases like clojure.lang.Var is good enough.

Comment by David Nolen [ 08/Jun/16 8:53 AM ]

JSValue encoding/decoding landed in master fixing part of the issue. Still need more information about the Var case however.

Comment by António Nuno Monteiro [ 07/Nov/16 3:17 PM ]

Attached patch with fix.

Comment by Thomas Heller [ 07/Nov/16 3:53 PM ]

Will the toggle to :edn not just "hide" a problem?

Say the user somehow manages to break the transit encoding by using custom value. A google search will reveal the solution of switching to :edn. But since the custom value runs through the "default" handler of :edn there is no correct way to re-build it when reading the cache. Instead it will just be a placeholder TaggedValue without meaning. If anyone ever tries to access it we now have a case where it will work without caching but not with a cached value.

I think caching should just gracefully fail instead of adding a broken alternative?

Comment by Thomas Heller [ 08/Nov/16 4:13 AM ]

As per Slack discussion I opened a separate issue to focus on the actual problem.

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

I still think however that the newly introduced :cache-analysis-format is not useful to the user. If Transit is available it should always be used as it is the "better" option. I'm not sure there is a benefit of letting the user decide other than maybe work around issues that Transit has that EDN might not have. Hiding the actual problem by doing so.

If the cache write "fails" for any reason it should just warn about it instead of failing the build.

Comment by António Nuno Monteiro [ 08/Nov/16 7:44 AM ]

Attached CLJS-1666-1.patch which takes into account feedback obtained in Slack.
Specifically:

  • added validation for the `:cache-analysis-format` option
  • explicit inference of the cache file format via the new function `cache-analysis-ext`
Comment by David Nolen [ 08/Nov/16 12:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/80bbded9527dce6216f36763f23b7e5ebf8930df





[CLJS-1843] EDN analysis cache may write unusable data Created: 08/Nov/16  Updated: 08/Nov/16

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

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


 Description   

EDN has a built-in default writer for all objects, this may cause the cache to write something like

#object[Thing "thing-str"]
that cannot be read to construct an actual Thing instance.

This leads to an issue when trying to use the analysis data since it will contain different things when coming from cache or not.

This issue was highlighted by transit since it has no default writer and didn't know how to encode JSValue. [1] Instead of writing something unusable it failed early.

The cache write should rather gracefully fail (and warn) instead of writing unusable data or exploding.

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






[CLJS-1840] Provide more descriptive error message when invalid libspec detected Created: 06/Nov/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: errormsgs

Attachments: Text File CLJS-1840.patch     Text File CLJS-1841.v1.patch    
Patch: Code and Test

 Description   

When an invalid libspec is detected while compiling, the compiler throws an error which doesn't specify what the invalid libspec is. Combined with the line/column number referring to the start of the file, this can make it difficult to track down what is causing the problem.

I ran into this when an incorrect git merge left the ns spec in an invalid state:

(ns re-frame.fx-test
  (:require
    [cljs.test :refer-macros [is deftest async use-fixtures]]
    [cljs.core.async :as async :refer [<! timeout]]
    [clojure.string :as str]
    [re-frame.core :as re-frame]
    [re-frame.fx]
    [re-frame.interop :refer [set-timeout!]])
    [re-frame.loggers :as log]
  (:require-macros [cljs.core.async.macros :refer [go]]))

Even after looking at the ns form a few times, I still couldn't see what was wrong.

I've attached a patch which prints the invalid libspec in the error message. The new error message should make it much easier to find and fix the problem.



 Comments   
Comment by Daniel Compton [ 06/Nov/16 4:49 PM ]

Updated with ticket number in commit message.

Comment by David Nolen [ 07/Nov/16 2:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/78f6504e1c67c935c283a362ecf6cb5a3766022e





[CLJS-1794] incomplete alias created for namespace cljs.spec warning under advanced compilation Created: 26/Sep/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

Type: Defect Priority: Major
Reporter: Michael Glaesemann Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

λ lein --version
Leiningen 2.6.1 on Java 1.8.0_45 Java HotSpot(TM) 64-Bit Server VM


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

 Description   

Under advanced compilation, use of cljs.spec can cause analyzer warnings.

out/cljs/analyzer.js:4740: WARNING - incomplete alias created for namespace cljs.spec
var mchk = (function (){var and__6935__auto__ = cljs.spec;
                                                ^

Sep 26, 2016 7:18:00 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 1 warning(s)
Successfully compiled "out/main.js" in 43.391 seconds.


 Comments   
Comment by Michael Glaesemann [ 26/Sep/16 6:28 PM ]

I came across this when upgrading Om from alpha44 to alpha45. The commit that changed the behavior moved to from cljs to cljc for server-side rendering support. (https://github.com/omcljs/om/commit/439802239fdfb95b143cde33df26be7801069bf2)

Test case: https://gist.github.com/grzm/0d2db1d1364daeb6118b610c2fc60178

António Nuno Monteiro pointed out:
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L2666

            (let [mchk  #?(:clj  (some-> (find-ns 'clojure.spec)
                                   (ns-resolve 'macroexpand-check))
                           :cljs (and ^::no-resolve cljs.spec
                                      ^::no-resolve cljs.spec/macroexpand-check))
Comment by Antonin Hildebrand [ 10/Oct/16 9:27 AM ]

I'm also seeing it: https://travis-ci.org/binaryage/dirac/builds/166433885#L815

Comment by António Nuno Monteiro [ 07/Nov/16 11:01 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 07/Nov/16 2:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/88ff3178509ffe93f2522eaeb6a8e2f63163605a





[CLJS-1842] Remove analyzer `:merge` hack for REPLs Created: 07/Nov/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

Type: Task Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1842.patch    

 Description   

This is not needed anymore now that `require` et al. are a thing.



 Comments   
Comment by António Nuno Monteiro [ 07/Nov/16 12:52 PM ]

Attached patch.

Comment by David Nolen [ 07/Nov/16 2:36 PM ]

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





[CLJS-1768] cljs.spec perf tweaks Created: 31/Aug/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

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

Attachments: Text File CLJS-1768.patch    

 Description   

Same as the following commits:

https://github.com/clojure/clojure/commit/de6a2b528a18bcb4768e82d0d707d2cab26268a6
https://github.com/clojure/clojure/commit/defa7b8ef268ea2b8772658ade2010ca5ad00dc4



 Comments   
Comment by António Nuno Monteiro [ 07/Nov/16 8:12 AM ]

Attached a patch with fix.

To make it easy to review, I've pushed a branch with the 4 separate (Clojure) commits that make up this patch here: https://github.com/anmonteiro/clojurescript/tree/spec-perf-tweaks

Comment by David Nolen [ 07/Nov/16 2:36 PM ]

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





[CLJS-1622] `with-redefs` can cause `&` argument to be assigned incorrectly under advanced compilation Created: 12/Apr/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

Type: Defect Priority: Minor
Reporter: Peter Jaros Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Given the following:

(ns with-redefs-bug.core)

(enable-console-print!)

(defn a-function [arg-1 arg-2 & args]
  nil)

(with-redefs [a-function (fn [& args] args)]
  (prn (a-function :arg-1))
  (prn (a-function :arg-1 :arg-2))
  (prn (a-function :arg-1 :arg-2 :arg-3)))

Under :optimizations :none, this code correctly prints:

(:arg-1)
(:arg-1 :arg-2)
(:arg-1 :arg-2 :arg-3)

However, under :optimizations :advanced, this code prints:

(:arg-1)
(:arg-1 :arg-2)
:arg-1

That is, as long as the function is called with exactly or less than the number of non-variadic arguments in the original function bound to a-function, args is (correctly) bound to a seq of all the arguments, but if any more arguments are given, args is bound to the first argument.

Also, under either compilation, the following warning is generated:

WARNING: Wrong number of args (1) passed to with-redefs-bug.core/a-function at line 9

That surprises me, but since it happens under both methods, perhaps it's intentional.



 Comments   
Comment by Peter Jaros [ 12/Apr/16 4:21 PM ]

Reproduced on r1.8.40 and current master (29eb8e0).

Comment by Mike Fikes [ 12/Apr/16 8:24 PM ]

This is actually not specific to :optimizations :advanced, but to :static-fns true.

Comment by António Nuno Monteiro [ 05/Nov/16 9:22 AM ]

This seems to be the same case as CLJS-1623.

If we revise the failing example to include the `^:dynamic` metadata, it works as expected.

Comment by David Nolen [ 07/Nov/16 10:48 AM ]

This is not a bug.





[CLJS-1518] Case macro expansion evaluates expression twice Created: 21/Dec/15  Updated: 07/Nov/16

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

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

The issue is present in version 1.7.189.


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

 Description   

The value being checked is evaluated twice if all of the test values are keywords.

(macroexpand-1 '(case (expensive) :a 1 2))
(cljs.core/let [G__123555 (if (cljs.core/keyword? (expensive)) (.-fqn (expensive)) nil)]
  (case* G__123555 [["a"]] [1] 2))


 Comments   
Comment by Mike Fikes [ 31/Jan/16 11:38 PM ]

Patch takes advantage of the existing gensym as a temp place to stash the evaluated value before test / FQN conversion.

Adds a unit test specifically checking for single evaluation in this case.

Comment by Mike Fikes [ 31/Jan/16 11:40 PM ]

With the patch, Darrick's macroexpansion example becomes:

(cljs.core/let [G__7663 (expensive) 
                G__7663 (if (cljs.core/keyword? G__7663) (.-fqn G__7663) nil)] 
  (case* G__7663 [["a"]] [1] 2))
Comment by António Nuno Monteiro [ 07/Nov/16 10:11 AM ]

Still present in 1.9.293.





[CLJS-1533] Null pointer error on non-existent file in advanced mode Created: 01/Jan/16  Updated: 07/Nov/16

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

Type: Defect Priority: Trivial
Reporter: Glen Mailer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: error-reporting


 Description   

When building without optimisations, having a foreign-lib point to a non-existent file gives a fairly clear FileNotFoundException

When doing the same build but with advanted optimisations, the much more cryptic error java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil is given

Repo with code that reproduces and full bash output is here: https://gist.github.com/glenjamin/2614f71a50dbbccf9040



 Comments   
Comment by António Nuno Monteiro [ 07/Nov/16 10:09 AM ]

Can't repro in 1.9.293. I get the expected FileNotFoundException.





[CLJS-1646] Internal compiler error when importing Showdown via CommonJS Created: 24/May/16  Updated: 07/Nov/16

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

Type: Defect Priority: Minor
Reporter: Juan Pedro Bolivar Puente Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

cljs 1.8.51, clojure 1.8, lein-cljsbuild 1.1.3



 Description   

Hi!

I am trying to use Showdown 1.4.1 from NPM [1] using a `:commonjs` foreign-lib. Sadly, compilation crashes with the following error:

java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
Node(SCRIPT): node_modules/showdown/dist/showdown.js:1:0
[source unknown]
Parent: NULL
at com.google.javascript.jscomp.ES6ModuleLoader.normalizeInputAddress(ES6ModuleLoader.java:118)
at com.google.javascript.jscomp.ProcessCommonJSModules.inputToModuleName(ProcessCommonJSModules.java:89)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visitScript(ProcessCommonJSModules.java:336)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visit(ProcessCommonJSModules.java:245)
at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:623)
at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:297)
at com.google.javascript.jscomp.NodeTraversal.traverseEs6(NodeTraversal.java:564)
at com.google.javascript.jscomp.ProcessCommonJSModules.process(ProcessCommonJSModules.java:85)
at cljs.closure$eval5486$fn__5487.invoke(closure.clj:1541)
at clojure.lang.MultiFn.invoke(MultiFn.java:233)
at cljs.closure$write_javascript.invokeStatic(closure.clj:1601)
at cljs.closure$write_javascript.invoke(closure.clj:1578)
at cljs.closure$source_on_disk.invokeStatic(closure.clj:1624)
at cljs.closure$source_on_disk.invoke(closure.clj:1619)
at cljs.closure$output_unoptimized$fn__5536.invoke(closure.clj:1662)
at clojure.core$map$fn__4785.invoke(core.clj:2646)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$filter$fn__4812.invoke(core.clj:2700)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$map$fn__4785.invoke(core.clj:2637)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:688)
at clojure.core$next__4341.invokeStatic(core.clj:64)
at clojure.core$str$fn__4419.invoke(core.clj:546)
at clojure.core$str.invokeStatic(core.clj:544)
at clojure.core$str.doInvoke(core.clj:533)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:646)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$deps_file.invokeStatic(closure.clj:1340)
at cljs.closure$deps_file.invoke(closure.clj:1337)
at cljs.closure$output_deps_file.invokeStatic(closure.clj:1360)
at cljs.closure$output_deps_file.invoke(closure.clj:1359)
at cljs.closure$output_unoptimized.invokeStatic(closure.clj:1670)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1653)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:648)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$build.invokeStatic(closure.clj:1981)
at cljs.closure$build.invoke(closure.clj:1882)
at cljs.build.api$build.invokeStatic(api.clj:210)
at cljs.build.api$build.invoke(api.clj:198)
at cljs.build.api$build.invokeStatic(api.clj:201)
at cljs.build.api$build.invoke(api.clj:198)
at cljsbuild.compiler$compile_cljs$fn__5771.invoke(compiler.clj:60)
at cljsbuild.compiler$compile_cljs.invokeStatic(compiler.clj:59)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:48)
at cljsbuild.compiler$run_compiler.invokeStatic(compiler.clj:168)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:122)
at user$eval5908$iter_59445948$fn5949$fn_5967.invoke(form-init8819033363931377476.clj:1)
at user$eval5908$iter_59445948$fn_5949.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$dorun.invokeStatic(core.clj:3024)
at clojure.core$doall.invokeStatic(core.clj:3039)
at clojure.core$doall.invoke(core.clj:3039)
at user$eval5908.invokeStatic(form-init8819033363931377476.clj:1)
at user$eval5908.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6927)
at clojure.lang.Compiler.eval(Compiler.java:6917)
at clojure.lang.Compiler.load(Compiler.java:7379)
at clojure.lang.Compiler.loadFile(Compiler.java:7317)
at clojure.main$load_script.invokeStatic(main.clj:275)
at clojure.main$init_opt.invokeStatic(main.clj:277)
at clojure.main$init_opt.invoke(main.clj:277)
at clojure.main$initialize.invokeStatic(main.clj:308)
at clojure.main$null_opt.invokeStatic(main.clj:342)
at clojure.main$null_opt.invoke(main.clj:339)
at clojure.main$main.invokeStatic(main.clj:421)
at clojure.main$main.doInvoke(main.clj:384)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:383)
at clojure.lang.AFn.applyToHelper(AFn.java:156)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: java.lang.NullPointerException
... 85 more

[1] https://www.npmjs.com/package/showdown



 Comments   
Comment by Juan Pedro Bolivar Puente [ 24/May/16 12:39 PM ]

Forgot to mention, this is the line in my :foreign-libs

{:file "node_modules/showdown/dist/showdown.js"
:provides ["showdown"]
:module-type :commonjs}

Same problem occurs if I use showdown.min.js

Comment by António Nuno Monteiro [ 07/Nov/16 9:37 AM ]

Can't repro in ClojureScript 1.9.293. Showdown seems to be correctly consumed by the Closure Compiler.

Comment by António Nuno Monteiro [ 07/Nov/16 9:40 AM ]

FWIW, it also works for me in 1.8.51, could this related to tooling / dependency conflicts?





[CLJS-1715] Self-host: Symbols in cljs.spec.test/instrument Created: 27/Jul/16  Updated: 07/Nov/16  Resolved: 07/Nov/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Not Reproducible Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1715-2.patch     Text File CLJS-1715.patch    

 Description   

For bootstrapped ClojureScript, within cljs.spec.test/instrument:

1. There is a use of speced-vars that conditionally needs the $macros suffix (analogous to CLJS-1656).
2. The references to the runtime fns instrumentable-syms and distinct-by need to be qualified (otherwise their expansions use $macros.



 Comments   
Comment by Mike Fikes [ 10/Aug/16 4:12 PM ]

Previous patch no longer applies. Attaching a rebased patch. This patch is still relevant to master.

Comment by David Nolen [ 16/Sep/16 2:12 PM ]

Needs a rebase.

Comment by David Nolen [ 16/Sep/16 2:23 PM ]

Does CLJS-1773 address this?

Comment by Mike Fikes [ 07/Nov/16 9:36 AM ]

Item 1 in the description was covered separately via CLJS-1735. (And, interestingly, I don't think CLJS-1773 covers this case; we still need an explicit use of $macros there.)

Item 2 should be covered by CLJS-1773.

So, in short, I think we can close this ticket. I'll do so.

Comment by Mike Fikes [ 07/Nov/16 9:36 AM ]

Covered by other tickets at this point. Issue no longer reproducible.





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

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

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


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

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






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

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

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

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

 Description   

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

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

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

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






[CLJS-1838] cljs.compiler.api/compile-root doesn't use .cljc/.cljs precedence rules correctly. Created: 03/Nov/16  Updated: 05/Nov/16

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

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


 Description   

When you have both a .cljc and a .cljs for the same namespace, compile-root seems to arbitrarily choose which gets compiled. Case-in-point: cljs.test.



 Comments   
Comment by António Nuno Monteiro [ 05/Nov/16 9:29 AM ]

What ClojureScript version was this tested against? It would seem to me that CLJS-1772 fixed this issue (which was shipped in 1.9.229).





[CLJS-1839] Relax the constraint that `new` and dot forms must be passed a symbol Created: 04/Nov/16  Updated: 04/Nov/16  Resolved: 04/Nov/16

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 04/Nov/16 11:42 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 04/Nov/16 11:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/8e03b787f052612d3be3f230ff0b4b50a3333dfd





[CLJS-693] new and dot form work on locals Created: 20/Nov/13  Updated: 04/Nov/16  Resolved: 04/Nov/16

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

Type: Defect Priority: Minor
Reporter: Joshua Headapohl Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Given some JavaScript library included in the same page as CLJS output:

var Lib = {};

Lib.Thing = function (val) {
    this.val = val;
};

Lib.Thing.prototype.log = function () {
    console.log(this.val);
};

I can bind the Thing property to a local and construct it using new or the dot form. This code compiles and runs without errors, and logs three lines to the console.

(ns cljs-construct-locals-bug.core)

; Legit
(let [thing (new js/Lib.Thing "hello")]
  (.log thing))

; Questionable
(let [Thing (.-Thing js/Lib)
      thing1 (new Thing "maybe")
      thing2 (Thing. "no way")]
  (.log thing1)
  (.log thing2))

I talked to David Nolen and he said this behavior is not correct.



 Comments   
Comment by David Nolen [ 02/Dec/13 8:58 PM ]

Upon further consideration this probably requires some feedback from the community. I'd forgotten that (Thing. foo bar ...) is just sugar for (new Thing foo bar ...) ...

Comment by António Nuno Monteiro [ 04/Nov/16 11:13 AM ]

Issue is not valid anymore.





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

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

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


 Description   

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

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

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


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

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


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

From @ykomatsu on Github:

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

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

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

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





[CLJS-1835] REPL load special fn Created: 24/Oct/16  Updated: 30/Oct/16  Resolved: 30/Oct/16

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

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

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

 Description   

We have `load-file` but we also want load which works via the classpath.



 Comments   
Comment by António Nuno Monteiro [ 27/Oct/16 4:29 PM ]

Patch attached.

Comment by David Nolen [ 30/Oct/16 6:18 AM ]

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





[CLJS-1834] REPL regression, require of ns from the ns itself errors out in circular reference Created: 24/Oct/16  Updated: 30/Oct/16  Resolved: 30/Oct/16

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

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

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

 Comments   
Comment by David Nolen [ 24/Oct/16 2:11 AM ]

It seems we probably still need special handling of `require` for REPLs.

Comment by António Nuno Monteiro [ 27/Oct/16 4:39 PM ]

Attached patch with fix.

Comment by David Nolen [ 30/Oct/16 6:16 AM ]

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





[CLJS-1837] Port halt-when over from Clojure Created: 28/Oct/16  Updated: 30/Oct/16  Resolved: 30/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

https://github.com/clojure/clojure/commit/7aad2f7dbb3a66019e5cef3726d52d721e9c60df



 Comments   
Comment by António Nuno Monteiro [ 29/Oct/16 4:55 AM ]

Attached patch with fix and tests.

Comment by David Nolen [ 30/Oct/16 5:15 AM ]

fixed https://github.com/clojure/clojurescript/commit/316ac08f9e117648afee29c282051bd5416e48b2





[CLJS-1820] "No such namespace" warning when referring to JS module namespace without using alias Created: 13/Oct/16  Updated: 30/Oct/16  Resolved: 30/Oct/16

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

Type: Defect Priority: Minor
Reporter: Maria Geller Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1820.patch    

 Description   

Currently, when requiring a JS module it is not possible to refer to it using the original namespace. For example, the following code will result in a warning:

(ns my-calculator.core
  (:require [my.calc]))

(println (my.calc/add 5 5))
WARNING: No such namespace: my.calc, could not locate my/calc.cljs, my/calc.cljc, or Closure namespace "" at line 4 src/my_calculator/core.cljs
WARNING: Use of undeclared Var my.calc/add at line 4 src/my_calculator/core.cljs

And the resulting JavaScript will throw the following error:

Uncaught ReferenceError: my is not defined

However, defining an alias for the JS module and using it works fine, for example:

(ns my-calculator.core
  (:require [my.calc :as calc]))

(println (calc/add 5 5))

Here is a minimal repo to reproduce this problem: https://github.com/mneise/my-calculator.



 Comments   
Comment by Maria Geller [ 27/Oct/16 8:23 PM ]

Added a patch for this.

Since JS modules are loaded using the new name created by the Google Closure compiler during module conversion, we always need to add an alias from the name given in the :provides compiler option to the new name.

Comment by David Nolen [ 30/Oct/16 5:12 AM ]

fixed https://github.com/clojure/clojurescript/commit/1f2c0a922130769a3da398aa501b3621fd1f8cd5





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

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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





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

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

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

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

 Description   

Examples:

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

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






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

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

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





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

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

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

Oracle Java 1.8.0_91
Apache Maven 3.3.9



 Description   

Run the following in a REPL:

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

Expected: nil or exception

Actual: :a






[CLJS-1832] destructuring with #js at :or breaks the compilation when transit is part of the project Created: 23/Oct/16  Updated: 23/Oct/16

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

Type: Defect Priority: Minor
Reporter: Wilker Lúcio da Silva Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Since Om `1.9.293` I was having some compilation issues, I was able to narrow it down to this code:

(defn f [{:keys [a] :or {a #js {}}}])

The code above fails to compile when [com.cognitect/transit-clj "0.8.290"] is part of the project dependencies. The problem seems to happen when we try to destructure data at function arguments, using `:or` and having `#js` at part of the `:or`.

I put up a repository with a minimal case here: https://github.com/wilkerlucio/cljs-compilation-fail

Error stack when compiling:

Wilkers-MacBook-Pro:cljs-compile-bug wilkerlucio$ lein clean && lein cljsbuild once site
Compiling ClojureScript...
Compiling "resources/public/site/site.js" from ["src"]...
Compiling "resources/public/site/site.js" failed.
clojure.lang.ExceptionInfo: failed compiling file:src/cljs_compile_bug/core.cljs {:file #object[java.io.File 0x21399e53 "src/cljs_compile_bug/core.cljs"]}
        at clojure.core$ex_info.invokeStatic(core.clj:4725)
        at clojure.core$ex_info.invoke(core.clj:4725)
        at cljs.compiler$compile_file$fn__4104.invoke(compiler.cljc:1410)
        at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1376)
        at cljs.compiler$compile_file.invoke(compiler.cljc:1356)
        at cljs.closure$compile_file.invokeStatic(closure.clj:432)
        at cljs.closure$compile_file.invoke(closure.clj:423)
        at cljs.closure$eval6005$fn__6006.invoke(closure.clj:499)
        at cljs.closure$eval5941$fn__5942$G__5930__5949.invoke(closure.clj:389)
        at cljs.closure$compile_task$fn__6096.invoke(closure.clj:779)
        at cljs.closure$compile_task.invokeStatic(closure.clj:777)
        at cljs.closure$compile_task.invoke(closure.clj:770)
        at cljs.closure$parallel_compile_sources$fn__6102.invoke(closure.clj:806)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:657)
        at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1963)
        at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1963)
        at clojure.lang.RestFn.invoke(RestFn.java:425)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.RestFn.applyTo(RestFn.java:132)
        at clojure.core$apply.invokeStatic(core.clj:661)
        at clojure.core$bound_fn_STAR_$fn__6761.doInvoke(core.clj:1993)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at clojure.lang.AFn.run(AFn.java:22)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.RuntimeException: clojure.lang.ArityException: Wrong number of args (1) passed to: analyzer/fn--1412/fn--1413
        at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:64)
        at cognitect.transit$write.invokeStatic(transit.clj:149)
        at cognitect.transit$write.invoke(transit.clj:146)
        at cljs.analyzer$write_analysis_cache.invokeStatic(analyzer.cljc:3320)
        at cljs.analyzer$write_analysis_cache.invoke(analyzer.cljc:3307)
        at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1307)
        at cljs.compiler$emit_source.invoke(compiler.cljc:1237)
        at cljs.compiler$compile_file_STAR_$fn__4081.invoke(compiler.cljc:1328)
        at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1159)
        at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1150)
        at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1317)
        at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1313)
        at cljs.compiler$compile_file$fn__4104.invoke(compiler.cljc:1398)
        ... 25 more
Caused by: clojure.lang.ArityException: Wrong number of args (1) passed to: analyzer/fn--1412/fn--1413
        at clojure.lang.AFn.throwArity(AFn.java:429)
        at clojure.lang.AFn.invoke(AFn.java:32)
        at cognitect.transit$write_handler$reify__1328.tag(transit.clj:79)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:147)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:82)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
        at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:87)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
        at com.cognitect.transit.impl.AbstractEmitter.emitTagged(AbstractEmitter.java:34)
        at com.cognitect.transit.impl.AbstractEmitter.emitEncoded(AbstractEmitter.java:59)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:169)
        at com.cognitect.transit.impl.AbstractEmitter.emitArray(AbstractEmitter.java:87)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:164)
        at com.cognitect.transit.impl.AbstractEmitter.emitTagged(AbstractEmitter.java:34)
        at com.cognitect.transit.impl.AbstractEmitter.emitEncoded(AbstractEmitter.java:59)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:169)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.JsonEmitter.emitMap(JsonEmitter.java:158)
        at com.cognitect.transit.impl.AbstractEmitter.emitMap(AbstractEmitter.java:70)
        at com.cognitect.transit.impl.AbstractEmitter.marshal(AbstractEmitter.java:166)
        at com.cognitect.transit.impl.AbstractEmitter.marshalTop(AbstractEmitter.java:193)
        at com.cognitect.transit.impl.JsonEmitter.emit(JsonEmitter.java:28)
        at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:61)
        ... 37 more
Subprocess failed





[CLJS-1819] name collision for protocol member and function Created: 13/Oct/16  Updated: 23/Oct/16  Resolved: 23/Oct/16

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

Type: Defect Priority: Major
Reporter: Dan Johansson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

[org.clojure/clojure "1.8.0"]
[org.clojure/clojurescript "1.9.229"]



 Description   
(defprotocol IDiff1
   (diff1 [a b]))

(defn diff1 [a b c]
   :diff12)

The code above gives the warning: diff1 at line x being replace at line y



 Comments   
Comment by Dan Johansson [ 13/Oct/16 10:34 AM ]

Looking closer I think this is how it is supposed to be!
Please close this!

Comment by Wilker Lúcio da Silva [ 23/Oct/16 6:21 AM ]

the behavior of the issue is expected





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

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

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

Attachments: Text File CLJS-1786.patch    

 Description   

See these Clojure commits:

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



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

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

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

Why should the default be different from Clojure?

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

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

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

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

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

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

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





[CLJS-1831] Self-host: Improperly munge ns names Created: 22/Oct/16  Updated: 22/Oct/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1831.patch    

 Description   

If you have a namespace containing a reserved JavaScript keyword, in self-hosted ClojureScript the namespace isn't properly munged for the goog.provide call. A result is that symbols defined in the namespace will be attached to a nonexistent JavaScript object.

To reproduce, add a test namespace, say static.core that does nothing but have an assert that (= 1 1).



 Comments   
Comment by Mike Fikes [ 22/Oct/16 1:04 PM ]

Without the prod code fixes in cljs.js, a new unit test fails, producing the desired warning but then tripping up on attempting to attach to an undefined JavaScript static$ object, with the root cause being a goog.provide('static.core-test') call instead of the needed goog.provide('static$.core-test') as is done in regular ClojureScript:

WARNING: Namespace static.core-test contains a reserved JavaScript keyword, the corresponding Google Closure namespace will be munged to static$.core_test at line 1 src/test/cljs/static/core_test.cljs
#error {:message "ERROR in file src/test/cljs/static/core_test.cljs", :data {:tag :cljs/analysis-error}, :cause #object[ReferenceError ReferenceError: static$ is not defined]}

The production fix is to simply call cljs.compiler/munge instead of cljs.core/munge.





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

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

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

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

 Description   

Examples of failing cases:

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



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

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

Mind if I work on this?

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

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

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

Sorry António, I just saw your comment.





[CLJS-1827] Improve reader performance on Firefox/Windows Created: 20/Oct/16  Updated: 21/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: Michael Sperber Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, reader
Environment:

Firefox on Windows


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

 Description   

cljs.reader/read-string speeds up by a factor of 2 on Firefox/Windows through this change without complicating the code.

(Other JS engines, including Firefox on Linux/Mac do not seem to be affected as significantly.)



 Comments   
Comment by David Nolen [ 20/Oct/16 10:33 AM ]

It would be nice to have a bit more information on this ticket as to what Google Closure does that's unnecessary or whether this path is actually a faithful port of Clojure behavior (copies the implementation of the EDN reader in these hot spots??). Finally the patch names David Frese, have they submitted a CA?

Thanks!

Comment by Michael Sperber [ 21/Oct/16 5:49 AM ]

I believe the Google functions are too general, work on strings in addition to characters etc.

It's not clear to us though why only Firefox on Windows benefits.

(David Frese is a co-worker - yes, has submitted a CA.)





[CLJS-844] Optimize js->clj by switching to transients Created: 22/Aug/14  Updated: 21/Oct/16

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

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

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

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

Did you any benchmarks on other JS engines?

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

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

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

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

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

Is there a existing one that I can work from?

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

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

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

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

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

The code is at:

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

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

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

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

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

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

Comment by Marcin Kulik [ 07/Oct/15 1:46 PM ]

I have tested and benchmarked the patch on a big js objects (5MB+ json files) and I confirm that new-js->clj3 function is more than 2x faster. It runs now in the player on https://asciinema.org and all seems good so far.

Comment by David Nolen [ 08/Oct/15 2:33 PM ]

Marcin thanks for the feedback. Experience reports always help push tickets forward.

Darrick, the patch need to be rebased to master. Please remove all patches except the one that will be applied.

Comment by Rohit Aggarwal [ 08/Oct/15 3:06 PM ]

[ClojureScript beginner here. so do ignore this if I am mistaken]

The patch of 25/Aug uses `(aget x k)` instead of `(goog.object/get x k)` for getting the value of a key if x is an object. I believe it isn't the right way even though it works.

This contains why the latter is preferred over the former.

Comment by David Nolen [ 09/Oct/15 12:33 PM ]

Rohit good catch. Yes that should be changed.

Comment by Thomas Spellman [ 21/Oct/16 3:01 AM ]

Added a patch, "js-to-clj-using-transients.patch", on Oct 16, 2016 that supersedes "use-transducers-in-js-to-clj.patch" from Aug, 2014. This patch changes cljs.core/js->clj to use transients. Also included is a change to both js->clj and clj->js to use gobject/get and gobject/set instead of aget and aset on JS object.

The JSperf shows a 17% speed improvement in Chrome on the first run, but about equal on the second run: https://jsperf.com/js-clj-transient-perf-test

The code for the JSperf is here: https://gist.github.com/thos37/41c0b38bea3270988a3275332686ab49

What would be the ideal test data for this?





[CLJS-1828] Add `:rename` to `require`'s docstring Created: 20/Oct/16  Updated: 20/Oct/16  Resolved: 20/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Comments   
Comment by António Nuno Monteiro [ 20/Oct/16 8:01 AM ]

Patch attached.

Comment by David Nolen [ 20/Oct/16 1:33 PM ]

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





[CLJS-1808] cljs.spec.test documentation recommends using an incorrect key for (check) options maps Created: 03/Oct/16  Updated: 19/Oct/16  Resolved: 19/Oct/16

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

Type: Defect Priority: Trivial
Reporter: JR Heard Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File patch-0001.patch    

 Description   

In https://github.com/clojure/clojurescript/blob/7e90ad51452ec5edd8ee7f2b7af9c7e7fb759c97/src/main/cljs/cljs/spec/test.cljc#L210 , the documentation of (check) instructs users to include options maps that look like

{:clojure.spec.test.check/opts {:num-tests 100}}
In reality, though, per https://github.com/clojure/clojurescript/blob/7e90ad51452ec5edd8ee7f2b7af9c7e7fb759c97/src/main/cljs/cljs/spec/test.cljs#L19 , those options maps should look like
{:clojure.test.check/opts {:num-tests 100}}

The documentation should be updated to reflect this.



 Comments   
Comment by JR Heard [ 03/Oct/16 11:31 AM ]

(I'm a first-time contributor, so please let me know if I make any mistakes throughout this process)

Comment by JR Heard [ 03/Oct/16 12:38 PM ]

<patch 2 deleted>

Comment by David Nolen [ 18/Oct/16 10:55 AM ]

Unrelated changes are not desirable. In the future just leave other code alone. Thanks

Comment by David Nolen [ 19/Oct/16 11:08 AM ]

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





[CLJS-1762] Bump Closure Compiler Created: 22/Aug/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Task Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: closure

Attachments: Text File CLJS-1762-3.patch     Text File CLJS-1762-4.patch     Text File CLJS-1762.patch    

 Description   

A new version of the Closure Compiler features an optimization that results in faster compilation, as well as some some changes to ES6-related features.



 Comments   
Comment by Juho Teperi [ 27/Aug/16 4:39 AM ]

Here is my version of the change.

Updating cljs.closure was quite straightforward, just removed all the checks and changed a few constructor calls.

I think I found all places to update the Closure version: pom.template.xml, project.clj and script/bootstrap. Bootstrap script also needed a change to account for changes closure-compiler filename.

  • ES6ModuleLoader has been replaced with ModuleLoader, it no longer
    needs to be passed to ProcessCommonJSModules etc. constructors
  • This version requires the version 20160822 and any checks needed to
    work with older versions have been removed
  • Closure-compiler jar name has been changed so lib/ and closure/
    directories should be removed before running bootstrap
Comment by Juho Teperi [ 27/Aug/16 4:55 AM ]

While tests pass with the patch, looks like module processing is broken. Tested with https://github.com/mneise/circle-color

Comment by Juho Teperi [ 15/Sep/16 5:19 AM ]

Current WIP branch at https://github.com/clojure/clojurescript/compare/master...Deraen:cljs-1762

I started writing some tests for module processing based on Mneise's circle-color example. I heavily simplified React UMD module and JSX processing for the tests, but not yet very happy with this method. Probably best to write the JS for tests from scratch.

Currently stuck on getting Closure compiler to return the processed code: https://github.com/clojure/clojurescript/compare/master...Deraen:cljs-1762#diff-84ffd22349e5ca1fe6322cb0e379b3b1R1530

Comment by Juho Teperi [ 27/Sep/16 5:15 PM ]

Proposed patch. Contains several FIXME notes, some I know how to fix, on others I would like to have some comments what would be best way to solve them.

Comment by David Nolen [ 28/Sep/16 8:20 AM ]

This is fantastic. Will review!

Comment by Juho Teperi [ 12/Oct/16 2:34 PM ]

Went through my fixme notes and added measure call around module processing.

Comment by David Nolen [ 18/Oct/16 4:40 PM ]

fixed https://github.com/clojure/clojurescript/commit/732034bc8cb7e356b211c5d2b88a7af94ba184e5





[CLJS-1658] implements? may report false positives Created: 01/Jun/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

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

Attachments: Text File implements-false-positives.patch     Text File implements-false-positives-with-sentinel.patch    
Patch: Code

 Description   

The cljs.core/implements? checks whether a protocol is implemented via a property on the tested object. When implementing the protocol this property will be set to true.

// the implementation
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = true;

// the check (implements? TheProtocol x)
((!((x == null)))?(((false) || (x.protocol$check$TheProtocol$))?true:false):false)

// only the relevant bit
x.protocol$check$TheProtocol$

This works fine under :none but :advanced may rename this to something like x.A. If you now try to check (implements? TheProtocol js/window) for example it may (or may not) report a false positive. For larger projects the likelyhood of creating a false positives goes way up since window contains all the variables created by the advanced compiled js.

The attached patch changes the patch the emitted code to check x.protocol$check$TheProtocol$ === true which reduces the chance of a false positive by a lot. We might chose another sentinel value instead to reduce the chance some more, this seems good enough though.



 Comments   
Comment by Thomas Heller [ 01/Jun/16 2:13 PM ]

Implemented the same change for cljs.core/satisfies?.

Comment by Thomas Heller [ 01/Jun/16 2:18 PM ]

Benchmark on my MBP indicate that the change comes with a slight performance cost.

// v8 with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 10 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 27 msecs

//v8 without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 8 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 19 msecs
Comment by Thomas Heller [ 01/Jun/16 2:28 PM ]
// spidermonkey with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 68 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 146 msecs

// spidermonkey without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 63 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 149 msecs
Comment by Thomas Heller [ 01/Jun/16 2:35 PM ]

JavascriptCore

// jsc with path
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 7 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 14 msecs

// jsc without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 6 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 13 msecs
Comment by Thomas Heller [ 02/Jun/16 3:40 AM ]

Added a second patch that uses a sentinel value as the protocol marker. Currently just a plain empty javascript object.

Running the benchmarks shows no real difference to just checking "true".

I also tried using a number or string as the sentinel value but could not find any significant difference either.

// emitted code on protocol impls
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = cljs.core.PROTOCOL_SENTINEL;

// the check (identical?)
cljs.core.PROTOCOL_SENTINEL === x.protocol$check$TheProtocol$
Comment by David Nolen [ 18/Oct/16 4:15 PM ]

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





[CLJS-1826] Self-host: load-deps doesn't honor `:reload` and `reload-all` Created: 18/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Comments   
Comment by António Nuno Monteiro [ 18/Oct/16 1:38 PM ]

Patch attached.

Comment by David Nolen [ 18/Oct/16 2:01 PM ]

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





[CLJS-1825] :source-map error when passing `false` under simple optimizations Created: 18/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

although CLJS-1805 landed, it still isn't enough to allow passing `source-map false`



 Comments   
Comment by António Nuno Monteiro [ 18/Oct/16 1:14 PM ]

Patch attached.

Comment by David Nolen [ 18/Oct/16 1:25 PM ]

fixed https://github.com/clojure/clojurescript/commit/9de9face6c06c3998eec3aec4730a071e30a8c47





[CLJS-1821] `add-preloads` should only touch sources if `:preloads` option specified Created: 14/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Defect Priority: Major
Reporter: Robert Stuttaford Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1821-2.patch     Text File CLJS-1821.patch    
Patch: Code

 Description   

Should also add a warning if `:preloads` is specified with an optimization setting other than `:none`



 Comments   
Comment by António Nuno Monteiro [ 18/Oct/16 12:42 PM ]

Attached CLJS-1821-2.patch with the fix.

Comment by David Nolen [ 18/Oct/16 1:24 PM ]

fixed https://github.com/clojure/clojurescript/commit/62e4bc982c44c123d6af9981470f7d9f7bfc2946





[CLJS-1824] transit cache feature leaks files Created: 18/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

We read transit files outside of `with-open` blocks.



 Comments   
Comment by David Nolen [ 18/Oct/16 12:17 PM ]

fixed https://github.com/clojure/clojurescript/commit/176c681b25b75e907bf376698263dacf6ce998f4





[CLJS-1814] Move docstrings for require, etc. from `cljs.repl` to their new definitions in `cljs.core` Created: 08/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

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

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

Attached patch with fix.

Comment by David Nolen [ 18/Oct/16 11:29 AM ]

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





[CLJS-1809] Add 0/1 arity to `into` Created: 05/Oct/16  Updated: 18/Oct/16  Resolved: 18/Oct/16

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

As per Clojure commit https://github.com/clojure/clojure/commit/e547d35bb796051bb2cbf07bbca6ee67c5bc022f



 Comments   
Comment by António Nuno Monteiro [ 05/Oct/16 9:12 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 18/Oct/16 11:25 AM ]

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





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

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

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

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

 Description   

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

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



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

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

Both however don't throw an error.

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

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





[CLJS-1519] Collection invoke errors report arity off by 1 Created: 22/Dec/15  Updated: 14/Oct/16

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: André Luís Anastácio
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

Runtime collection invokes will report arity that is off by one. This is because we use the generic function arity dispatching logic which doesn't account for the 1st self argument.



 Comments   
Comment by Mike Jackson [ 15/Apr/16 1:02 AM ]

Hey David,

Can I pick this one up? I'm a first time contributor and I wouldn't mind using this to get a lay of the land. I've already signed the Contributor Agreement.

Cheers

Comment by David Nolen [ 19/Apr/16 2:12 PM ]

Mike, I've updated your permissions. Please assign the ticket to yourself. Thanks!

Comment by Mike Jackson [ 19/Apr/16 2:42 PM ]

Awesome, thanks. Looking forward to it.

Comment by André Luís Anastácio [ 05/Oct/16 10:32 PM ]

This issue are assigned to Mike a long time ago, maybe I can solve this.

David, sorry for the newbie question but I don't understand the issue. Do you have some example?

Comment by António Nuno Monteiro [ 06/Oct/16 7:34 AM ]

Here go a couple of examples:

cljs.user=> ({:a 1})
Error: Invalid arity: 1

cljs.user=> ({:a 1} :a :not-found :other)
Error: Invalid arity: 4

Notice how the arity is off by one? In the first example, we passed 0 args and in the 2nd example we passed 3 args.





[CLJS-1294] Macroexpand only accept quoted lists Created: 01/Jun/15  Updated: 12/Oct/16  Resolved: 12/Oct/16

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

Type: Enhancement Priority: Minor
Reporter: Julien Eluard Assignee: André Luís Anastácio
Resolution: Completed Votes: 0
Labels: newbie

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

 Description   

In Clojure macroexpand and macroexpand-1 accept any quoted argument while in ClojureScript anything but quoted seq will throw an exception.



 Comments   
Comment by Julien Eluard [ 01/Jun/15 2:16 PM ]

In Clojure some special forms are handled specifically i.e. (macroexpand '(Boolean true)) => (new Boolean true).

I am not sure if/how it applies to ClojureScript.

Comment by David Nolen [ 14/Jul/15 5:58 AM ]

This patch needs to be rebased to master. Thanks!

Comment by André Luís Anastácio [ 08/Oct/16 9:49 PM ]

Rebased patch created by Julien Eluard

Comment by David Nolen [ 12/Oct/16 1:25 PM ]

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





[CLJS-1818] (hash false) returns different value from Clojure Created: 12/Oct/16  Updated: 12/Oct/16  Resolved: 12/Oct/16

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

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

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

 Description   

Discovered while investigating CLJS-1817



 Comments   
Comment by António Nuno Monteiro [ 12/Oct/16 8:15 AM ]

Patch with fix and test.

Comment by David Nolen [ 12/Oct/16 1:14 PM ]

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





[CLJS-1817] Strange result when assoc'ing 0 to persistent hash map Created: 12/Oct/16  Updated: 12/Oct/16  Resolved: 12/Oct/16

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

Type: Defect Priority: Major
Reporter: Dan Johansson Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   
(assoc (hash-map false 0 0 -1) 0 0) {false 0, 0 -1}


 Comments   
Comment by Dan Johansson [ 12/Oct/16 5:22 AM ]

Oops accidentally pushed enter and the issue got submitted before I was finished editing it.
How do I edit?

But the headline is pretty specific about the issue.

Comment by Dan Johansson [ 12/Oct/16 5:26 AM ]

Tested on:
[org.clojure/clojure "1.8.0"]
[org.clojure/clojurescript "1.9.229"]

Comment by António Nuno Monteiro [ 12/Oct/16 7:39 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 12/Oct/16 1:05 PM ]

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





[CLJS-1536] REPL def symbol init collision Created: 03/Jan/16  Updated: 10/Oct/16  Resolved: 10/Oct/16

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

In a REPL, if you try def where the init is a local matching the symbol being defined, then analysis fails.

(let [x 1]
  (def x x))

This can be verified in script/noderepljs and you can see it is some bad interaction with REPL var emission because if :def-emits-var false is added to the script, things work.



 Comments   
Comment by Txus Bach [ 04/Jan/16 3:01 PM ]

Confirmed that evaluating this breaks:

(require 'clojure.tools.reader
         'cljs.analyzer
         'cljs.compiler)

(let [env '{:ns {:name cljs.user}
            :def-emits-var true
            :locals {}}]
  (->> "(let [x 1] (def x 3))"
       clojure.tools.reader/read-string
       (cljs.analyzer/analyze env)
       cljs.compiler/emit-str))

(When emitting, not when just analyzing.)

Removing `:def-emits-var true` makes the bug disappear.

Looking into how to fix it – any clues are welcome, it's my first time around the codebase

Comment by Txus Bach [ 04/Jan/16 3:45 PM ]

Seems that var-ast is returning nil, because resolve-var doesn't return a map with an :ns key. Not sure if this code should work, but it returns nil:

(let [env {:ns {:name 'cljs.user},
 :def-emits-var true,
 :locals
 {'x
  {:init
   {:op :constant,
    :env
    {:ns {:name 'cljs.user},
     :def-emits-var true,
     :locals {},
     :line nil,
     :column nil,
     :context :expr},
    :form 1,
    :tag 'number},
   :name 'x,
   :binding-form? true,
   :op :var,
   :env {:line nil, :column nil},
   :column nil,
   :line nil,
   :info {:name 'x, :shadow nil},
   :tag 'number,
   :shadow nil,
   :local true}}}]
  (ana/resolve-var env 'x))

Will continue tomorrow. It's so much fun!

Comment by António Nuno Monteiro [ 16/Aug/16 4:29 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 16/Aug/16 6:27 PM ]

I tried António's patch with various expressions in the REPL and couldn't find a way to break it.

Comment by David Nolen [ 19/Sep/16 9:24 AM ]

Would like to address the fact that `((let [x 1] (defn x [] x))` does not appear to work in the REPL. I'm looking into it.

Comment by António Nuno Monteiro [ 19/Sep/16 9:28 AM ]

Yeah, that's weird. This is what I tested with and works:

cljs.user=> (let [x 1] (defn x [] x))
#'cljs.user/x
cljs.user=> (x)
1
Comment by António Nuno Monteiro [ 08/Oct/16 4:17 PM ]

Attached a new patch that addresses the recently raised issues. Also includes tests for them.

Comment by David Nolen [ 10/Oct/16 12:15 PM ]

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





[CLJS-1815] Fix failing analyzer tests Created: 08/Oct/16  Updated: 10/Oct/16  Resolved: 10/Oct/16

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: test

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

 Description   

Following CLJS-1807, analyzer tests that check for exception messages on invalid `require` and `ns` calls are failing due to those errors messages having been revised.



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

Patch with fix.

Comment by David Nolen [ 10/Oct/16 12:14 PM ]

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





[CLJS-1812] cljs.spec.test.check has the side effect of instrumenting vars it's called on Created: 05/Oct/16  Updated: 06/Oct/16

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

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

Attachments: Text File patch-0001.patch    

 Description   

When using (stest/check) to test a function, I noticed that calls to the function later on in my code behaved as if the function were instrumented, even though I never explicitly called (stest/instrument) on it.

I think that there's something wrong with this line: https://github.com/clojure/clojurescript/blob/7e90ad5/src/main/cljs/cljs/spec/test.cljc#L157 .

As far as I can tell from poking around at this code, re-inst# is always true, because in cljs.spec.test, calling (unstrument `foo/bar) will return [foo/bar] even if foo/bar is not currently instrumented. From what I can tell from poking around in the REPL, this is a departure from Clojure's behavior, which is to return [] in this situation.

As always, it's possible I'm insane or misinterpreting how this system is intended to behave; but for what it's worth, the print statements I've added to my local copy of Clojurescript indicate that if you call (stest/check `foo/bar) when @instrumented-vars is {}, re-inst# will be true, and `foo/bar will therefore be instrumented at the end of check's execution. This seems like it's probably a bug.



 Comments   
Comment by JR Heard [ 06/Oct/16 10:16 AM ]

I think I'd actually like to take a stab at this one, if that's all right.

Comment by JR Heard [ 06/Oct/16 3:03 PM ]

Added a patch. Let me know if I goofed in its formatting, commit message style, etc. Includes two tests that fail before this change and pass after it.

I had trouble writing the second test assertion in the way I wanted - (is (not (thrown? js/Error (h-cljs-1812 "foo")))) didn't seem to do the trick - so I ended up just calling the helper function with invalid args directly; please let me know if there's a better way





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

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

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


 Description   

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






[CLJS-1811] Can't compose cljs.spec.test.instrument (or cljs.spec.test.check) with cljs.spec.test.enumerate-namespace Created: 05/Oct/16  Updated: 05/Oct/16

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

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

Clojurescript 1.9.229



 Description   

When I call

(stest/instrument
  (stest/enumerate-namespace 'my.ns))

I get a stack trace that includes a message like `Caused by: java.lang.RuntimeException: No such namespace: stest`.

The same thing happens when I call

(stest/check
  (stest/enumerate-namespace 'my.ns))


 Comments   
Comment by JR Heard [ 05/Oct/16 5:29 PM ]

(I'm still a newbie here, please let me know if there's any more information I can provide!)





[CLJS-1810] Refactoring of find-and-cache-best-method Created: 05/Oct/16  Updated: 05/Oct/16

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

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

Attachments: Text File 0001-refactor-find-and-cache-best-method.patch    
Patch: Code

 Description   

find-and-cache-best-method was pretty messy and confusing. cache reset is done in -get-method fn itself and it was basically a dead code. find-best-method is the replacement of it and operates with immutable data instead of internal multimethod's mutable state.
prefers* function didn't mutate the atom too, so now it takes an immutable value.
dominates is now an internal helper of find-best-method since it is private and not used by anything else.






[CLJS-1791] :hierarchy option of defmulti is not used to determine if one dispatch value dominates on another Created: 22/Sep/16  Updated: 05/Oct/16  Resolved: 05/Oct/16

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

Type: Defect Priority: Major