<< Back to previous view

[CLJS-1987] don't index node modules blindly Created: 21/Mar/17  Updated: 21/Mar/17

Status: Open
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: Unresolved Votes: 0
Labels: None

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

 Description   

We shouldn't call `cljs.closure/index-node-modules` if `npm-deps` hasn't been specified






[CLJS-1539] Parallel compilation fails on circular dependencies Created: 06/Jan/16  Updated: 21/Mar/17  Resolved: 06/Jan/16

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

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


 Description   

Master test will not run due to the circular dependency between circular-deps.a and circular-deps.b test namespaces.



 Comments   
Comment by David Nolen [ 06/Jan/16 2:34 PM ]

We should probably just validate that the inputs do not include a circular dependency before starting parallel compilation.

Comment by David Nolen [ 06/Jan/16 4:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/5724f72ad92d62b93cdf1afe19e4bbfc7b6ddecc

Comment by Maarten Truyens [ 20/Mar/17 5:30 AM ]

In version 1.9.494 (but also earlier versions, I tried up to 1.9.293), in my fairly large codebase of about 200 CLJC/CLJS namespaces, I have noticed that circular dependencies sometimes cause the compiler to simply idle after initial compilation efforts, without any error indication whatsoever. I then have to kill the compiler, disable parallel building, and restart it, in order to be notified about the actual circular dependency error. This happens both with (1) a clean and then cold recompile of a version of the codebase that happens to contain the error, and (2) during a Figwheel editing session where everything was fine until I happen to introduce the circular dependency. For clarity: in the former case, the compiler does start working during about 10 seconds, but then simply stops and idles (a full recompile takes about 40 seconds on my codebase).

I have tried recreating this error with another codebase, but was unable to do so, so it could be related to the size of the codebase. Do you have any idea where this could be coming from?

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

So are you saying if you disable parallel compilation and you try to build it fails with a circular dependency error?

Comment by Maarten Truyens [ 21/Mar/17 3:07 AM ]

Correct: when I disable parallel compilation, the build will (rightfully) fail.





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

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

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

Attachments: Text File generators.patch    
Patch: Code

 Description   

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

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



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

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

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

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

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

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

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





[CLJS-1984] Fix one-argument arity implementation of cljs.closure/index-node-modules Created: 18/Mar/17  Updated: 19/Mar/17

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

Type: Defect Priority: Trivial
Reporter: Thomas Spellman Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bug, patch
Environment:

N/A


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

 Description   

cljs.closure/index-node-modules is a multimethod, and the 1-arity args are getting dropped in the pass to the 2-arity method causing it to loop until stack overflow. The attached patch fixes it in latest master (1.9.508).






[CLJS-1985] `index-node-modules` should pass opts to `node-inputs` Created: 18/Mar/17  Updated: 18/Mar/17

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

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

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




[CLJS-1964] Validate that `:target :nodejs` and no optimizations requires a `:main` option to be present Created: 01/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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: newbie, nodejs

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

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

fixed https://github.com/clojure/clojurescript/commit/197ff2e7c3f96b365e31cf4c1a4af39bdd60fc88





[CLJS-1956] Add missing JS reserved keywords Created: 27/Feb/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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

 Comments   
Comment by David Nolen [ 17/Mar/17 2:33 PM ]

fixed https://github.com/clojure/clojurescript/commit/2171ae9859a2e982497764a04de10916aae68307





[CLJS-1983] res -> mres in spec.cljs Created: 17/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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

 Comments   
Comment by David Nolen [ 17/Mar/17 2:26 PM ]

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





[CLJS-1976] hash-map assoc stackoverflow Created: 13/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Using clojurescript master and a browser environment


Attachments: Text File CLJS-1976.patch    

 Description   

A particular combination of keys in a hash-map will cause a stackoverflow when added into the same hash-map, whether using transient or non-transient assoc.

Minimal case:

(defrecord T [index a b])

(def bad-key-1 (map->T {:index :eavt, :a 17592186192276, :b nil}))
(def bad-key-2 (map->T {:index :avet, :a 10, :b :fhir.ElementDefinition/minValueDateTime$cr}))

;; Implicit Transient assoc
(hash-map bad-key-1 nil bad-key-2 nil)

;RangeError: Maximum call stack size exceeded

;;; From a browser console:
;core.cljs:5034 Uncaught RangeError: Maximum call stack size exceeded
;cljs.core.PersistentVector.cljs$core$ISeqable$_seq$arity$1 @ core.cljs:5034
;cljs$core$seq @ core.cljs:1121
;cljs.core.concat.cljs$core$IFn$_invoke$arity$2 @ core.cljs:3619
;cljs.core.LazySeq.sval @ core.cljs:3269
;cljs.core.LazySeq.cljs$core$ISeqable$_seq$arity$1 @ core.cljs:3323
;cljs$core$seq @ core.cljs:1121
;cljs.core.pr_str.call.cljs.user.T.cljs$core$ISeqable$_seq$arity$1 @ VM2295:158
;cljs$core$seq @ core.cljs:1121
;cljs$core$first @ core.cljs:1143
;(anonymous) @ core.cljs:4049
;cljs$core$every_QMARK_ @ core.cljs:4049
;cljs$core$equiv_map @ core.cljs:5836
;(anonymous) @ VM2295:114
;cljs.core.pr_str.call.cljs.user.T.cljs$core$IEquiv$_equiv$arity$2 @ VM2295:118
;cljs$core$_equiv @ core.cljs:617
;cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2 @ core.cljs:1179
;cljs$core$key_test @ core.cljs:6494
;cljs.core.BitmapIndexedNode.inode_assoc_BANG_ @ core.cljs:6752
;cljs.core.create_node.cljs$core$IFn$_invoke$arity$7 @ core.cljs:7050
;(anonymous) @ core.cljs:6760
;;; Last 3 lines repeat forever


;; Explicit non-transient assoc
(assoc (hash-map bad-key-1 nil) bad-key-2 nil)

;RangeError: Maximum call stack size exceeded
;;; Stack is similar to above, except `inode-assoc` instead of `inode-assoc!`
;;; is repeating, because this version is not transient

As near as I can tell, the fundamental problem is that the (zero? (bit-and bitmap bit)) test in inode-assoc is always false. I have a hunch this is due to some bit-twiddling difference between js and Java that isn't accounted for in bit-count or bitmap-indexed-node-index or the recursive call to inode-assoc. I am investigating further.



 Comments   
Comment by Francis Avila [ 13/Mar/17 6:13 PM ]

(Just to note: we encountered this issue in production.)

The fundamental problem is that JS hash values may use more than 32 bits, but only 32 bits should be considered.

These two records have different hash values, but if only 32 bits are compared they have the same hash value. At least create-node naively assumes that hash values are normalized and does normal comparison between them using ==, meaning it does not detect the hash collision correctly.

This is a problem for records because they use an older (pre-murmur) hash-imap, which is not careful to truncate to 32 bits.

However, it is also a problem for dates for the same reason:

(hash-map #inst "2017-03-13T22:21:08.666-00:00" nil  #inst "2015-11-02T19:53:15.706-00:00" nil)
;RangeError: Maximum call stack size exceeded

My proposed fix is to force all hash values to be normalized to 32 bits to preserve the invariant that two equal hash values means they collide.

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

fixed https://github.com/clojure/clojurescript/commit/3449c5328356fb4951d1a950158b88e339818d0b





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

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

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


 Description   

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

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

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






[CLJS-1973] Add support for `:npm-deps` in upstream `deps.cljs` Created: 11/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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-1973.patch    
Patch: Code

 Comments   
Comment by Thomas Heller [ 12/Mar/17 4:19 AM ]

Wrote about this in Slack but I wanted to properly voice my concerns so they don't get lost in Slack archives.

I think managing npm via CLJS is a bad idea. While this is very interesting it could/should probably happen in a library. We will inherit all the woes npm brings and force this onto the user. What if the user prefers yarn? What if the user is using a tool with support for package.json but not deps.cljs?

I voiced similar concerns a while ago [1] and with everything happening recently with regards to JS modules the situation is getting worse. It may appear that this work must happen in CLJS but really could happen somewhere else without making cljs.core more complicated. clojure.core doesn't bundle with leiningen for good reason.

IMHO, YMMV

[1] https://groups.google.com/d/msg/clojurescript/xMQuEmQt7oQ/AWT5RZe_nQQJ

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

fixed https://github.com/clojure/clojurescript/commit/21da03e03e229a0b38fb872485d26a30fbf034b8





[CLJS-1978] Port CLJ-2035 Created: 14/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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: spec

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

 Description   

CLJ-2035 has made it into Clojure 1.9.0-alpha15



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

Attached patch with fix and tests.

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

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





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

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

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

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

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

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



 Description   

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

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



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

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





[CLJS-1979] Port CLJ-2043 (fix s/form of s/conformer) Created: 14/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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: spec

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

 Description   

this is in clojure 1.9.0-alpha15



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

fixed https://github.com/clojure/clojurescript/commit/6f4b313e4f9523e9e8345f13c4a55bb1b6f93ebc





[CLJS-1980] port CLJ-2100 (s/nilable form should retain original spec form) Created: 14/Mar/17  Updated: 17/Mar/17  Resolved: 17/Mar/17

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

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

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

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

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





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

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

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


 Description   

Little inconsistence with Clojure here:

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

while in lumo, planck and replumb:

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

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

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





Generated at Wed Mar 22 21:18:13 CDT 2017 using JIRA 4.4#649-r158309.