<< Back to previous view

[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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-1807] Better error messages for `ns*` calls Created: 02/Oct/16  Updated: 04/Oct/16  Resolved: 04/Oct/16

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

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

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

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

Patch attached. Error message matches the used form.

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

fixed https://github.com/clojure/clojurescript/commit/9ec238fbe8e26f73dedb171475dd59b7b606fc9a





[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-1805] Source map should take false Created: 01/Oct/16  Updated: 01/Oct/16  Resolved: 01/Oct/16

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

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

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

 Description   

This is a follow-up to CLJS-1563 which allowed source-map to take nil, but didn't allow it to take false



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

Patch attached.

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

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





[CLJS-1804] Self-host: process namespace side-effects for new require without NS Created: 01/Oct/16  Updated: 01/Oct/16  Resolved: 01/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-1804.patch    
Patch: Code

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

Attached fix.

Comment by David Nolen [ 01/Oct/16 1:03 PM ]

fixed https://github.com/clojure/clojurescript/commit/2f2b7f253cd2bc5156bf74caeb1145823570470b





[CLJS-1803] Use new require capability in REPLs Created: 01/Oct/16  Updated: 01/Oct/16  Resolved: 01/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: repl

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

 Description   

With the landing of CLJS-1346, we can port REPLs to use this new functionality instead of the current hacks.



 Comments   
Comment by António Nuno Monteiro [ 01/Oct/16 7:36 AM ]

Attached patch with fix.

Couldn't find any way to break REPL requires. Appreciate it if anyone can give it a spin and report back.

Comment by David Nolen [ 01/Oct/16 1:00 PM ]

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





[CLJS-1802] Generated namespaces should be of the form `cljs.user.fileXXXX` Created: 01/Oct/16  Updated: 01/Oct/16  Resolved: 01/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-1802.patch    
Patch: Code and Test

 Description   

Following CLJS-1346, we should generate proper namespaces instead of `user$$gen_ns$$_file_XXXX.js`



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

Attached patch with fix and test.

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

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





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

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

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





[CLJS-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-1799] Fix cljs.predicates-test Created: 30/Sep/16  Updated: 30/Sep/16  Resolved: 30/Sep/16

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

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


 Description   

the tests are not supposed to work, because they assume we can detect integer.
we should fix them so that platform semantics are documented



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

fixed https://github.com/clojure/clojurescript/commit/647736a7dcb43521e25cc2cd2b96b2d497a3749f





[CLJS-1798] Add new tests to all test-runner namespaces Created: 30/Sep/16  Updated: 30/Sep/16  Resolved: 30/Sep/16

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 30/Sep/16 8:12 AM ]

Attached patch.

Comment by David Nolen [ 30/Sep/16 8:18 AM ]

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





[CLJS-1797] Update aot_core to support build with MINGW on Windows Created: 30/Sep/16  Updated: 30/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Thomas Kidd Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 10


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

 Description   

When using Git Bash (which uses MINGW), ./script/build can get nearly all the way through it's work. However, it dies in aot_core because of the way that the classpath is generated by the Maven dependency:build-classpath plugin.



 Comments   
Comment by Thomas Kidd [ 30/Sep/16 7:40 AM ]

I was able to get this working locally, and will submit a patch when I am sure things are working

Comment by Thomas Kidd [ 30/Sep/16 8:45 AM ]

Updates aot_core to set the classpath correctly. More comments in patch.

Comment by Thomas Kidd [ 30/Sep/16 8:49 AM ]

Tested that patched version of master works on Windows 10 and OSX

Comment by Thomas Kidd [ 30/Sep/16 8:54 AM ]

Needs review

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

Issues don't get closed until the issue is actually resolved





[CLJS-1796] Measure Google Closure specific optimization time Created: 28/Sep/16  Updated: 30/Sep/16  Resolved: 30/Sep/16

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

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

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

 Description   

Additionally to the overall optimization pass in the ClojureScript compiler (which also includes source map generation), we should also measure the time spent specifically in the Google Closure Compiler optimization pass.



 Comments   
Comment by António Nuno Monteiro [ 28/Sep/16 3:33 PM ]

Attached patch with fix.

Comment by David Nolen [ 30/Sep/16 12:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/3ab6de48b4541900d83e4b41e26080408d742de7





[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-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-1793] Few misplaced docstrings Created: 24/Sep/16  Updated: 24/Sep/16

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

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

Attachments: Text File CLJS-1793.patch    

 Description   

A few docsctrings were placed after the binding form in the src/main/clojure/cljs/closure.clj namespace.






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

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

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


 Description   

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

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

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

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



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

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





[CLJS-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
Reporter: Andrey Zaytsev Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug

Attachments: Text File 0001-follow-up-on-CLJS-460-defmulti-ignores-optional-hier.patch     Text File 0001-refactor-find-and-cache-best-method.patch    
Patch: Code and Test

 Description   

It leads to exception on method invocation.
Patch for similar issue CLJS-460 does not address conflicting dispatch values.



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

The patch needs to be rebased on master due to the test nses splitting refactor.

Comment by Andrey Zaytsev [ 01/Oct/16 6:03 AM ]

rebased on master

Comment by Andrey Zaytsev [ 01/Oct/16 6:07 AM ]

and one more patch with some refactor.
find-and-cache-best-method did too much work. cache gets reset in -get-method fn and there is nothing to do.
pass atoms dereferenced to fns that only read it.

Comment by David Nolen [ 05/Oct/16 3:52 PM ]

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

Comment by David Nolen [ 05/Oct/16 3:52 PM ]

Lets move the 2nd patch to another ticket. Thanks!





[CLJS-1790] Port CLJ-1935: Use multimethod dispatch value method lookup to take hierarchies into account in multi-spec Created: 21/Sep/16  Updated: 26/Sep/16  Resolved: 26/Sep/16

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

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

Attachments: Text File CLJS-1790.patch    

 Description   

See Clojure commit: https://github.com/clojure/clojure/commit/522ba8b82ba6eb6c50284a211e7533db51363b8f



 Comments   
Comment by Lauri Oherd [ 25/Sep/16 4:53 AM ]

Code and Test

Comment by David Nolen [ 26/Sep/16 2:20 PM ]

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





[CLJS-1789] Port CLJ-1988: Extend coll-of to handle sequences Created: 21/Sep/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

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

Attachments: Text File cljs-1789.patch    

 Description   

See Clojure commit: https://github.com/clojure/clojure/commit/edf869a0fa56df3aa2503980af65931d76e2e00b



 Comments   
Comment by Joshua Miller [ 21/Sep/16 2:56 PM ]

Adds fix and test.

Comment by David Nolen [ 23/Sep/16 1:01 PM ]

fixed https://github.com/clojure/clojurescript/commit/7ef1ed8debc647ddef847ac85262b24bfe96c48a





[CLJS-1788] Port CLJ-2004: include retag in multi-spec form Created: 21/Sep/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

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

Attachments: Text File CLJS-1788.patch    

 Description   

See Clojure commit:

https://github.com/clojure/clojure/commit/5e83c2ab898fefe655ee45495d56d69a6bd10304



 Comments   
Comment by Lauri Oherd [ 23/Sep/16 11:52 AM ]

Code and Test

Comment by Lauri Oherd [ 23/Sep/16 11:57 AM ]

Is it possible to edit the ticket's Patch field in order to add the "Code and Test" value to it (as described on https://github.com/clojure/clojurescript/wiki/Patches page)?
I couldn't find the Patch field.

Comment by David Nolen [ 23/Sep/16 12:56 PM ]

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





[CLJS-1787] Make cljs.spec explain pluggable Created: 21/Sep/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

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


 Description   

See Clojure commit:

https://github.com/clojure/clojure/commit/99ab306f82620e6db6a978a5565d2ccd668c0798



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

fixed https://github.com/clojure/clojurescript/commit/8399062f0179770580f53ac331485c5e944a773c





[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-1785] Warn on reference to js/foo shadowed by local binding Created: 21/Sep/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

Type: Enhancement Priority: Trivial
Reporter: Trevor Schmidt Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None
Environment:

OS X



 Description   

When a local binding shadows a JS global, and an attempt is made to access said global with js/foo, it would be nice to get a warning indicating that the global is inaccessible.

Example:

(defn simple-repro []
  (let [location (str js/location.pathname)]
    location))

Per discussion in Clojurians Slack, this is the same issue as http://dev.clojure.org/jira/browse/CLJS-833.

The previous issue was closed because the suggested solution was inadequate, but per discussion it seems appropriate to provide a warning so it is less surprising.



 Comments   
Comment by David Nolen [ 23/Sep/16 1:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/00e46feb03e7c4e7784f6b0759762d537ea1daaa





[CLJS-1784] Cleanup set creation functions Created: 20/Sep/16  Updated: 28/Sep/16

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

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

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

 Description   

Use .fromArray for consistency/speed when handling zeroed IndexedSeqs.

Use reduce as the default construction path to take advantage of reducible collections.



 Comments   
Comment by Rohit Aggarwal [ 26/Sep/16 4:13 PM ]

Thomas Mulvaney, could you provide some benchmarks for the speed assertion? It would be nice to run it on Chrome/Firefox/Safari.

Comment by Thomas Mulvaney [ 28/Sep/16 1:30 AM ]

Sure thing, I'll do some more benchmarks.





[CLJS-1783] Unify List creation code Created: 20/Sep/16  Updated: 20/Sep/16

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

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

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

 Description   

There is some duplication and redundant functions around List creation.

In this patch a fromArray method was added to List, consistent with other persistent data structures in the code base.






[CLJS-1782] Self-host: allow namespaces to require their own macros Created: 19/Sep/16  Updated: 30/Sep/16  Resolved: 30/Sep/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: 1
Labels: None

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

 Description   

Currently a namespace can only require its own macros in JVM ClojureScript (called macro-loop in this post).

Because there's no reader conditional distinction between JVM and Bootstrapped ClojureScript, requiring a namespace that requires its own macros in self-host will result in the following error:

Could not require foo.core in file /Users/anmonteiro/Desktop/foo/src/foo/core.cljc
Maximum call stack size exceeded.

Below is an example that can be used to reproduce the issue:

;; foo/core.cljc
(ns foo.core
  #?(:cljs (:require-macros [foo.core])))

(defmacro a-macro [& args]
  `(println ~@args))

This ticket proposes the following solution: when loading a macros namespace in self-hosted ClojureScript, patch its `require-macros` and `use-macros` entries to remove itself, if present.



 Comments   
Comment by António Nuno Monteiro [ 19/Sep/16 9:09 PM ]

Attached patch with proposed fix.

Comment by António Nuno Monteiro [ 27/Sep/16 8:05 AM ]

Replaced the patch with a new one rebased on current master.

Comment by David Nolen [ 30/Sep/16 12:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/04751f337620279b0228856e4d224ae3d41abe72





[CLJS-1781] Add cljs.hash-map-test to self-parity tests Created: 19/Sep/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

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

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

 Comments   
Comment by David Nolen [ 23/Sep/16 1:37 PM ]

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





[CLJS-1780] Records without extmap fail to iterate Created: 15/Sep/16  Updated: 16/Sep/16  Resolved: 16/Sep/16

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

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

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

 Description   

Calling hasNext on a Record with no extmap fails.



 Comments   
Comment by David Nolen [ 16/Sep/16 1:11 PM ]

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





[CLJS-1779] keyword 2-arity constructor accepts anything for both parameters which leads to different hashing Created: 14/Sep/16  Updated: 14/Sep/16  Resolved: 14/Sep/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


 Description   
(keyword 'app "foo")
(keyword "app" "foo")

Will produce different hash codes.



 Comments   
Comment by David Nolen [ 14/Sep/16 3:05 PM ]

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





[CLJS-1778] cljs.spec.test/check only accepts literal syms, unlike clojure.spec.test/check Created: 14/Sep/16  Updated: 14/Sep/16  Resolved: 14/Sep/16

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

Type: Defect Priority: Major
Reporter: Jannis Pohlmann Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

As of commit https://github.com/clojure/clojurescript/commit/d0be39660f3a65422c3de6a774ceec0b6a863ee2, ClojureScript's implementation of `cljs.spec.test/check` only accepts a literal sym or a literal collection of syms as its inputs, e.g.

(cljs.spec.test/check 'some.ns/foo)
(cljs.spec.test/check '[some.ns/foo some.ns/bar])

It does not allow input syms to be passed in as a named value or to be generated (e.g. by filtering all checkable syms). So the following will fail:

(def test-syms '[some.ns/foo some.ns/bar])
(cljs.spec.test/check test-syms)
;; Results in an "`Unable to resolve symbol: test-syms`" error.

(cljs.spec.test/check (filter in-some-ns? (cljs.spec.test/checkable-syms)))
;; Results in an "`Unable to resolve symbol: filter`" error - I think.

This is different from the behavior of `clojure.spec.test/check` in Clojure, where all of the above are fine. Is this a bug or a deliberate difference?



 Comments   
Comment by David Nolen [ 14/Sep/16 3:35 PM ]

This is just how it works. Most of the clojure.spec.test/check must necessarily be implemented as macros.





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

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

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


 Description   

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

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

This becomes

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

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

This seems like something Google Closure should address.

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



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

To test

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

Compiler options

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

Code

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

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

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

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

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

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

This is what webpack generates

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

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





[CLJS-1776] Add fixed arities for mapcat Created: 13/Sep/16  Updated: 13/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: Robert C Faber Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS_1776__Add_fixed_arities_for_mapcat.patch     Text File CLJS-1776.patch    
Patch: Code

 Description   

Following the pattern of map, this patch adds three fixed arities for mapcat.



 Comments   
Comment by Alex Miller [ 13/Sep/16 10:25 AM ]

Presumably this is to improve performance. Please include a benchmark showing the difference.





[CLJS-1775] `get` with `nil` returns as if `get` with `0` Created: 12/Sep/16  Updated: 16/Sep/16  Resolved: 16/Sep/16

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

Type: Defect Priority: Major
Reporter: James Tyler Solomon Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Calling `(get "test" nil)` returns `"t"` whereas previously it would return `nil`, as is the case in clojure.



 Comments   
Comment by António Nuno Monteiro [ 12/Sep/16 3:52 PM ]

Attached patch with fix and test.

Comment by David Nolen [ 14/Sep/16 3:36 PM ]

This patch needs a rebase it seems.

Comment by António Nuno Monteiro [ 14/Sep/16 3:42 PM ]

Replaced the patch with a rebased version.

Comment by Thomas Mulvaney [ 14/Sep/16 6:12 PM ]

Would it not be better to check if k is an integer rather than non-nil value when calling get on indexed collections?

Comment by António Nuno Monteiro [ 14/Sep/16 6:23 PM ]

It's not really how Clojure works, AFAICT:

boot.user=> (get "asd" 1.7)
\s
Comment by David Nolen [ 16/Sep/16 1:23 PM ]

fixed https://github.com/clojure/clojurescript/commit/9cb8da1d82078cfe21c7f732d94115867f455a0a





[CLJS-1774] Self-host: Report filenames in warns in test-self-parity Created: 07/Sep/16  Updated: 14/Sep/16  Resolved: 14/Sep/16

Status: Resolved
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: bootstrap

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

 Description   

When running script/test-self-parity, make use of the new feature introduced with CLJS-1515 to add the capability of reporting filenames when diagnostics are emitted.

For example, you currently see

WARNING: baz is a single segment namespace at line 1

when running script/test-self-parity while script/test gives more info for the same diagnostic:

WARNING: baz is a single segment namespace at line 1 src/test/cljs/baz.cljs


 Comments   
Comment by David Nolen [ 14/Sep/16 3:37 PM ]

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





[CLJS-1773] Self-host: Don't resolve unqualified symbols / keywords with $macros Created: 05/Sep/16  Updated: 16/Sep/16  Resolved: 16/Sep/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: bootstrap

Attachments: Text File CLJS-1773.patch    

 Description   

Background: Since, in self-hosted ClojureScript, macro namespaces are compiled as ClojureScript, a $macros suffix is appended to macros namespace names in order to differentiate them from same-named runtime namespaces. As a concrete example: The foo.core runtime namespace will be processed normally, but the same, when compiled as a macro namespace is internally renamed to be foo.core$macros.

A consequence is that the internal $macros suffix surfaces in a few places where it is undesired. Specifically this ticket focuses on the cases of unqualified symbols subject to syntax-quote resolution and namespaced keywords that use the :: construct (as in ::bar). When these are used in macro namespaces, and then compiled with bootstrapped ClojureScript, a symbol like `baz would turn in to foo.core$macros/baz, and a keyword like ::bar would turn in to :foo.core$macros/bar.

In the case of symbols, it might be the case that the user wanted the symbol to refer to a var in the runtime namespace, in which case foo.core/baz is desired. And, even if the desire was that it end up referring to a macro named baz, then foo.core/baz works just fine. To achieve this, when porting regular ClojureScript code to be compatible with bootstrapped ClojureScript, one common pattern is the need to qualify symbols inside of syntax-quote scope so that they don't end up with the $macros suffix.

The same, parallel arguments essentially go for keywords: You want ::bar to turn into :foo.core/bar in both JVM and bootstrapped ClojureScript, and today, to achieve that, you must qualify those kinds of keywords when they appear in macros namespaces.

This ticket asks that when unqualified symbols in syntax quote are resolved and when the analogous thing occurs for keywords, that the $macros suffix be avoided as experience has shown now that it is practically always not the thing that you want (and perhaps also not correct from the desired language semantics.)



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

fixed https://github.com/clojure/clojurescript/commit/7e90ad51452ec5edd8ee7f2b7af9c7e7fb759c97





[CLJS-1772] Dependency index can incorrectly add `.cljc` files instead of `.cljs` if both are present Created: 02/Sep/16  Updated: 03/Sep/16  Resolved: 03/Sep/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-1772.patch    

 Comments   
Comment by António Nuno Monteiro [ 02/Sep/16 10:09 PM ]

Attached patch with fix.

Comment by David Nolen [ 03/Sep/16 7:37 AM ]

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





[CLJS-1771] Add helper for converting ex-info to js/Error Created: 02/Sep/16  Updated: 02/Sep/16

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

Type: Enhancement Priority: Trivial
Reporter: Artem Yarulin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ex-info


 Description   

ex-info/ex-data/ex-message didn't get a lot of attention in CLJS community, still I find quiet often (js/Error. "Err"). I guess the main reason is that we cannot pass ex-info object back to JS world, clj->js doesn't work in this case. We can solve it by adding helper like ex->js which will convert ex-info backward to js/Error.

What do you think about such enhancement? I can add patch later if you would find it useful






[CLJS-1770] goog-defines broken for integers Created: 01/Sep/16  Updated: 16/Sep/16  Resolved: 16/Sep/16

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

Type: Defect Priority: Minor
Reporter: Tom Connors Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: closure

Attachments: Text File CLJS-1770.patch    

 Description   

Using goog-define with integer values results in cljs.closure/make-options attempting to call .setDefineToIntegerLiteral on the compiler-options object. That method does not exist; the correct method is .setDefineToNumberLiteral. Note however that calls to .setDefineToNumberLiteral will fail when the value is a long; it might make sense to always use .setDefineToDoubleLiteral when the value is a number. Integers passed to .setDefineToDoubleLiteral remain integers in the output javascript, so it seems harmless enough to do it that way.



 Comments   
Comment by Tom Connors [ 02/Sep/16 8:55 AM ]

Patch for using setDefineToDoubleLiteral for all numbers.

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

fixed https://github.com/clojure/clojurescript/commit/1d8964ea632fbb3c775cbacb1f11807fbdfe9e72





[CLJS-1769] Use reduce pathway for arrays in js->clj Created: 01/Sep/16  Updated: 01/Sep/16

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

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

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

 Description   

Currently js->cljs uses `(vec (map some-fn some-js-array))` on arrays. By using `(mapv some-fn some-js-array)` we take advantage of array-reduce and avoid a lazy-sequence.






[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-1767] Make spec explain printer pluggable Created: 31/Aug/16  Updated: 23/Sep/16  Resolved: 23/Sep/16

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

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


 Description   

Same as https://github.com/clojure/clojure/commit/99ab306f82620e6db6a978a5565d2ccd668c0798



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

http://dev.clojure.org/jira/browse/CLJS-1787





[CLJS-1766] Set literals in REPL end up reified as ArrayMap backed PersistentHashSets. Created: 28/Aug/16  Updated: 28/Aug/16

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

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


 Description   

Entering a set literal in the REPL with more than 8 elements should create a PHM backed set but instead it is array backed.

Example (in REPL):
cljs.user=> (type (.-hash-map #{1 2 3 4 5 6 7 8 9}))
cljs.core/PersistentArrayMap

This means operations such as `get` and `contains?` end up doing long scans and are slower than a user would expect.






[CLJS-1765] Empty iterator for some hash maps with a nil key Created: 27/Aug/16  Updated: 19/Sep/16  Resolved: 19/Sep/16

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

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

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

 Description   

Two specific examples:

cljs.user=> (.hasNext (-iterator {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 nil 9}))
false
cljs.user=> (.hasNext (-iterator (hash-map nil 1))))
false

This can affect "user-level" code as follows:

cljs.user=> (sequence (map identity) {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 nil 9})
()
cljs.user=> (sequence (map identity) (hash-map nil 1))
()


 Comments   
Comment by Thomas Mulvaney [ 27/Aug/16 5:27 PM ]

I've attached a simple patch with some tests.

Comment by Mike Fikes [ 27/Aug/16 6:22 PM ]

Confirmed CA at http://clojure.org/community/contributors

Comment by Mike Fikes [ 27/Aug/16 6:35 PM ]

The first ^boolean in the form (or ^boolean (not seen) ^boolean (.hasNext root-tier)) could be omitted.

The patch passes unit tests for me (both JVM and self-host) and causes the correct output to appear for the examples

Comment by Thomas Mulvaney [ 28/Aug/16 4:05 AM ]

Thanks Mike Fikes, I omitted that boolean flag per your suggestion. Updated patch now attached.

Comment by David Nolen [ 14/Sep/16 3:40 PM ]

This patch needs a rebase.

Comment by Thomas Mulvaney [ 14/Sep/16 6:06 PM ]

Most recent patch is rebased.

Comment by David Nolen [ 16/Sep/16 1:30 PM ]

This ticket needs a rebase again. We should probably consider breaking up our core tests into individual namespaces.

Comment by Thomas Mulvaney [ 16/Sep/16 8:47 PM ]

I would be happy to put the tests in a new namespace. Does `src/test/cljs/cljs/hash_map_test.cljs` sound sensible?

Comment by Thomas Mulvaney [ 18/Sep/16 8:27 PM ]

Newest patch has been rebased and tests were split out into the namespace I proposed in previous comment.

Comment by David Nolen [ 19/Sep/16 8:29 AM ]

fixed https://github.com/clojure/clojurescript/commit/7923f80fd50b3a7d1f808dd746758a1375a8e25d





[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-1763] Defining a var that clashes with `cljs.core` throws a compiler error instead of warning Created: 24/Aug/16  Updated: 24/Aug/16  Resolved: 24/Aug/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-1763.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 24/Aug/16 10:43 AM ]

Attached patch with fix and test.

Comment by David Nolen [ 24/Aug/16 11:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/7a06d008fadf56b11dba0f9e2ab97e61059f44fc





[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-1761] Allow parallel Transit analysis cache writes Created: 19/Aug/16  Updated: 30/Sep/16  Resolved: 30/Sep/16

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

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

Attachments: Text File CLJS-1761.patch    

 Description   

With CLJS-1759, Transit analysis cache writes are being serialized. This ticket asks that analysis be done to find the root cause, so we can fix it and removed the workaround in CLJS-1759.



 Comments   
Comment by Francis Avila [ 19/Aug/16 1:53 PM ]

I suspect these lines in transit are a race condition:

https://github.com/cognitect/transit-java/blob/3f359c434264f675460bc2662dde2a1b2d8e9559/src/main/java/com/cognitect/transit/impl/WriterFactory.java#L74-L76

newHandlerCache is a cognitect.transit.impl.Cache instance, which is just a normal LinkedHashMap with size 10.

In between the containsKey() call and the get() call a cached item may have been evicted, causing the JsonEmitter to have a null map handler, causing the "Not supported: class java.lang.Integer" exception to bubble out of the emit() call inside write().

I think the fix is to call newHandlerCache.get(customHandlers) by itself and check for null instead of looking up twice. (I am not a seasoned Java dev, so I am not sure if concurrent reads are safe here without synchronization.) The synchronized block should probably also monitor the cache itself instead of the factory class.

Comment by Francis Avila [ 19/Aug/16 1:55 PM ]

Also, all of this is just from reading the code and your stacktrace, i.e., it is purely hypothesis.

Comment by Francis Avila [ 19/Aug/16 2:19 PM ]

On the other hand, the transit writer is created repeatedly with the same handler map instance, so I don't know why the write handler cache would ever have more than one entry (its limit is 10).

Comment by Mike Fikes [ 19/Aug/16 10:38 PM ]

Francis has logged a ticket against transit-java https://github.com/cognitect/transit-java/issues/20 and I've done some further experimentation and recorded what I found in that ticket.

Comment by Francis Avila [ 22/Aug/16 7:04 AM ]

This patch uses write-handler-map and read-handler-map to completely pre-assemble the transit handler maps and sidestep all handler cache construction caching. (I think this is a good thing to do even without these transit problems.)

If there are indeed races related handler construction and caching, this patch should sidestep them all and allow fully parallel use of transit.

Comment by David Nolen [ 16/Sep/16 1:48 PM ]

fixed https://github.com/clojure/clojurescript/commit/92459050162fce53312d9df7d7b5c703d2d992c2

Comment by Mike Fikes [ 16/Sep/16 7:47 PM ]

CLJS-1761.patch does not resolve this issue. With it applied the failures as described in CLJS-1759 occur.

Comment by Mike Fikes [ 16/Sep/16 8:02 PM ]

I have failed to create a repro for CLJS-1759 that does not involve any downstream tooling (and it also appears to require a multicore box). For reference and for the record, the specific steps used to reproduce the issue are at https://github.com/cognitect/transit-java/issues/20#issuecomment-241312648 (and, if you happen to have access to box that can repro, you can confirm that ClojureScript master with this patch doesn't resolve the issue by editing the project.clj in the referenced downstream project to use a locally-built ClojureScript master).

Comment by David Nolen [ 19/Sep/16 8:31 AM ]

Thanks for the confirmation that the issue is not resolved.

Comment by Mike Fikes [ 29/Sep/16 3:08 PM ]

A PR now exists for transit-java that would provide a fix for this ticket: https://github.com/cognitect/transit-java/pull/21

Comment by David Nolen [ 30/Sep/16 2:06 PM ]

resolved upstream in Transit

Comment by Mike Fikes [ 30/Sep/16 2:29 PM ]

Confirmed fixed downstream using the newly released Transit implementation.

David, do you need a patch to update the ClojureScript project.clj and script/bootstrap?





[CLJS-1760] Self-host: test-cljs-1757 failing in test-self-parity Created: 19/Aug/16  Updated: 19/Aug/16  Resolved: 19/Aug/16

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

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

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

 Description   
$ script/test-self-parity
Testing with Node
WARNING: baz is a single segment namespace at line 1
WARNING: Use of undeclared Var cljs.spec$macros/gen at line 77

Testing cljs.core-test

Testing cljs.reader-test

Testing clojure.string-test

Testing clojure.data-test

Testing cljs.letfn-test

Testing cljs.reducers-test

Testing cljs.binding-test

Testing cljs.macro-test

Testing cljs.top-level

Testing cljs.ns-test.foo

Testing foo.ns-shadow-test

Testing cljs.import-test

Testing cljs.spec-test

ERROR in (test-cljs-1757) (TypeError:NaN:NaN)
expected: (s/exercise-fn (quote cljs.spec-test/cljs-1757-x))
  actual: #object[TypeError TypeError: Cannot read property 'call' of undefined]

Testing cljs.clojure-alias-test

Ran 215 tests containing 17428 assertions.
0 failures, 1 errors.


 Comments   
Comment by António Nuno Monteiro [ 19/Aug/16 10:25 AM ]

Attached path with fix.

Comment by Mike Fikes [ 19/Aug/16 10:39 AM ]

LGTM. It is the “correct” fix; I missed when putting together CLJS-1720. All tests pass for me with António's patch.

Comment by David Nolen [ 19/Aug/16 10:42 AM ]

fixed https://github.com/clojure/clojurescript/commit/86a83d720beb44deb5d55d7d9c0bc2d5174816a3





[CLJS-1759] Errors writing transit analysis cache if parallel build Created: 19/Aug/16  Updated: 19/Aug/16  Resolved: 19/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   
  1. It fails only if parallel builds enabled
  2. When so, it fails, maybe 30 to 50% of the time
  3. If I put a coarse-grained mutex around the write calls, it succeeds
  4. It appears to occur under heave write load 3 concurrent writes

Here is an example of one of the failures:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:out/cljs/source_map/base64.cljs {:file #object[java.io.File 0x125a8ab6 "out/cljs/source_map/base64.cljs"]}, compiling:(/Users/mfikes/Projects/planck/planck-cljs/script/build.clj:16:1)
        at clojure.lang.Compiler.load(Compiler.java:7391)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$script_opt.invokeStatic(main.clj:335)
        at clojure.main$script_opt.invoke(main.clj:330)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:out/cljs/source_map/base64.cljs {:file #object[java.io.File 0x125a8ab6 "out/cljs/source_map/base64.cljs"]}
        at clojure.core$ex_info.invokeStatic(core.clj:4617)
        at clojure.core$ex_info.invoke(core.clj:4617)
        at cljs.compiler$compile_file$fn__3456.invoke(compiler.cljc:1386)
        at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1352)
        at cljs.compiler$compile_file.invoke(compiler.cljc:1332)
        at cljs.closure$compile_file.invokeStatic(closure.clj:474)
        at cljs.closure$compile_file.invoke(closure.clj:465)
        at cljs.closure$eval5203$fn__5204.invoke(closure.clj:541)
        at cljs.closure$eval5139$fn__5140$G__5128__5147.invoke(closure.clj:431)
        at cljs.closure$compile_from_jar.invokeStatic(closure.clj:523)
        at cljs.closure$compile_from_jar.invoke(closure.clj:511)
        at cljs.closure$eval5209$fn__5210.invoke(closure.clj:551)
        at cljs.closure$eval5139$fn__5140$G__5128__5147.invoke(closure.clj:431)
        at cljs.closure$compile_task$fn__5294.invoke(closure.clj:821)
        at cljs.closure$compile_task.invokeStatic(closure.clj:819)
        at cljs.closure$compile_task.invoke(closure.clj:812)
        at cljs.closure$parallel_compile_sources$fn__5300.invoke(closure.clj:848)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1881)
        at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1881)
        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:650)
        at clojure.core$bound_fn_STAR_$fn__4671.doInvoke(core.clj:1911)
        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: java.lang.Exception: Not supported: class java.lang.Integer
        at com.cognitect.transit.impl.WriterFactory$1.write(WriterFactory.java:129)
        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:3127)
        at cljs.analyzer$write_analysis_cache.invoke(analyzer.cljc:3114)
        at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1283)
        at cljs.compiler$emit_source.invoke(compiler.cljc:1232)
        at cljs.compiler$compile_file_STAR_$fn__3433.invoke(compiler.cljc:1304)
        at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1154)
        at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1145)
        at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1293)
        at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1289)
        at cljs.compiler$compile_file$fn__3456.invoke(compiler.cljc:1374)
        ... 29 more


 Comments   
Comment by Mike Fikes [ 19/Aug/16 9:48 AM ]

The attached patch introduces a coarse-grained mutex as indicated in item (3) of the description, providing a work-around for the issue.

Comment by Mike Fikes [ 19/Aug/16 10:43 AM ]

A follow up enhancement ticket that would potentially lead to removing the (presumed) workaround in the patch: CLJS-1761

Comment by David Nolen [ 19/Aug/16 10:44 AM ]

fixed https://github.com/clojure/clojurescript/commit/002708e530b6b3449151d3d077883beeadb92f94





[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-1757] cljs.spec/exercise-fn doesn't work when passed a quoted symbol Created: 17/Aug/16  Updated: 17/Aug/16  Resolved: 17/Aug/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: spec

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

 Description   
cljs.user=> (require '[clojure.spec :as s])
nil
cljs.user=> (defn a [b] 2)
#'cljs.user/a
cljs.user=> (s/fdef a :args (s/cat ::first number?) :ret #(= % 2))
cljs.user/a
cljs.user=> (s/exercise-fn `a)
clojure.lang.ExceptionInfo: Assert failed: (symbol? sym) at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (s/exercise-fn (quote cljs.user/a))}, :tag :cljs/analysis-error}


 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 12:31 PM ]

Attached patch with fix and test.

Comment by David Nolen [ 17/Aug/16 7:32 PM ]

fixed https://github.com/clojure/clojurescript/commit/74db88011dd6dd64f55521f074f0315231be0e12





[CLJS-1756] Add test.check JAR to the bootstrap script Created: 17/Aug/16  Updated: 18/Aug/16  Resolved: 18/Aug/16

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 10:03 AM ]

Added patch with fix.

Comment by David Nolen [ 18/Aug/16 3:59 PM ]

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





[CLJS-1755] Support sourcesContent in source maps Created: 16/Aug/16  Updated: 16/Aug/16

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

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


 Description   

This issue adds sourcesContent support for source maps: https://github.com/google/closure-compiler/issues/1890. This means that your source maps can include your source as well in one bundled file. This makes handling sourcemaps much easier for things like error tracking services. It could also simplify config for source mapping as everything is included in the source map and you don't need to specify relative paths, e.t.c.

This will need to wait for the next release of the Closure Compiler.






[CLJS-1754] Add boolean? generator Created: 16/Aug/16  Updated: 17/Aug/16  Resolved: 17/Aug/16

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

Type: Defect Priority: Major
Reporter: Matt Burbidge Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: cljs, spec

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

 Description   

In clojure I can do
`(gen/generate (s/gen int?)) ;; => 1094`
or
`(gen/generate (s/gen boolean?)) ;; => false`.

In clojurescript I can do
`(gen/generate (s/gen int?)) ;; => -308`.
But in clojurescript I can't do
`(gen/generate (s/gen boolean?)) ;; => Error: Unable to construct gen at: [] for: function cljs$core$boolean_QMARK_{return (x === true) || (x === false);}].`

As far as I can tell it's because there is no boolean generator.



 Comments   
Comment by António Nuno Monteiro [ 17/Aug/16 10:04 AM ]

Added patch with fix and test.

Note that running self parity tests now depends on CLJS-1756 being in place.

Comment by David Nolen [ 17/Aug/16 10:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/2a7454837244cf7de3dfed1e48f46f86c33a1809





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

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

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

Win 7 64bit



 Description   

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

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

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

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

THIS WORKS:

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

BUT THIS DOESN'T:

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

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

Thanks for help!






[CLJS-1752] stest/instrument never throws or :ret and :fn Created: 16/Aug/16  Updated: 16/Aug/16  Resolved: 16/Aug/16

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

Type: Defect Priority: Minor
Reporter: Jan Hein Hoogstad Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cljs, spec


 Description   

When I replace the spec for the following function:

(defn create [query]
  (with-meta (map->Query query) {:spec ::specs/query}))

from:

(spec/fdef create
           :args (spec/cat :query ::specs/query)
           :ret ::specs/query
           :fn #(spec/valid? ::specs/meta (-> %1 :ret :meta)))

(stest/instrument `create)

to

(spec/fdef create
           :args (spec/cat :query ::specs/query)
           :ret string?
           :fn #(spec/valid? ::specs/meta (-> %1 :ret :meta)))

(stest/instrument `create)

it does not throw. As a matter of fact, I didn't get it to throw on any value that I give the :ret and :fn function. Removing the record with a plain map did not make a difference...



 Comments   
Comment by David Nolen [ 16/Aug/16 10:24 AM ]

This is by design.

Comment by Jan Hein Hoogstad [ 16/Aug/16 10:34 AM ]

Thanks for the quick reply, but it is not clear to me why? Can you maybe point me to an explanation?

Cheer!

Comment by Jan Hein Hoogstad [ 16/Aug/16 11:06 AM ]

Never mind, found it. Hadn't noticed it before...





[CLJS-1751] port fix lost type hints in map destructuring Created: 15/Aug/16  Updated: 18/Aug/16  Resolved: 18/Aug/16

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 15/Aug/16 3:24 PM ]

Attached patch with fix.

Comment by David Nolen [ 18/Aug/16 4:08 PM ]

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





[CLJS-1750] With `:language-out :ecmascript5-strict` goog.global is undefined Created: 15/Aug/16  Updated: 16/Sep/16  Resolved: 16/Sep/16

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

Type: Defect Priority: Minor
Reporter: Dusan Maliarik Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: closure


 Description   

When using `:language-out :ecmascript5-strict` the compiled code ends up in a wrapper where `this` resolves to `undefined`. This break goog/base.js where `this` is assigned to `goog.global`.



 Comments   
Comment by David Nolen [ 15/Aug/16 2:34 PM ]

This doesn't seem like a bug but rather a faithful interpretation of strict mode - see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.





[CLJS-1749] Missing `cljs.spec.impl.gen/double*` Created: 15/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/16

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

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

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

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

Attached patch with fix.

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

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





[CLJS-1748] Nth doesn't throw error for strings or arrays Created: 15/Aug/16  Updated: 19/Sep/16  Resolved: 19/Sep/16

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

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

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

 Description   

Calling nth on a string or array with an out of bounds index should throw an error but returns an empty string or nil.

(nth "012" 3) -> ""
(nth (array 0 1 2) 3) nil

Calling nth on a string or array with a negative index and a not-found value returns an empty string or nil.

(nth "012" -1 :not-found) -> ""
(nth (array 0 1 2) -1 :not-found) nil



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

This one needs a rebase.

Comment by Thomas Mulvaney [ 18/Sep/16 8:45 PM ]

Rebased patch attached. Also fixed inconsistent capitalisation on some of the errors being thrown.

Comment by David Nolen [ 19/Sep/16 8:25 AM ]

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





[CLJS-1747] Port `clojure.spec/assert` over to ClojureScript Created: 15/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/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-1747.patch    
Patch: Code and Test

 Comments   
Comment by António Nuno Monteiro [ 15/Aug/16 11:28 AM ]

Attached patch with fix and test.

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

fixed https://github.com/clojure/clojurescript/commit/862950da92581b60abc3bc615305461a7e1c1bfb





[CLJS-1746] Log the result of loading a dependency Created: 14/Aug/16  Updated: 15/Aug/16  Resolved: 15/Aug/16

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

Type: Enhancement Priority: Trivial
Reporter: Andrea Richiardi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1746.patch    

 Description   

It would be great to add a (debug-prn "Loading result: " res) in load-deps so that we see in the logs why a dependency failed to load.
It is a one line patch and I can take care of it.



 Comments   
Comment by Andrea Richiardi [ 14/Aug/16 9:16 PM ]

Patch attached

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

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





[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-1744] rest produces nil for larger maps Created: 13/Aug/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

CLJS-1739 illustrated an error where rest can produce nil for maps of size 9 or larger.

With code like this

(doseq [i (range 1 20)]
  (prn [i (last (take (inc i) (iterate rest (zipmap (range i) (range i)))))]))

you can demonstrate that while CLJS-1739 addressed it for 9, a similar issue occurs at 17.



 Comments   
Comment by David Nolen [ 13/Aug/16 10:54 PM ]

You mean rest not next, right?

Comment by Mike Fikes [ 13/Aug/16 11:07 PM ]

Yes, the first sentence in the description should have said rest can produce nil.

The doseq above produces output that looks like this (note how it produces nil values starting at 17:

[1 ()]
[2 ()]
[3 ()]
[4 ()]
[5 ()]
[6 ()]
[7 ()]
[8 ()]
[9 ()]
[10 ()]
[11 ()]
[12 ()]
[13 ()]
[14 ()]
[15 ()]
[16 ()]
[17 nil]
[18 nil]
[19 nil]
nil
Comment by David Nolen [ 14/Aug/16 8:25 AM ]

fixed https://github.com/clojure/clojurescript/commit/709cc57d003590945329ea8745b799e75ac4ba88





[CLJS-1743] Transient{Hash,Array}Map should support IFn like in Clojure Created: 13/Aug/16  Updated: 13/Aug/16

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

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

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

 Description   

Users should be able to invoke transient maps like in Clojure.






[CLJS-1742] Add docstring for new refer-clojure REPL special Created: 12/Aug/16  Updated: 14/Aug/16  Resolved: 14/Aug/16

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

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

Attachments: Text File CLJS-1742-2.patch     Text File CLJS-1742.patch    
Patch: Code

 Description   

Add a docstring for the new REPL special refer-clojure introduced with CLJS-1730.



 Comments   
Comment by António Nuno Monteiro [ 14/Aug/16 1:58 PM ]

Attached patch with fix.

Comment by António Nuno Monteiro [ 14/Aug/16 2:09 PM ]

Attached revised CLJS-1742-2.patch which drops reference to `inclusion` (used with Clojure's `:only` filter, not supported in ClojureScript)

Comment by David Nolen [ 14/Aug/16 2:36 PM ]

fixed https://github.com/clojure/clojurescript/commit/63e0d7380ee5c45d74a93613aafe982ee600e990





[CLJS-1741] Add :rename to :refer-clojure in ns docstring Created: 12/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/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: docstring

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

 Description   

Currently it indicates the only option is :exclude.



 Comments   
Comment by David Nolen [ 13/Aug/16 10:10 AM ]

fixed https://github.com/clojure/clojurescript/commit/0b8fd1271a13ba4f072f85c0f408dd4dfb174bf5





[CLJS-1740] Self-host: Need to port more of CLJS-1733 Created: 12/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/16

Status: Resolved
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-1740.patch    
Patch: Code

 Description   

With CLJS-1733 some changes were made that needed to be carried over to self-host.

CLJS-1738 took care of a simple argument refactoring, thus leading to script/test-self-parity passing.

But, script/test-self-host is still failing with failures surrounding unit tests involving the ability to disable analyze deps.

Looking further at the changes made for CLJS-1733, you can see two additional things that need to be ported over:

  1. cljs.js makes a few calls to the cljs.analyzer namespace were cljs.analyzer/analyze-deps is used internally and thus needs to be bound (this fixes script/test-self-host).
  2. With https://github.com/clojure/clojurescript/commit/d197bcb2bef327e00bcb39346258ed314a69b8d9 there is a missing that was previously calculated early in a let and its calculation was moved down to occur later at its use point.


 Comments   
Comment by Mike Fikes [ 12/Aug/16 8:09 PM ]

With the attached patch script/test-self-host goes from failing to passing.

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

fixed https://github.com/clojure/clojurescript/commit/712a8d8f1e69b4f440088ce21d32f63a2e363988





[CLJS-1739] seq on map literal with 9 elements leads to rest producing nil Created: 12/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/16

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Description   

Observe that (take 11 (iterate rest {:a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9})) produces a nil instead of () right near the end.

({:e 5, :g 7, :c 3, :h 8, :b 2, :d 4, :f 6, :i 9, :a 1} ([:g 7] [:c 3] [:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:c 3] [:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:h 8] [:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:b 2] [:d 4] [:f 6] [:i 9] [:a 1]) ([:d 4] [:f 6] [:i 9] [:a 1]) ([:f 6] [:i 9] [:a 1]) ([:i 9] [:a 1]) ([:a 1]) nil ())


 Comments   
Comment by Rohit Aggarwal [ 12/Aug/16 5:21 PM ]

The following code works as expected:

(take 11 (iterate rest (array-map :a 1 :b 2 :c 3 :d 4 :e 5 :f 6 :g 7 :h 8 :i 9)))
Comment by David Nolen [ 12/Aug/16 6:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/7fe544696ca7dd45785980097f6033f9274e433b





[CLJS-1738] Self-host: need to update call to check-use-macros-inferring-missing Created: 12/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/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-1738.patch    
Patch: Code

 Description   

script/test-self-parity fails owing to a slight simplification in arguments to check-use-macros-inferring-missing. Also, should incorporate call to check-rename-macros-inferring-missing.



 Comments   
Comment by David Nolen [ 12/Aug/16 3:55 PM ]

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





[CLJS-1737] Self-host: clojure alias implicit macro use regression Created: 11/Aug/16  Updated: 13/Aug/16  Resolved: 13/Aug/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-1737.patch    
Patch: Code and Test

 Description   

If you add a new test case to cljs.clojure-alias-test

(deftest use-macros
  (s/def ::even? (s/and number? even?))
  (is? (s/valid? ::even? 2)))

then script/test-self-parity will fail with

ERROR in (use-macros) (TypeError:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[TypeError TypeError: Cannot read property 'call' of undefined]

and issue warnings while failing:

WARNING: Can't take value of macro cljs.spec/def at line 14
WARNING: Can't take value of macro cljs.spec/and at line 14

This appears to be a regression associated with the change made for CLJS-1716. If you check with the previous commit, it will pass. (Note that the proposed unit test needs to be revised slightly to use is instead of is? in order to be applicable to the older code.)



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

Attached patch with fix and test.

Comment by Mike Fikes [ 13/Aug/16 9:09 AM ]

I tested this locally and it passes script/test-self-parity. With CLJS-1740 it passes script/test-self-host.

I also tried it downstream with Planck and it works there:

cljs.user=> (require '[clojure.spec :as s])
nil
cljs.user=> (s/def ::even? (s/and number? even?))
:cljs.user/even?
cljs.user=> (s/valid? ::even? 3)
false
cljs.user=> (s/valid? ::even? 4)
true

I also tried the above successfully in script/noderepljs, and I successfully ran script/test (both with and without CLJS-1740).

Comment by David Nolen [ 13/Aug/16 10:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/8a49ce62bb5f53167bc44d75ada31156bcec38b0





[CLJS-1736] cljs.spec.test: checkable-syms* called with 0-arity Created: 11/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

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


 Description   

On this line https://github.com/clojure/clojurescript/blob/85bab0736aae442d55d7f25ffc502b5195afe5c1/src/main/cljs/cljs/spec/test.cljc#L235
checkable-sums* is called with no arguments, but the function is only defined for a single argument arity.



 Comments   
Comment by David Nolen [ 11/Aug/16 12:31 PM ]

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





[CLJS-1735] Self-host: cljs.spec speced-vars instance Created: 11/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/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-1735.patch    
Patch: Code

 Description   

Similar to CLJS-1656 but a new instance here: https://github.com/clojure/clojurescript/blob/85bab0736aae442d55d7f25ffc502b5195afe5c1/src/main/cljs/cljs/spec/test.cljc#L105



 Comments   
Comment by David Nolen [ 11/Aug/16 12:32 PM ]

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





[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-1733] Macro inference issue for macros & runtime vars with the same name Created: 11/Aug/16  Updated: 12/Aug/16  Resolved: 12/Aug/16

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

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


 Description   

The new :rename feature has several cases that are currently not covered and should either throw or warn.

A) :refer-clojure does not account for inline macros.

(ns some.test
  (:refer-clojure :rename {+ plus}))

(plus 1 2) ;; produces cljs.core._PLUS_.cljs$core$IFn$_invoke$arity$2((1),(2));
(cljs.core/+ 1 2) ;; produces ((1) + (2));

The macro version should be used automatically as well.

B) Conflicting :refer and :rename

(:require [clojure.string :refer (blank? starts-with?) :rename {blank? starts-with?}])

The :refer starts-with? is chosen and the rename is ignored. The :rename should either throw or emit a warning.

C) Conflicting :rename

(:require [some.ns :refer (foo bar) :rename {foo baz bar baz}])

Both renames want to use the alias baz. This should warn or throw as well.



 Comments   
Comment by David Nolen [ 12/Aug/16 12:14 PM ]

The problem is not specific to :rename. Even with a normal :require :refer you will see that the only the runtime var will get considered if the name is shared.

Comment by David Nolen [ 12/Aug/16 2:07 PM ]

Solved the typical refer case https://github.com/clojure/clojurescript/commit/b49c19819f46a78e41a1e3c9147b1127a006664d

Comment by David Nolen [ 12/Aug/16 2:27 PM ]

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





[CLJS-1732] Add docstrings for new use and use-macros REPL specials Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/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: docstring

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

 Description   

To go along with CLJS-1729



 Comments   
Comment by David Nolen [ 11/Aug/16 8:10 AM ]

fixed https://github.com/clojure/clojurescript/commit/60d534977ace15667c38cf2c2609e27ad72f539a





[CLJS-1731] Self-host: do_template problem with script/test-self-parity Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/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-1731.patch    
Patch: Code and Test

 Description   
Testing cljs.spec-test

ERROR in (conform-explain) (TypeError:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[TypeError TypeError: Cannot read property 'do_template' of undefined]


 Comments   
Comment by Mike Fikes [ 10/Aug/16 4:04 PM ]

The attached patch makes some slight adjustments to the self-host parity unit test namespace loading logic so that it can successfully load the clojure.template namespace.

Note that the patch fixes this issue, but at the same time it reveals that there are now 6 new test failures related to cljs.spec.

Comment by Mike Fikes [ 10/Aug/16 10:05 PM ]

Good news: The changes in CLJS-1720 cover the newly failing unit tests mentioned above.

Comment by David Nolen [ 11/Aug/16 8:08 AM ]

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





[CLJS-1730] Support `refer-clojure` special function in REPLs Created: 10/Aug/16  Updated: 11/Aug/16  Resolved: 11/Aug/16

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

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

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

 Description   

Now that we have support for `:rename` in require specifications, the `refer-clojure` function at the REPL can be a useful addition.



 Comments   
Comment by António Nuno Monteiro [ 10/Aug/16 3:26 PM ]

Attached patch with fix.

Comment by David Nolen [ 11/Aug/16 8:13 AM ]

fixed https://github.com/clojure/clojurescript/commit/39f6d7bae0919fe54e847db2fdcfd621cc6f930d





[CLJS-1729] Support `use` special function in REPLs Created: 10/Aug/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

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

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

 Description   

While `:use` is supported at the NS declaration level, REPLs only support `import` and `require`.

Supporting `use` and `use-macros` in REPLs would be a useful addition.



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

Attached patch with fix

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

fixed https://github.com/clojure/clojurescript/commit/8c6bbf8bc9f5b3e04fd8b172e725894256076e75





[CLJS-1728] Update doc for ns for new :rename capability Created: 09/Aug/16  Updated: 10/Aug/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: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

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

 Description   

Update the docstring for the ns special to cover the :rename option landed with CLJS-1508.



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

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





[CLJS-1727] Regression when evaluating non-sequential forms at the REPL Created: 05/Aug/16  Updated: 08/Aug/16  Resolved: 08/Aug/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
Environment:

Affects master as of commit 8524d7b1ac7c1c418ec394e89322e341070414ca


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

 Description   

this commit [1] assumes that the forms evaluated at the REPL are always sequential by comparing `(first form)` with 'ns.
This makes it impossible to evaluate e.g. numbers or symbols.

[1]: https://github.com/clojure/clojurescript/commit/8524d7b1ac7c1c418ec394e89322e341070414ca



 Comments   
Comment by António Nuno Monteiro [ 05/Aug/16 7:39 AM ]

Attached patch with fix.

Comment by David Nolen [ 08/Aug/16 7:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/2cc8f921cb1e56dbeb901e61a7149d283ad78bc7





[CLJS-1726] demunge is too agreesive and incorrect in some cases Created: 04/Aug/16  Updated: 04/Aug/16

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

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


 Description   

I have implemented some "demunging" logic in cljs-devtools (and dirac) to present original user-friendly names in UI.

During my testing I spotted some wrong edge-cases and incorrect behaviours of demunge:

1) it is too aggressive in replacing dollars - some dollars can be "real" dollars as part of original name
2) it does not revert js-reserved? transformation applied during munging
3) it is oblivious to underscores/dashes - some underscores were "real" underscores before munging
(this may be not complete)

I have worked around those issues on my side and implemented some heuristics[1] based on context, but it is far from perfect.

I'm not sure how to properly fix those, so I wanted to open a ticket with discussion. Maybe people will have some clever ideas.

Currently munging is lossy and we probably don't want to touch it for compatibility reasons.
Maybe we could mark original underscores and dollars in some way, so demunge could properly skip them.

1) One strategy could be to use some (rare) unicode characters, but that would be problematic for people to type.
2) Another strategy could be to escape original dollars and underscores somehow (using more dollars and underscores .
3) Better ideas?

[1] https://github.com/binaryage/cljs-devtools/blob/52899e61e33373df36be8dcb23c69377936821b2/src/lib/devtools/munging.cljs#L154-L185






[CLJS-1725] defrecord does not set cljs$lang$ctorStr Created: 04/Aug/16  Updated: 04/Aug/16

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

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

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

 Description   

While implementing some demunging code[1] for cljs-devtools, I stumbled upon inconsistency in type->str. It works for deftypes but not for defrecords.

The problem is that defrecord does not set! cljs$lang$ctorStr field for some reason. This has been likely overlooked, because type->str is not used often, just for some rare error exceptions as far I can see.

Anyways, this patch fixes it by copy&pasting it from deftype. A better solution to avoid future problems like this would be to extract shared code between deftypes and defrecords, but that seems to be a lot of work.

The patch also adds some tests. I had to add some utility macros to run those tests only in simple mode, because type->str is not supposed to work under advanced builds.
Also test for CLJS-1722 was added to be uncommented after potentially merging it.

[1] https://github.com/binaryage/cljs-devtools/blob/52899e61e33373df36be8dcb23c69377936821b2/src/lib/devtools/munging.cljs#L56-L60






[CLJS-1724] Include IIterable in fast-path-protocols Created: 04/Aug/16  Updated: 04/Aug/16

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

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

Attachments: Text File CLJS-1724.patch    

 Description   

While exploring CLJS record instances printed via cljs-devtools v0.8.0, I spotted that IIterable is not a fast-path protocol:

https://dl.dropboxusercontent.com/u/559047/iiterable-slow-path.png

Please notice that IIterable is the only white label in the protocol list there.






[CLJS-1723] pr-writer-ex-info does not print false values and tries to reimplement map printer Created: 04/Aug/16  Updated: 08/Aug/16

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

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

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

 Description   

While working on CLJS-1722 I spotted two issues with pr-writer-ex-info:

1) if data or cause happen to be "false", they are skipped in printing
2) the code tries to mimic map printer

This patch fixes both issues. It turns out printing real info map yields exactly same string output and plays better with cljs-devtools printer (which newly sees real map instead of soup of strings and values).

For reference: pr-writer-ex-info was implemented in CLJS-983



 Comments   
Comment by David Nolen [ 08/Aug/16 7:39 AM ]

Can you explain why you think 1 is a problem? data should be a map. cause should be an error but I suppose that could be a problem if you have a rethrow which wraps some throwable value (which of course could be anything).

Comment by Antonin Hildebrand [ 08/Aug/16 2:32 PM ]

Disclaimer: I don't have much experience with Clojure and almost none with Java.

Just by reading the code I didn't see any checks in ex-info (or in ExceptionInfo ctor) to restrict data or cause. So I assumed anything can be passed in and printer should print it as-is without additional logic. I assumed that checking for nil values was intentional abbreviation when CLJS-983 was implemented. And I assumed that false value was supposed to be printed. At least I personally would want it to be distinguished from nil. Or there should be no abbreviation at all.





[CLJS-1722] Upgrade ExceptionInfo to proper deftype Created: 03/Aug/16  Updated: 04/Aug/16

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

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

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

 Description   

Currently ExceptionInfo is implemented as a raw constructor function which inherits from js/Error with some ad-hoc javascript-level patches to satisfy a tiny subset of deftype functionality (mainly for printing).

Unfortunately this does not play well with cljs-devtools[1]. This problem surfaced when I started playing with ExceptionInfo and cljs-devtools v0.8 which newly supports printing deftypes[2]. ExceptionInfo does not contain getBasis, cljs$lang$type, cljs$lang$ctorStr and similar machinery.

My proposed patch implements ExceptionInfo as a proper deftype and does some patch-work to provide backward compatibility. I'm pretty sure we must not break current contract of ExceptionInfo constructor accepting 3 args and synthesizing other fields on the fly in constructor.

Implementation details:
1) first we define ExceptionInfo as normal deftype (to get a template)
2) then we remember reference to ExceptionInfo in ExceptionInfoTypeTemplate
3) then we redefine ExceptionInfo with raw constructor function which should mimic original behaviour (by scraping newly created js/Error instance, but calling ExceptionInfoTypeTemplate to do proper deftype initialization)
4) then we copy keys from ExceptionInfoTypeTemplate over ExceptionInfo
5) then we set ExceptionInfo's prototype to be ExceptionInfoTypeTemplate's prototype
6) then we fix ExceptionInfo's prototype's constructor to point to our re-defined constructor function
7) then we patch ExceptionInfo's prototype to inherit from js/Error (note this clobbers ExceptionInfoTypeTemplate as well - but we don't care about it)

This effectively gives us properly working ExceptionInfo deftype with redefined constructor function wrapping deftype's constructor for backwards compatibility.
We also patch ExceptionInfo's prototype to inherit from js/Error the same was as original code did.

Note: With working deftype, we can move IPrintWithWriter and toString implementation to the deftype itself.

[1] https://github.com/binaryage/cljs-devtools/issues/23
[2] https://github.com/binaryage/cljs-devtools/releases/tag/v0.8.0



 Comments   
Comment by Thomas Heller [ 04/Aug/16 4:25 AM ]

Why not just add the missing getBasis, cljs$lang$type, cljs$lang$ctorStr bits per set!?

The patch looks like it would mess up advanced compilation although that is just an instinct not something I verified, did you?

Comment by Antonin Hildebrand [ 04/Aug/16 4:44 AM ]

I ran clojurescript tests and I assumed they run also against advanced-mode build. During development when my tests were failing I saw error messages about minified names.

This may seem as a hacky solution, but IMO the original code was also a hack. My hack will stay up-to-date with future changes to deftype implementation. I can imagine people would forget to update this part when touching deftype.

btw. there is another patch coming related to discrepancies between deftype and defrecord. That could have been avoided if defrecord shared common implementation with deftype.

Comment by Thomas Heller [ 04/Aug/16 6:27 AM ]

Closure is usually very strict about re-defining stuff but I guess my instincts were wrong, the tests should cover advanced.

My issue with this is that deftype is for defining Clojure-specific types. ExceptionInfo is not since it inherits from Error, just like you can't have a superclass in Clojure you can't in CLJS. So IF we were to change deftype in the future we might break things in unexpected ways that just re-use deftype but aren't actually deftype.

Yes, you have to do some house-keeping but you can't enforce the rules of deftype when dealing with inheritance.

Just my 2 cents, it has advantages to re-use deftype too (as you suggested).

Comment by Antonin Hildebrand [ 04/Aug/16 6:36 AM ]

Unfortunately I was unable to look up any comments or docs explaining the reasoning why we do that js/Error inheritance there.

My first attempt to "fix" ExceptionInfo was to simply implement it as an ordinary deftype. And that worked just fine (for my tests). Then I tried to re-implement original behaviours on top just to make it 100% compatible.

Comment by Antonin Hildebrand [ 04/Aug/16 12:47 PM ]

Just adding a motivational screenshot:

https://dl.dropboxusercontent.com/u/559047/CLJS-1722-example.png

Those yellow warnings are listing which properties are getting copied by gobject/extend call.
The expanded log item is new implementation logged via cljs-devtools v0.8.0.
The last log item is the old implementation logged via cljs-devtools v0.8.0 (cljs-devtools does not recognise ExceptionInfo as CLJS type, but detects IPrintWithWriter and uses it to present the value)





[CLJS-1721] 3-arity get-in fails on types which do not implement ILookup Created: 02/Aug/16  Updated: 10/Aug/16  Resolved: 10/Aug/16

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

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

Attachments: Text File CLJS-1721.patch    

 Description   

The following are examples which fail:

(get-in {:a "data"} [:a 0] :not-found)
(get-in {:a (array 0 1 2 3)} [:a 0] :not-found)

Whilst the 2-arity version succeeds.



 Comments   
Comment by Thomas Mulvaney [ 04/Aug/16 7:57 AM ]

Also worth noting that the 3-arity version is quicker than the 2-arity version on long paths because it bails when a lookup fails. For example (get-in {} [:a :b :c :d :e :f]) is slower than (get-in {} [:a :b :c :d :e :f] :not-found) which bails out as soon a lookup fails. The former continues to call `get` on nil for the rest of the keys.

Perhaps it would be better for the 2-arity version to make a call to the 3-arity version passing nil as the last argument to take advantage of the early return. Would be happy to update the patch to include this.

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

fixed https://github.com/clojure/clojurescript/commit/817b44d34795696eda473776e355fb956d3aa5ae





[CLJS-1720] Self-host: Qualify symbols and namespaced keywords in spec macros Created: 31/Jul/16  Updated: 11/Aug/16  Resolved: 11/Aug/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-1720-2.patch     Text File CLJS-1720-3.patch     Text File CLJS-1720.patch    
Patch: Code

 Description   

There are currently a few symbols and namespaced keywords in the cljs.spec macros namespace that either need to be qualified for proper operation, or should be.

The symbols fall into the category of calls to runtime functions in the cljs.spec namespace, and need qualification in order to avoid the $macros suffix. These comprise with-gen and gen.

In terms of keywords, an example that causes a failure is ::kvs->map in keys*: It resolves to :cljs.spec$macros/kvs->map which doesn't match the :cljs.spec/kvs->map spec registered near the bottom of the cljs.spec runtime namespace.

An example that doesn't cause an outright failure, but arguably inhibits its proper use by client code is ::nil and ::pred in the nilable macro. Ideally these would resolve to :cljs.spec/nil and :cljs.spec/pred so that client code can rely on these namespaced symbols identifying the branches.

Given the nilable example, you could argue that the intent is that all namespaced keywords in the cljs.spec macro namespace that employ :: resolve in :cljs.spec so that they can be used not simply as unique identifiers (the intent is not simply to avoid potential collisions), but so that they can be used as stable identifiers.

The set of keywords that should be qualified comprises: ::kind-form, ::kfn, ::conform-all, ::kvs->map, ::nil, and ::pred



 Comments   
Comment by Mike Fikes [ 10/Aug/16 9:43 PM ]

Patch no longer applies; attaching re-baselined CLJS-1720-2.patch

Comment by Mike Fikes [ 10/Aug/16 9:55 PM ]

Sorry David, I botched that last re-baseline. Attaching a corrected CLJS-1720-3.patch.

Comment by David Nolen [ 11/Aug/16 8:09 AM ]

fixed https://github.com/clojure/clojurescript/commit/20b08fe442a24c46526c812f6c52ad8c7d3f6c9b





[CLJS-1719] Port destructuring namespaced keys and symbols Created: 29/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/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-1719.patch    
Patch: Code and Test

 Description   

Port CLJ-1919 to Clojurescript



 Comments   
Comment by António Nuno Monteiro [ 29/Jul/16 6:44 PM ]

Attached patch with code and test.

Comment by David Nolen [ 29/Jul/16 8:46 PM ]

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





[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-1717] remove unnecessary map from equiv-map Created: 29/Jul/16  Updated: 29/Jul/16  Resolved: 29/Jul/16

Status: Closed
Project: ClojureScript
Component/s: None