[CLJS-2531] Self-host: Can't require cljs.js in script/nashornrepljs on Java 9 Created: 16/Feb/18  Updated: 18/Feb/18

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   

The below works under Java 8, but not 9:

$ java -version
java version "9.0.4"
Java(TM) SE Runtime Environment (build 9.0.4+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.4+11, mixed mode)
$ script/nashornrepljs
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
Function has more than 262143 program points
cljs.user=> cljs.js
nil

Likely closely related to CLJS-2530.



 Comments   
Comment by Mike Fikes [ 18/Feb/18 6:20 PM ]

The JavaScript code produced for (dump-core) is evidently large enough to cause us to hit a limit that existed in JDK 8 but is now more easy to hit with JDK 9:

Specifically see

http://hg.openjdk.java.net/jdk9/jdk9/nashorn/file/17cc754c8936/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ProgramPoints.java#l57

and

http://hg.openjdk.java.net/jdk9/jdk9/nashorn/file/17cc754c8936/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornCallSiteDescriptor.java#l156

The limit in question was revised in this changeset http://hg.openjdk.java.net/jdk9/jdk9/nashorn/rev/7cb19fa78763#l48.108

where CALLSITE_PROGRAM_POINT_SHIFT was increased from 11 to 14, reducing MAX_PROGRAM_POINT_VALUE from 21 bits to 18 bits. (2^18 - 1 = 262143)

This is associated with this ticket https://bugs.java.com/view_bug.do?bug_id=8139931

Note that if you read the comments in the JDK code, you can see that there is an assumption that a JavaScript method splitter will have taken a large function and broken it down to sufficiently small chunks to fit within the limits. Perhaps the function generated by ClojureScript isn't amenable to splitting.





[CLJS-2530] Self-host: empty-state fails in Nashorn / Java 9 Created: 16/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

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

Java 9.0.4



 Description   

This works if you are using 1.8.0_162 but not Java 9.0.4:

$ java -jar cljs.jar
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)

cljs.user=> (cljs.js/empty-state)
	 <anonymous> (.cljs_nashorn_repl/cljs/core.cljs:4450:16)
	 cljs$core$IFn$_invoke$arity$2 (.cljs_nashorn_repl/cljs/core.cljs:4450:6)
	 cljs.core/swap! (.cljs_nashorn_repl/cljs/core.cljs:4443:1)
	 cljs$core$IFn$_invoke$arity$0 (.cljs_nashorn_repl/cljs/js.cljs:134:7)
	 cljs$js$empty_state (.cljs_nashorn_repl/cljs/js.cljs:129:1)
	 (NO_SOURCE_FILE <eval>:1:0)
	 (NO_SOURCE_FILE <eval>:1:0)
	 (NO_SOURCE_FILE <eval>:1:0)
cljs.user=> (.-message *e)
"Code generation bug in \"cljs$core$IFn$_invoke$arity$0#L:185#L:186\": likely stack misaligned: java.lang.NullPointerException .cljs_nashorn_repl/goog/../cljs/js.js"

Likely closely related to CLJS-2531.



 Comments   
Comment by Mike Fikes [ 18/Feb/18 6:23 PM ]

This is a duplicate of CLJS-2531.





[CLJS-2400] test-module-processing and test-global-exports fail under Java 9 Created: 17/Nov/17  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

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


 Description   

If you apply the fix in CLJS-2377, and then run script/test under Java 8, things pass, but under Java 9 test-module-processing and test-global-exports fail:

ERROR in (test-module-processing) (TypeError:NaN:NaN)
expected: (= (array/nth #js [1 2 3] 1) 2)
  actual: #object[TypeError TypeError: g4.ki is not a function]

ERROR in (test-global-exports) (TypeError:NaN:NaN)
expected: (= (plus 1 2) 3)
  actual: #object[TypeError TypeError: Cannot read property 'add' of undefined]


 Comments   
Comment by Mike Fikes [ 17/Nov/17 6:13 PM ]

For the first error, under Java 8, the :advanced JavaScript (in builds/out-adv/core-advanced-test.js) has this bit of code in it

var b=f4.nth([1,2,3],1);

while under Java 9, we evidently have a rename failure:

var b=g4.ki([1,2,3],1);
Comment by Mike Fikes [ 18/Feb/18 9:38 AM ]

See CLJS-2529

Comment by Mike Fikes [ 18/Feb/18 4:36 PM ]

Fixed with CLJS-2529





[CLJS-2492] Self-host: munge file name when appending source maps Created: 07/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

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

Attachments: Text File CLJS-2492-0.patch     Text File CLJS-2492-1.patch     Text File CLJS-2492-2.patch     Text File CLJS-2492-3.patch    

 Description   

In self-host, we generate source map stack traces like:

{"version":3,"file":"foo/user_with_underscores.js?rel=1518049631768", ...}

If the file field ends up having underscores, at the moment these are not converted back to dashes when mapping back, for example with mapped-stacktrace-st.



 Comments   
Comment by Andrea Richiardi [ 07/Feb/18 7:55 PM ]

Another thing I did not verify is the other way around...maybe source map keys are actually stored with underscores in normal Cljs. In lumo I get keys with dashes so a conversion is necessary, patch coming.

Comment by Andrea Richiardi [ 07/Feb/18 8:03 PM ]

Here is patch plus tests.

Comment by David Nolen [ 13/Feb/18 6:28 PM ]

I'm skeptical about this change as it's perfectly fine to have ClojureScript namespaces with underscores in them.

Comment by Andrea Richiardi [ 14/Feb/18 4:34 PM ]

After Slack conversation with David I am closing this. I will try to find a way to write the source map keys so that it does not require any modification of the reading side (the target of this patch). This would probably mean, as suggested by David, using munge somewhere along the way. Thank you!

Comment by Andrea Richiardi [ 14/Feb/18 7:38 PM ]

I further investigated and I see here that the code that assoc-in uses name. For an generic statement/expression though, name is usually not really significant and while we could make document that says that name has to be munged because it will end up in :source-maps, I see that you are already doing it the same at this

I was then wondering whether the code should use that `smn` for the key in :source-maps. Ready for patching if necessary.

Comment by Andrea Richiardi [ 14/Feb/18 7:47 PM ]

An example of what happens when name is not transformed somehow is this:

cljs.user=> (keys (:source-maps @lumo.repl/st))
(cljs.spec.alpha$macros
 cljs.pprint$macros
 expound.printer
 lumo.repl$macros
 "(require '[lumo.repl :refer [apropos find-doc] :refer-macros [dir doc source]])"
 expound.alpha
 cljs.analyzer.macros$macros
 cljs.tools.reader.reader-types$macros
 cljs.analyzer.api
 cljs.spec.gen.alpha$macros
 cljs.reader$macros
 repro.expound-tuple
 cljs.pprint
 cljs.env.macros$macros
 expound.util
 "(require '[repro.expound-tuple :as e])"
 expound.paths
 cljs.compiler.macros$macros
 expound.problems)
Comment by Andrea Richiardi [ 15/Feb/18 3:28 PM ]

I attached a test of what I want to achieve, which fails at the moment.

Comment by Andrea Richiardi [ 15/Feb/18 3:32 PM ]

Note that it actually does not matter what the name param to eval-str is, it will be assigned to :source-maps without underscore, but then parsing it back is not in my control (cljs.stacktrace takes care of that).

Comment by Andrea Richiardi [ 16/Feb/18 12:36 PM ]

Attaching a patch that fixes the problem and adds a couple of tests. It was an easy fix after all, hopefully valid.

Comment by Andrea Richiardi [ 16/Feb/18 12:43 PM ]

Ready for screening

Comment by Andrea Richiardi [ 18/Feb/18 3:03 PM ]

As per our Slack conversation, here a patch for handling nil plus test.

Comment by David Nolen [ 18/Feb/18 4:30 PM ]

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





[CLJS-2535] cljs.main :analyze-path support Created: 18/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

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

Attachments: Text File CLJS-2535.patch    
Approval: Accepted

 Description   

:analyze-path doesn't appear to be currently supported via cljs.main.

Steps to repro:

Have src/foo/core.cljs with

(ns foo.core)

(defn square [x]
  (* x x))

and index.html with

<html>
    <body>
        <script type="text/javascript" src="out/main.js"></script>
    </body>
</html>

and deps.edn with a recent hash or pointing at the local clojurescript tree:

{:deps
  {org.clojure/clojurescript {:git/url "https://github.com/clojure/clojurescript"
                              :sha "87e61bd45cb9f4897f0bb7caa75d550461ebd72b"}}}

Start up the browser REPL via:

clj -m cljs.main -re browser -d "out" -o "out/main.js" -c foo.core -r

and in a browser go to http://localhost:9000

Observe that the path is not currently analyzed, even though foo.core has been loaded:

cljs.user=> (ns-interns 'foo.core)
{}
cljs.user=> (foo.core/square 3)
WARNING: Use of undeclared Var foo.core/square at line 1 <cljs repl>
9

Attempt to rectify that by editing out/cljsc_opts.edn and adding :analyze-path ["src"] and re-running the compile / repl command above and re-connecting with the browser. You will see that the :analyze-path option has been removed from out/cljsc_opts.edn and the REPL behavior reflects that the path is not analyzed.

Finally, start up the browser REPL directly using

clj -e "(require '[cljs.repl :as repl] '[cljs.repl.browser :as browser])" -e "(repl/repl (browser/repl-env) :analyze-path [\"src\"])"

with this you will see that the path has been analyzed:

cljs.user=> (ns-interns 'foo.core)
{square #'foo.core/square}
cljs.user=> (foo.core/square 3)
9


 Comments   
Comment by Mike Fikes [ 18/Feb/18 9:02 AM ]

The attached patch fixes the issue by doing two things:

  1. When doing a compile, it first reads any existing cljsc_opts.edn and uses it as the base set of opts that are merged against when producing the opts passed to cljs.build.api/build. Without this, any user-specified opts in cljsc_opts.edn will be ignored and the file clobbered.
  2. Any REPL opts discovered are conveyed by injecting them into the :options portion of the cfg passed to the REPL launch fn.
Comment by David Nolen [ 18/Feb/18 4:29 PM ]

fixed https://github.com/clojure/clojurescript/commit/06e591d02dcce8abc253826f9e4de9dbb4b01e06





[CLJS-2529] Externs are ignored under Java 9 Created: 16/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

Type: Defect Priority: Major
Reporter: Jan Břečka Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: compiler
Environment:

Java 9, macOS High Sierra 10.13.3


Attachments: Text File CLJS-2529-2.patch     Text File CLJS-2529.patch    
Patch: Code
Approval: Accepted

 Description   

When compiling any project with :optimizations set to :advanced under Java 9 (build 9.0.1+11), externs specified in :externs compiler option are not applied.

I did some digging and found out that cljs.js-deps/all-classpath-urls function always returns empty list, so no js resource can be resolved.



 Comments   
Comment by Mike Fikes [ 18/Feb/18 9:39 AM ]

See CLJS-2400

Comment by Jan Břečka [ 18/Feb/18 11:05 AM ]

Can be reproduced with https://github.com/funcool/promesa (version 1.9.0) dependency. The problem is renamed Promise.noConflict() function.

Comment by David Nolen [ 18/Feb/18 4:28 PM ]

fixed https://github.com/clojure/clojurescript/commit/8670cc40cf09f90f1d36cb4bf0a2c68713fb1d68





[CLJS-2534] Eliminate icon suppression code Created: 18/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

 Description   

There is icon suppression code in cljs.cli/-main that is no longer needed.



 Comments   
Comment by David Nolen [ 18/Feb/18 8:45 AM ]

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





[CLJS-2523] RecordIter should return MapEntry Created: 15/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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


 Description   

RecordIter should spit out MapEntry rather than PersistentVector on iteration.

The RecordIter is used when doing reduce on a a record.



 Comments   
Comment by David Nolen [ 18/Feb/18 8:04 AM ]

fixed https://github.com/clojure/clojurescript/commit/46167fe9c216e054386c825d2c8a7742f37f4331





[CLJS-2525] Nashorn process fails to terminate Created: 15/Feb/18  Updated: 18/Feb/18  Resolved: 18/Feb/18

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

 Description   

If you compile to JavaScript and run in Nashorn, the process doesn't terminate.

(This is the case if running the core.async tests, for example.)



 Comments   
Comment by Mike Fikes [ 15/Feb/18 1:11 PM ]

If we revise the executor to use daemon threads (by making it use a thread factory that creates daemon threads), this would eliminate blocking during process shutdown, but it would also eliminate the desired blocking that occurs during orderly shutdown here https://github.com/clojure/clojurescript/commit/a4713db2ca94b53cbad600c4e2675c2ccd63f676

The attached patch meets both goals by setting the executor so that it can have 0 threads, but at most 1 thread. The result is that it causes single-threaded execution if there is one or more task in flight, but it will back down to 0 threads when there are none, thus allowing shutdown. (As an aside, the previous code was incorrect in that it was not setting the maximum number of allowed threads, nor calling newSingleThreadScheduledExecutor).

Comment by David Nolen [ 18/Feb/18 7:38 AM ]

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





[CLJS-2533] Document that -t accepts none Created: 18/Feb/18  Updated: 18/Feb/18

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

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

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

 Description   

To go along with the change for CLJS-2532






[CLJS-2532] cljs.main targetless compile Created: 17/Feb/18  Updated: 17/Feb/18  Resolved: 17/Feb/18

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

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


 Description   

It would be nice to be able to invoke -c without the target defaulting to Nashorn.

Steps to repro:

Let src/foo/core.cljs contain (ns foo.core) and deps.edn set up for a recent hash:

{:deps
  {org.clojure/clojurescript {:git/url "https://github.com/clojure/clojurescript"
                              :sha "6f05654dccd3460e054790122f1d321cec172ca8"}}}

With this in place, compile the source using the build API:

clojure -e "(require 'cljs.build.api)" -e '(cljs.build.api/build "src" {:output-to "out/main.js"})'

and then take a look at out/main.js:

goog.addDependency("base.js", ['goog'], []);
goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.Uri', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
goog.addDependency("../foo/core.js", ['foo.core'], ['cljs.core']);

Now remove the out directory and attempt to replicate this build using the new cljs.main -c feature:

clojure -m cljs.main -o out/main.js -c foo.core

If you look at the resulting out/main.js it will contain references to Java classes and code to load "nashorn.js".

It would be nice that if there were no -t option, it would not default to nashorn, if feasible. Or, alternatively support for an explicit -t none or somesuch might be nice.



 Comments   
Comment by Mike Fikes [ 17/Feb/18 8:59 PM ]

By specifying a namespace after -c, this is equivalent to adding a :main option to the compiler map passed to cljs.build.api/build, which causes the out/main.js to be more than just the Closure dependency graph loading code, but code to load the main namespace.

Another way to look at this: At the beginning of Quick Start, the first builds done don't involve specification of the main namespace, but lead the user to explicitly adding script tags to load this namespace, later followed by making use of the :main option to implicitly care of that.

In this light, it appears what I was really asking for in this ticket would be an ability to omit a namespace for -c in order to produce a "clean" compile that doesn't include target-specific code to load the main namespace. This would only be used in environments like Planck or Ambly that take care of bootstrapping the Closure environment, so is not really worth doing (those builds can just use cljs.build.api/build directly, and this ticket could be declined).

This does raise an interesting side question for the beginning of Quick Start. If Quick Start is rewritten in terms of cljs.main and variations involving -c and -r, while completely avoiding cljs.build.api/build, then the first build or two may not be possible (unless something along the lines of this ticket is done.) Even though these first builds with explicit load commands are instructive for the end user, that might need to be skipped, with the text commentary jumping straight into what you get when you add -c with the main namespace.

Comment by Mike Fikes [ 17/Feb/18 9:13 PM ]

Confirmed that

clojure -m cljs.main -t none -o out/main.js -c foo.core
meets this tickets request with https://github.com/clojure/clojurescript/commit/a76db38b6c0c7e33547942b1a6586a1dc9b50523





[CLJS-2514] Specify that .cljsc_opts is an EDN map Created: 12/Feb/18  Updated: 17/Feb/18  Resolved: 17/Feb/18

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

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


 Description   

In java -jar cljs.jar -h help output, add a bit to specify that the .cljsc_opts file format is EDN and that its value is a map of compiler options.



 Comments   
Comment by David Nolen [ 17/Feb/18 7:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/633aaef52be892336c0b18264d57d012c4116483





[CLJS-2528] `clojure.string/split` with a limit can not correctly split a string by a regular expression that using Lookahead or Lookbehind Created: 16/Feb/18  Updated: 17/Feb/18

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

Type: Defect Priority: Minor
Reporter: Jinseop Kim Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

All


Attachments: Text File CLJS-2528_2.patch     Text File CLJS-2528_3.patch     Text File CLJS-2528_4.patch     Text File CLJS-2528_5.patch     Text File CLJS-2528.patch    

 Description   

clojure.string/split with a limit can not correctly split a string as following:

(clojure.string/split "quaqb" #"q(?!u)") ; <- match a 'q' not followed by a 'u'
;;=> ["qua" "b"]

(clojure.string/split "quaqb" #"q(?!u)" 2)
;;=> ["" "uaqb"]

the result of the first case is what we want, but the result of second case is wrong.
because, if there is a limit, it split a string using a own routine instead of a String.prototype.split function of JavaScript.
and the routine have a problem when using Lookaehad or Lookbehind in the regular expression.

;; clojure.string/split:
(let [re #"q(?!u)"]
  (loop [s "quaqb"
         limit 2
         parts []]
    (if (== 1 limit)
      (conj parts s)
      (let [m (re-find re s)] ; <- 1!
        (if-not (nil? m)
          (let [index (.indexOf s m)] ; <- 2!
            (recur (.substring s (+ index (count m)))
                   (dec limit)
                   (conj parts (.substring s 0 index))))
          (conj parts s))))))
;;=> ["" "uaqb"]

(re-find #"q(?!u)" "quaqb") ; <- 1!
;; => "q"

(.indexOf "quaqb" "q") ; <- 2!
;;=> 0

we should get a index from RegExp.prototype.exec function of JavaScript instead of calculating a index.

;; clojure.string/split:
(let [re #"q(?!u)"]
  (loop [s "quaqb"
         limit 2
         parts []]
    (if (== 1 limit)
      (conj parts s)
      (let [m (.exec re s)]
        (if-not (nil? m)
          (let [index (.-index m)]
            (recur (.substring s (+ index (count (aget m 0))))
                   (dec limit)
                   (conj parts (.substring s 0 index))))
          (conj parts s))))))
;;=> ["qua" "b"]

i tested on V8, Spidermonkey, Nashorn.



 Comments   
Comment by Jinseop Kim [ 16/Feb/18 1:56 AM ]

add a patch

Comment by Jinseop Kim [ 16/Feb/18 7:02 AM ]

i updated the patch to change tests

Comment by Jinseop Kim [ 16/Feb/18 8:30 AM ]

i updated the patch to fix for edge case

Comment by Jinseop Kim [ 16/Feb/18 12:36 PM ]

i update the patch to fix bugs and add tests
(i am sorry for updating the patch continuously, i thought it is easy)

Comment by Jinseop Kim [ 17/Feb/18 1:37 AM ]

update the patch





[CLJS-2527] Add iterator-seq to cljs.core Created: 15/Feb/18  Updated: 15/Feb/18

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-2527.patch    
Patch: Code and Test

 Description   

It would be great to add iterator-seq which comes standard in Clojure to ClojureScript. While it might be more useful overall in Clojure as a way of dealing with iterable things from Java-land, both Clojure and ClojureScript have there own collections which support iteration which makes it useful.






[CLJS-2526] Graceful termination for Node.js REPL Created: 15/Feb/18  Updated: 15/Feb/18

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

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


 Description   
java -jar target/cljs.jar -t node -re node -e '(js/setTimeout #(println "hello") 5000)'

terminates early






[CLJS-2524] Nashorn REPL fails if ssh'd into macOS Created: 15/Feb/18  Updated: 15/Feb/18  Resolved: 15/Feb/18

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

macOS
Java 9.0.4


Attachments: Text File CLJS-2524.patch    

 Description   

SSH to a Mac and attempt to run the Nashorn REPL. You will get an error evidently related to JavaFX.

java -jar target/cljs.jar
RenderJob.run: internal exception
java.lang.ArrayIndexOutOfBoundsException: 0
	at java.base/java.util.Arrays$ArrayList.get(Arrays.java:4350)
	at java.base/java.util.Collections$UnmodifiableList.get(Collections.java:1310)
	at javafx.graphics/com.sun.glass.ui.Screen.getMainScreen(Screen.java:61)
	at javafx.graphics/com.sun.prism.es2.ES2Pipeline.getScreenForAdapter(ES2Pipeline.java:147)
	at javafx.graphics/com.sun.prism.es2.ES2Pipeline.findDefaultResourceFactory(ES2Pipeline.java:158)
	at javafx.graphics/com.sun.prism.es2.ES2Pipeline.getDefaultResourceFactory(ES2Pipeline.java:179)
	at javafx.graphics/com.sun.prism.GraphicsPipeline.getDefaultResourceFactory(GraphicsPipeline.java:120)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer.lambda$createResourceFactory$2(QuantumRenderer.java:161)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:514)
	at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
	at javafx.graphics/com.sun.javafx.tk.RenderJob.run(RenderJob.java:58)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:125)
	at java.base/java.lang.Thread.run(Thread.java:844)


 Comments   
Comment by David Nolen [ 15/Feb/18 8:54 AM ]

What happens if you add (System/setProperty "java.awt.headless" "true") to cljs.cli/main?

Comment by Mike Fikes [ 15/Feb/18 8:59 AM ]

With that change, I get the same exception.

Comment by David Nolen [ 15/Feb/18 9:13 AM ]

What version of Java 8?

Comment by Mike Fikes [ 15/Feb/18 9:15 AM ]

It is actually Java 9.0.4, but I was also seeing the same with Java 8.

Comment by David Nolen [ 15/Feb/18 9:37 AM ]

I found out a different way to initialize JavaFX, try master.

Comment by Mike Fikes [ 15/Feb/18 9:50 AM ]

With master I get:

$ script/nashornrepljs
javax.script.ScriptException: TypeError: __PImpl.startup is not a function in <eval> at line number 22
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.throwAsScriptException(NashornScriptEngine.java:469)
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:453)
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:405)
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:401)
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.eval(NashornScriptEngine.java:154)
	at java.scripting/javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264)
	at cljs.repl.nashorn$eval_str.invokeStatic(nashorn.clj:46)
	at cljs.repl.nashorn$eval_str.invoke(nashorn.clj:45)
	at cljs.repl.nashorn$eval_resource.invokeStatic(nashorn.clj:52)
	at cljs.repl.nashorn$eval_resource.invoke(nashorn.clj:48)
	at cljs.repl.nashorn$init_engine.invokeStatic(nashorn.clj:60)
	at cljs.repl.nashorn$init_engine.invoke(nashorn.clj:55)
	at cljs.repl.nashorn.NashornEnv._setup(nashorn.clj:110)
	at cljs.repl$repl_STAR_$fn__8383.invoke(repl.cljc:869)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1274)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:866)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:783)
	at cljs.repl$repl.invokeStatic(repl.cljc:1031)
	at cljs.repl$repl.doInvoke(repl.cljc:961)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at user$eval9031.invokeStatic(NO_SOURCE_FILE:3)
	at user$eval9031.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:7005)
	at clojure.lang.Compiler.eval(Compiler.java:6968)
	at clojure.core$eval.invokeStatic(core.clj:3194)
	at clojure.main$eval_opt.invokeStatic(main.clj:290)
	at clojure.main$eval_opt.invoke(main.clj:284)
	at clojure.main$initialize.invokeStatic(main.clj:310)
	at clojure.main$null_opt.invokeStatic(main.clj:344)
	at clojure.main$null_opt.invoke(main.clj:341)
	at clojure.main$main.invokeStatic(main.clj:423)
	at clojure.main$main.doInvoke(main.clj:386)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: <eval>:22 TypeError: __PImpl.startup is not a function
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.error(ECMAErrors.java:57)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:213)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:185)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:172)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.Undefined.lookup(Undefined.java:100)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.linker.NashornLinker.getGuardedInvocation(NashornLinker.java:106)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.linker.NashornLinker.getGuardedInvocation(NashornLinker.java:96)
	at jdk.dynalink/jdk.dynalink.linker.support.CompositeTypeBasedGuardingDynamicLinker.getGuardedInvocation(CompositeTypeBasedGuardingDynamicLinker.java:184)
	at jdk.dynalink/jdk.dynalink.linker.support.CompositeGuardingDynamicLinker.getGuardedInvocation(CompositeGuardingDynamicLinker.java:132)
	at jdk.dynalink/jdk.dynalink.LinkerServicesImpl.lambda$getGuardedInvocation$0(LinkerServicesImpl.java:160)
	at jdk.dynalink/jdk.dynalink.LinkerServicesImpl.getWithLookupInternal(LinkerServicesImpl.java:191)
	at jdk.dynalink/jdk.dynalink.LinkerServicesImpl.getGuardedInvocation(LinkerServicesImpl.java:158)
	at jdk.dynalink/jdk.dynalink.DynamicLinker.relink(DynamicLinker.java:265)
	at jdk.scripting.nashorn.scripts/jdk.nashorn.internal.scripts.Script$Recompilation$343$\^eval\_$cu1$restOf.:program(<eval>:22)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ScriptFunctionData.invoke(ScriptFunctionData.java:652)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ScriptFunction.invoke(ScriptFunction.java:513)
	at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ScriptRuntime.apply(ScriptRuntime.java:517)
	at jdk.scripting.nashorn/jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:448)
	... 34 more
Exception in thread "main" clojure.lang.ExceptionInfo: ReferenceError: "cljs" is not defined {:type :js-eval-exception, :error {:status :exception, :value "ReferenceError: \"cljs\" is not defined", :stacktrace "\tat <program> (<eval>:1)"}, :repl-env #cljs.repl.nashorn.NashornEnv{:engine #object[jdk.nashorn.api.scripting.NashornScriptEngine 0x400b61cc "jdk.nashorn.api.scripting.NashornScriptEngine@400b61cc"], :debug nil}, :form (set! cljs.core/*print-namespace-maps* true), :js "cljs.core._STAR_print_namespace_maps_STAR_ = true"}
	at clojure.core$ex_info.invokeStatic(core.clj:4725)
	at clojure.core$ex_info.invoke(core.clj:4725)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:532)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:876)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:783)
	at cljs.repl$repl.invokeStatic(repl.cljc:1031)
	at cljs.repl$repl.doInvoke(repl.cljc:961)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at user$eval9031.invokeStatic(NO_SOURCE_FILE:3)
	at user$eval9031.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:7005)
	at clojure.lang.Compiler.eval(Compiler.java:6968)
	at clojure.core$eval.invokeStatic(core.clj:3194)
	at clojure.main$eval_opt.invokeStatic(main.clj:290)
	at clojure.main$eval_opt.invoke(main.clj:284)
	at clojure.main$initialize.invokeStatic(main.clj:310)
	at clojure.main$null_opt.invokeStatic(main.clj:344)
	at clojure.main$null_opt.invoke(main.clj:341)
	at clojure.main$main.invokeStatic(main.clj:423)
	at clojure.main$main.doInvoke(main.clj:386)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Comment by Mike Fikes [ 15/Feb/18 10:09 AM ]

If you first add a utility function to get the current thread ID

(defn current-thread-id [] (.getId (.currentThread (js/Java.type "java.lang.Thread")))

You can see that when running a REPL, it returns 1, but if you use js/setTimeout with the JavaFX implementation, you see that the callbacks occur on a different, but yet consistent thread (for me it has always been thread 15). Alternatively, if you remove the JavaFX run-later, can just callback, you will see that the use of Timer causes the callbacks to occur on different threads, and this probably introduces race conditions, a lack of ordering, or even perhaps concurrent execution (I'm speculating somewhat).

But, without changing the overall behavior, it is possible to use a single threaded scheduled executor service to essentially implement an event loop. With that change, you will see that js/setTimeout also consistently happens on the same executor service thread (which for me also happens to alway be 15).

By avoiding JFX and just using stuff in java.util.concurrent this issue appears to be fixed when ssh'd into macOS.

Comment by David Nolen [ 15/Feb/18 10:14 AM ]

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





[CLJS-2520] Synthesize ClojureScript version if using non-built ClojureScript dep Created: 14/Feb/18  Updated: 15/Feb/18  Resolved: 15/Feb/18

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

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

Attachments: Text File CLJS-2520.patch    
Patch: Code
Approval: Accepted

 Description   

It is possible to use a non-built ClojureScript dep. (This can be done, for example, by putting :local/root for the ClojureScript tree on the classpath, or by using ClojureScript as a git dep via deps.edn).

In this mode, everything works fine, with the only consequence being that the ClojureScript version is not defined. (Part of the script/build is to define a version based on git.) Apart from *clourescript-vesion* being nil in REPLs (which is not overly problematic), compiled artifacts are cached with the default version number of 0.0.0000. This means that if the unbuilt ClojureScript dep changes (a new git dep is used, or, changes were made in a :local/root tree), the old 0.0.0000-based cached files are still used.

It is possible to avoid this, by simply synthesizing a ClojureScript version in this case, where the version is based on the hash of the contents of the main source files. This synthetic version can be calculated in 10s of milliseconds, is deterministic, and can be "cached" within the process during compilation. This results in a nice solution where you can use ClojureScript as a git dep, while also benefitting from proper cache behavior.



 Comments   
Comment by David Nolen [ 15/Feb/18 9:24 AM ]

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





[CLJS-2522] Handle map sources in build-modules Created: 14/Feb/18  Updated: 15/Feb/18  Resolved: 15/Feb/18

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

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

Attachments: Text File cljs-2522-handle-map-sources.patch    
Approval: Accepted

 Description   

Input sources to build-modules could also be maps instead of JavaScriptFile records.

Those were not handled, which caused an exception here:

https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L1192

because javascript-name is only defined on the record, not a map.

There's a desire to remove the JavaScriptFile record in the future.



 Comments   
Comment by Roman Scherer [ 15/Feb/18 3:38 AM ]

David, I added the instance? check for JavaScriptFile

Comment by David Nolen [ 15/Feb/18 9:11 AM ]

fixed https://github.com/clojure/clojurescript/commit/155c2162f87d1402d5f83b750833bd1d4c010eaa





[CLJS-2521] Only expand module graph when modules are actually used Created: 14/Feb/18  Updated: 14/Feb/18  Resolved: 14/Feb/18

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

Type: Defect Priority: Major
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-2521-only-expand-modules-when-used.patch    

 Comments   
Comment by David Nolen [ 14/Feb/18 9:31 AM ]

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





[CLJS-2519] Module loader doesn't load :cljs-base properly under :none optimizations Created: 13/Feb/18  Updated: 14/Feb/18  Resolved: 14/Feb/18

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

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

Attachments: Text File cljs-2519-code-split-no-orphans.patch     Text File cljs-2519-code-split.patch    

 Description   

When using namespaces that are shared accross multiple modules, the module loader doesn't load all namespaces that have been assigned to :cljs-base properly. This leads to runtime errors like the following:

nexttick.js:41 Uncaught TypeError: Cannot read property 'hello' of undefined
    at code$split$b$hello (eval at goog.globalEval (base.js:2184), <anonymous>:9:21)
    at cljs.core.Var.G__9784__1 (core.cljs:1123)
    at cljs.core.Var.G__9784 [as call] (core.cljs:1164)
    at a.cljs:23
    at goog.module.ModuleLoadCallback.execute (moduleloadcallback.js:60)
    at goog.module.ModuleInfo.callCallbacks_ (moduleinfo.js:324)
    at goog.module.ModuleInfo.onLoad (moduleinfo.js:271)
    at goog.module.ModuleManager.setLoaded (modulemanager.js:831)
    at cljs$loader$set_loaded_BANG_ (loader.cljs:80)
    at eval (eval at goog.globalEval (base.js:2184), <anonymous>:11:30)

I added a sample project based on the Code Splitting guide that demonstrates this issue. The sample project can be found in the src/test/cljs_build/code-split directory.

The project can be tested with the following commands:

./script/uberjar
cd src/test/cljs_build/code-split
java -cp ../../../../target/cljs.jar:src clojure.main repl.clj
chromium http://localhost:9000/index.html

The project builds 3 modules:

  • code.split.a (loaded on start from index.html)
  • code.split.b (loaded on button press)
  • code.split.c (loaded on button press)

The namespaces code.split.b and code.split.c require code from the shared namespace code.split.d. Since namespace code.split.d is used by 2 modules it is assigned to the implicit :cljs-base module.

When the user presses a button, the module manager should load the dependencies of the selected module and execute code from the loaded module. However, this fails at the moment with the runtime exception mentioned above.

The markup used to loade the code is:

<html>
  <body>
    <h2>Module A</h2>
    <button id="button-b">Load Module B!</button>
    <button id="button-c">Load Module C!</button>
    <script src="/out/cljs_base.js" type="text/javascript"></script>
    <script src="/out/a.js" type="text/javascript"></script>
  </body>
</html>


 Comments   
Comment by Roman Scherer [ 14/Feb/18 4:36 AM ]

David, the `output-unoptimized` function is called with the compiler
options map that doesn't contain the expanded module graph.

The patch above populates this map in `output-unoptimized` with the
:entries and now the cljs_base.js looks like this:

  • cljs_base.js
var CLOSURE_UNCOMPILED_DEFINES = {};
var CLOSURE_NO_DEPS = true;
if(typeof goog == "undefined") document.write('<script src="/out/goog/base.js"></script>');
document.write('<script src="/out/goog/deps.js"></script>');
document.write('<script src="/out/cljs_deps.js"></script>');
document.write('<script>if (typeof goog == "undefined") console.warn("ClojureScript could not load :main, did you forget to specify :asset-path?");</script>');
document.write('<script>goog.require("process.env");</script>');
document.write('<script>goog.require("clojure.browser.repl.preload");</script>');
document.write('<script>goog.require("goog.debug.Error");</script>');
document.write('<script>goog.require("goog.dom.NodeType");</script>');
document.write('<script>goog.require("goog.string");</script>');
document.write('<script>goog.require("goog.asserts");</script>');
document.write('<script>goog.require("goog.array");</script>');
document.write('<script>goog.require("goog.labs.userAgent.util");</script>');
document.write('<script>goog.require("goog.object");</script>');
document.write('<script>goog.require("goog.labs.userAgent.browser");</script>');
document.write('<script>goog.require("goog.labs.userAgent.engine");</script>');
document.write('<script>goog.require("goog.labs.userAgent.platform");</script>');
document.write('<script>goog.require("goog.reflect");</script>');
document.write('<script>goog.require("goog.userAgent");</script>');
document.write('<script>goog.require("goog.dom.BrowserFeature");</script>');
document.write('<script>goog.require("goog.dom.HtmlElement");</script>');
document.write('<script>goog.require("goog.dom.TagName");</script>');
document.write('<script>goog.require("goog.dom.asserts");</script>');
document.write('<script>goog.require("goog.dom.tags");</script>');
document.write('<script>goog.require("goog.string.TypedString");</script>');
document.write('<script>goog.require("goog.string.Const");</script>');
document.write('<script>goog.require("goog.html.SafeScript");</script>');
document.write('<script>goog.require("goog.fs.url");</script>');
document.write('<script>goog.require("goog.i18n.bidi");</script>');
document.write('<script>goog.require("goog.html.TrustedResourceUrl");</script>');
document.write('<script>goog.require("goog.html.SafeUrl");</script>');
document.write('<script>goog.require("goog.html.SafeStyle");</script>');
document.write('<script>goog.require("goog.html.SafeStyleSheet");</script>');
document.write('<script>goog.require("goog.html.SafeHtml");</script>');
document.write('<script>goog.require("goog.dom.safe");</script>');
document.write('<script>goog.require("goog.html.uncheckedconversions");</script>');
document.write('<script>goog.require("goog.math");</script>');
document.write('<script>goog.require("goog.math.Coordinate");</script>');
document.write('<script>goog.require("goog.math.Size");</script>');
document.write('<script>goog.require("goog.dom");</script>');
document.write('<script>goog.require("goog.structs");</script>');
document.write('<script>goog.require("goog.functions");</script>');
document.write('<script>goog.require("goog.iter");</script>');
document.write('<script>goog.require("goog.structs.Map");</script>');
document.write('<script>goog.require("goog.uri.utils");</script>');
document.write('<script>goog.require("goog.Uri");</script>');
document.write('<script>goog.require("goog.math.Integer");</script>');
document.write('<script>goog.require("goog.string.StringBuffer");</script>');
document.write('<script>goog.require("goog.math.Long");</script>');
document.write('<script>goog.require("cljs.core");</script>');
document.write('<script>goog.require("goog.userAgent.product");</script>');
document.write('<script>goog.require("goog.debug.errorcontext");</script>');
document.write('<script>goog.require("goog.debug");</script>');
document.write('<script>goog.require("goog.debug.LogRecord");</script>');
document.write('<script>goog.require("goog.debug.LogBuffer");</script>');
document.write('<script>goog.require("goog.debug.LogManager");</script>');
document.write('<script>goog.require("goog.log");</script>');
document.write('<script>goog.require("goog.net.xpc");</script>');
document.write('<script>goog.require("goog.Thenable");</script>');
document.write('<script>goog.require("goog.async.FreeList");</script>');
document.write('<script>goog.require("goog.async.WorkItem");</script>');
document.write('<script>goog.require("goog.debug.EntryPointMonitor");</script>');
document.write('<script>goog.require("goog.async.nextTick");</script>');
document.write('<script>goog.require("goog.async.run");</script>');
document.write('<script>goog.require("goog.promise.Resolver");</script>');
document.write('<script>goog.require("goog.Promise");</script>');
document.write('<script>goog.require("goog.disposable.IDisposable");</script>');
document.write('<script>goog.require("goog.Disposable");</script>');
document.write('<script>goog.require("goog.events.BrowserFeature");</script>');
document.write('<script>goog.require("goog.events.EventId");</script>');
document.write('<script>goog.require("goog.events.Event");</script>');
document.write('<script>goog.require("goog.events.EventType");</script>');
document.write('<script>goog.require("goog.events.BrowserEvent");</script>');
document.write('<script>goog.require("goog.events.Listenable");</script>');
document.write('<script>goog.require("goog.events.Listener");</script>');
document.write('<script>goog.require("goog.events.ListenerMap");</script>');
document.write('<script>goog.require("goog.events");</script>');
document.write('<script>goog.require("goog.events.EventTarget");</script>');
document.write('<script>goog.require("goog.Timer");</script>');
document.write('<script>goog.require("goog.json");</script>');
document.write('<script>goog.require("goog.json.hybrid");</script>');
document.write('<script>goog.require("goog.net.ErrorCode");</script>');
document.write('<script>goog.require("goog.net.EventType");</script>');
document.write('<script>goog.require("goog.net.HttpStatus");</script>');
document.write('<script>goog.require("goog.net.XhrLike");</script>');
document.write('<script>goog.require("goog.net.XmlHttpFactory");</script>');
document.write('<script>goog.require("goog.net.WrapperXmlHttpFactory");</script>');
document.write('<script>goog.require("goog.net.DefaultXmlHttpFactory");</script>');
document.write('<script>goog.require("goog.net.XhrIo");</script>');
document.write('<script>goog.require("goog.async.Deferred");</script>');
document.write('<script>goog.require("goog.Delay");</script>');
document.write('<script>goog.require("goog.events.EventHandler");</script>');
document.write('<script>goog.require("goog.messaging.MessageChannel");</script>');
document.write('<script>goog.require("goog.messaging.AbstractChannel");</script>');
document.write('<script>goog.require("goog.net.xpc.CrossPageChannelRole");</script>');
document.write('<script>goog.require("goog.net.xpc.Transport");</script>');
document.write('<script>goog.require("goog.net.xpc.DirectTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.FrameElementMethodTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.IframePollingTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.IframeRelayTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.NativeMessagingTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.NixTransport");</script>');
document.write('<script>goog.require("goog.net.xpc.CrossPageChannel");</script>');
document.write('<script>goog.require("goog.net.WebSocket");</script>');
document.write('<script>goog.require("clojure.browser.event");</script>');
document.write('<script>goog.require("clojure.browser.net");</script>');
document.write('<script>goog.require("clojure.walk");</script>');
document.write('<script>goog.require("cljs.spec.gen.alpha");</script>');
document.write('<script>goog.require("cljs.spec.alpha");</script>');
document.write('<script>goog.require("cljs.repl");</script>');
document.write('<script>goog.require("clojure.browser.repl");</script>');
document.write('<script>goog.require("clojure.browser.repl.preload");</script>');
document.write('<script>goog.require("goog.module");</script>');
document.write('<script>goog.require("goog.html.legacyconversions");</script>');
document.write('<script>goog.require("goog.module.BaseModule");</script>');
document.write('<script>goog.require("goog.module.ModuleLoadCallback");</script>');
document.write('<script>goog.require("goog.module.ModuleInfo");</script>');
document.write('<script>goog.require("goog.module.AbstractModuleLoader");</script>');
document.write('<script>goog.require("goog.net.BulkLoaderHelper");</script>');
document.write('<script>goog.require("goog.net.BulkLoader");</script>');
document.write('<script>goog.require("goog.net.jsloader");</script>');
document.write('<script>goog.require("goog.module.ModuleLoader");</script>');
document.write('<script>goog.require("process.env");</script>');
document.write('<script>goog.require("goog.debug.Formatter");</script>');
document.write('<script>goog.require("goog.structs.SimplePool");</script>');
document.write('<script>goog.require("goog.debug.Trace");</script>');
document.write('<script>goog.require("goog.module.ModuleManager");</script>');
document.write('<script>goog.require("cljs.loader");</script>');
document.write('<script>goog.require("code.split.d");</script>');
  • a.js
document.write('<script>goog.require("clojure.string");</script>');
document.write('<script>goog.require("goog.debug.RelativeTimeProvider");</script>');
document.write('<script>goog.require("goog.debug.Console");</script>');
document.write('<script>goog.require("cljs.pprint");</script>');
document.write('<script>goog.require("code.split.a");</script>');
  • b.js
document.write('<script>goog.require("code.split.b");</script>');
  • c.js
document.write('<script>goog.require("code.split.c");</script>');

The shared namespace code.split.d is now required in cljs_base.js.

I tested this patch on one of my projects and it seems to work fine.

Is there a better place to expand the module graph, and pass the
expanded one around?

Comment by Roman Scherer [ 14/Feb/18 5:08 AM ]

Hmm, wait. Now there's ending up too much in cljs_base.js. Even files that are not required, but on the classpath.

Comment by David Nolen [ 14/Feb/18 7:21 AM ]

Hrm that's interesting because I thought we were doing exactly what the patch does but I guess I only half finished it. So when you were running through the guide you saw the same thing? A problematic base? But I guess the dep order is such that you don't encounter your specific issue.

I'm pretty sure you need to leave the orphans change out of your patch.

Comment by David Nolen [ 14/Feb/18 7:43 AM ]

fixed https://github.com/clojure/clojurescript/commit/78b2395960767ea44b68ddd632d1dfe9a4957853





[CLJS-2493] Self host: respect :source-map-timestamp compiler option Created: 07/Feb/18  Updated: 13/Feb/18  Resolved: 13/Feb/18

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

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

Attachments: Text File CLJS-2493-0.patch     Text File CLJS-2493-1.patch    

 Description   

At the moment we generate inline source maps with file names resembling:

"foo/user_with_underscores.js?rel=1518049631768"

The ?rel= pattern though is not recognize and trimmed away in any step preceding cljs.stacktrace/remove-ext, which therefore does not replace correctly.

I am not sure what approach to take, but we could avoid generating the ?rel= in node given that there is no need for cache busting. Maybe there is a better way and the ?rel= is actually trimmed somewhere, but at the moment I am doing it manually in lumo otherwise the source maps are not working.



 Comments   
Comment by Andrea Richiardi [ 08/Feb/18 1:41 PM ]

Just found out about the :source-map-timestamp option and will prepare a patch for supporting it in js.cljs.

Comment by Andrea Richiardi [ 08/Feb/18 2:11 PM ]

Ok done, patch attached.

Comment by Andrea Richiardi [ 08/Feb/18 9:48 PM ]

Sorry the first was obviously wrong.

Comment by David Nolen [ 13/Feb/18 6:35 PM ]

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





[CLJS-2518] Command line fails to terminate on some OSs Created: 13/Feb/18  Updated: 13/Feb/18  Resolved: 13/Feb/18

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

Windows 10 / Java 1.8.0_66
Ubuntu Linux 14.04 Desktop / Java 1.8.0_161


Attachments: Text File CLJS-2518.patch    

 Description   

java -jar cljs.jar -e 3 will print 3 but not terminate (you need to hit Ctrl-C to stop the process). Likewise a similar thing will occur if you launch a REPL and type :cljs/quit—it will just hang. This is not the case if you specify -js rhino or -js node.



 Comments   
Comment by Mike Fikes [ 13/Feb/18 8:53 AM ]

Confirmed that this behavior is unrelated to https://github.com/clojure/clojurescript/commit/cc042e8a8f731a60ff36cb52178c0ba88eeb2eea

Comment by Mike Fikes [ 13/Feb/18 10:18 AM ]

Hey David, the attached patch addresses the issue (on both Windows 10 and Ubuntu Linux 14.04 Desktop) by explicitly shutting down JavaFX at REPL tear down time.

A more difficult problem to solve is the case when you are running, say the core.async tests under jjs---those will also pass but block on shudtdown. The patch introduces a nashhorn_tear_down JavaScript function that the REPL tear down is calling, but for the jjs case, we'd have to figure out a more generic way to hook in and do teardown upon script completion.

Comment by David Nolen [ 13/Feb/18 10:28 AM ]

fixed https://github.com/clojure/clojurescript/commit/908f5f301f2e7ddfa6fa016c9aad42d465ab2e1c

Comment by Mike Fikes [ 13/Feb/18 10:34 AM ]

By the way, I had also tried __Platform.setImplicitExit(true) but that is insufficient. (Perhaps this is only triggered by a user closing the last JavaFX-based window?)





[CLJS-2517] Avoid window popup running Nashorn Created: 12/Feb/18  Updated: 13/Feb/18  Resolved: 13/Feb/18

Status: Resolved
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-2517.patch     PNG File Napkin 02-12-18, 8.53.13 PM.png    

 Description   

When you run the Nashorn REPL a little window pops up in your macOS doc related to JavaFX. (It is especially annoying because it steals your keyboard focus if you are launching a REPL.)



 Comments   
Comment by Mike Fikes [ 12/Feb/18 7:50 PM ]

The js/setTimeout and js/setInterval functions appear to work fine without dispatching onto a JFX event queue. If we don't really need JFX in the mix, perhaps we can ditch the bit of code that is in there, and just delegate directly to the apply handler as is done in the patch.

The Jim Laskey post (https://blogs.oracle.com/nashorn/setinterval-and-settimeout-javascript-functions) appears to be related to the situation involving porting some HTML to Nashorn + JavaFX, but we probably end up using Nashorn just to run JavaScript is a non-graphical (certainly non-JFX) environment.

If we can get away with this, this would avoid the little popup that appears.

If not, perhaps we could try a little less aggressive patch that delays the JFX initialization until the time the first js/setTimeout or js/setInterval call is made (thus keeping the little popup but only lazily if actually needed).

Comment by David Nolen [ 12/Feb/18 8:48 PM ]

This patch unfortunately does not appear to work. With this applied core.async tests under Nashorn exit immediately. I made a branch on core.async called cljs-nashorn to make this easier to test.

Comment by Mike Fikes [ 13/Feb/18 8:58 AM ]

This solves it: https://github.com/clojure/clojurescript/commit/cc042e8a8f731a60ff36cb52178c0ba88eeb2eea with the icon also not appearing on Windows 10 and Ubuntu Linux 14.04 Desktop.





[CLJS-2511] Failure if -js specified twice Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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


 Description   

Init options are supposed to be repeatable. For example, here is repeating -v with evidently the last one winning.

$ java -jar cljs.jar -v true -e '(inc 1)' -v false
2

But, -js fails if repeated:

$ java -jar cljs.jar -js node -e '(inc 1)' -js node
ClojureScript Node.js REPL server listening on 52494
2
Exception in thread "main" java.lang.IllegalArgumentException: Cannot open <nil> as a Reader.
	at clojure.java.io$fn__10992.invokeStatic(io.clj:288)
	at clojure.java.io$fn__10992.invoke(io.clj:288)
	at clojure.java.io$fn__10894$G__10870__10901.invoke(io.clj:69)
	at clojure.java.io$reader.invokeStatic(io.clj:102)
	at clojure.java.io$reader.doInvoke(io.clj:86)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.analyzer$parse_ns$fn__2827.invoke(analyzer.cljc:3762)
	at cljs.analyzer$parse_ns.invokeStatic(analyzer.cljc:3745)
	at cljs.analyzer$parse_ns.invoke(analyzer.cljc:3724)
	at cljs.analyzer$parse_ns.invokeStatic(analyzer.cljc:3735)
	at cljs.closure$src_file__GT_target_file.invokeStatic(closure.clj:2964)
	at cljs.closure$src_file__GT_target_file.invoke(closure.clj:2957)
	at cljs.closure$src_file__GT_target_file.invokeStatic(closure.clj:2959)
	at cljs.repl$load_file$fn__6908.invoke(repl.cljc:555)
	at cljs.repl$load_file.invokeStatic(repl.cljc:555)
	at cljs.repl$load_file.invoke(repl.cljc:547)
	at cljs.repl$load_file.invokeStatic(repl.cljc:548)
	at cljs.cli$main_opt_STAR_$fn__7194.invoke(cli.clj:112)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:98)
	at cljs.cli$script_opt.invokeStatic(cli.clj:141)
	at cljs.cli$script_opt.invoke(cli.clj:141)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:204)
	at cljs.cli$main.doInvoke(cli.clj:162)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)


 Comments   
Comment by David Nolen [ 12/Feb/18 8:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/87646c7c73a2d69979f2dabfbe5c3069dc6881f0





[CLJS-2512] Allow -v / --verbose to take no args, and imply true Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

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


 Description   

When passing -v you need to supply a Boolean argument, but it is expected that users may fail to realize this, expecting that -v takes no arguments. This enhancement ticket asks for that use case to be supported.



 Comments   
Comment by David Nolen [ 12/Feb/18 7:43 PM ]

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





[CLJS-2508] No command line args should be nil instead of empty list Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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   

Expected behavior:

$ clj
Clojure 1.9.0
user=> *command-line-args*
nil

Actual behavior:

$ java -jar cljs.jar
To quit, type: :cljs/quit
cljs.user=> *command-line-args*
()

Docstring support:

cljs.user=> (doc *command-line-args*)
-------------------------
cljs.core/*command-line-args*
  A sequence of the supplied command line arguments, or nil if
  none were supplied


 Comments   
Comment by David Nolen [ 12/Feb/18 4:10 PM ]

fixed https://github.com/clojure/clojurescript/commit/82998109292c5982380e945e7565d7425879e0f3





[CLJS-2513] Extra out directory in out Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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   

First java -jar cljs.jar and then quit the REPL.

Then

$ ls out
cljs	clojure	goog	out


 Comments   
Comment by David Nolen [ 12/Feb/18 4:06 PM ]

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





[CLJS-2506] Can't require in REPL eval Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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   

Expected behavior:

$ clj -e "(require '[clojure.string :as s])" -e '(s/trimr "abc ")'
"abc"

Actual behavior:

$ java -jar cljs.jar -e "(require '[clojure.string :as s])" -e '(s/trimr "abc ")'
Exception in thread "main" clojure.lang.ExceptionInfo: Can't change/establish root binding of: *cljs-ns* with set at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (require (quote [clojure.string :as s]))}, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:693)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3352)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3376)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3545)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3603)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3378)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3545)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3593)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3619)
	at cljs.repl$evaluate_form$__GT_ast__6885.invoke(repl.cljc:476)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:477)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:464)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:462)
	at cljs.repl$run_inits.invokeStatic(repl.cljc:772)
	at cljs.cli$main_opt_STAR_$fn__7194.invoke(cli.clj:107)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:98)
	at cljs.cli$null_opt.invokeStatic(cli.clj:130)
	at cljs.cli$null_opt.invoke(cli.clj:130)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:204)
	at cljs.cli$main.doInvoke(cli.clj:162)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)
Caused by: java.lang.IllegalStateException: Can't change/establish root binding of: *cljs-ns* with set
	at clojure.lang.Var.set(Var.java:223)
	at cljs.analyzer$fn__2428.invokeStatic(analyzer.cljc:2698)
	at cljs.analyzer$fn__2428.invoke(analyzer.cljc:2688)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3348)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3351)
	... 30 more


 Comments   
Comment by David Nolen [ 12/Feb/18 3:10 PM ]

resolved by fixing CLJS-2505 & this commit https://github.com/clojure/clojurescript/commit/ff693219b8a58d6b9f6c0e85578b73cbdd5c92d9





[CLJS-2507] Can't def in REPL eval Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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   

Expected behavior:

$ clj -e "(def x 2)" -e '(inc x)'
#'user/x
3
$ planck -e "(def x 2)" -e '(inc x)'
#'cljs.user/x
3

Actual behavior:

$ java -jar cljs.jar -e "(def x 2)" -e '(inc x)'
Exception in thread "main" clojure.lang.ExceptionInfo: TypeError: Cannot set property "x" of undefined {:type :js-eval-exception, :error {:status :exception, :value "TypeError: Cannot set property \"x\" of undefined", :stacktrace "\tat <program> (<eval>:1)"}, :repl-env #cljs.repl.nashorn.NashornEnv{:engine #object[jdk.nashorn.api.scripting.NashornScriptEngine 0x329b8371 "jdk.nashorn.api.scripting.NashornScriptEngine@329b8371"], :debug nil}, :form (def x 2), :js "cljs.user.x = (2);\n"}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:532)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:464)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:462)
	at cljs.repl$run_inits.invokeStatic(repl.cljc:772)
	at cljs.cli$main_opt_STAR_$fn__7194.invoke(cli.clj:107)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:98)
	at cljs.cli$null_opt.invokeStatic(cli.clj:130)
	at cljs.cli$null_opt.invoke(cli.clj:130)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:204)
	at cljs.cli$main.doInvoke(cli.clj:162)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)


 Comments   
Comment by David Nolen [ 12/Feb/18 3:09 PM ]

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





[CLJS-2504] Extra blank line for nil REPL evals Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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


 Description   

Expected behavior:

$ clj -e "(+ 3 2)" -e "(identity nil)" -e "(+ 7 2)"
5
9

but observe the extra blank line here:

$ java -jar cljs.jar -e "(+ 3 2)" -e "(identity nil)" -e "(+ 7 2)"
5

9


 Comments   
Comment by Mike Fikes [ 12/Feb/18 2:45 PM ]

Note that, while this issue is no longer reproducible as written owing to CLJS-2505, the different behavior is that instead of a blank line, the returned nil is actually printed:

$ java -jar cljs.jar -e "(+ 3 2)" -e "(identity nil)" -e "(+ 7 2)"
5
nil
9
Comment by David Nolen [ 12/Feb/18 2:47 PM ]

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





[CLJS-2503] REPL eval simple value fails to print Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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: Duplicate Votes: 0
Labels: None


 Description   

-e with an expression prints the evaluated value

$ java -cp cljs.jar:src cljs.main -e "(inc 2)"
3

but not if the expression is "simple":

$ java -cp cljs.jar:src cljs.main -e 3


 Comments   
Comment by Mike Fikes [ 12/Feb/18 2:16 PM ]

Somehow the patch in CLJS-2505 solves this issue.

Comment by David Nolen [ 12/Feb/18 2:30 PM ]

fixed by CLJS-2505 https://github.com/clojure/clojurescript/commit/5b012c8cbf4d4061dfb8854b48190e30b67c243e





[CLJS-2505] cljs.main REPL -e evals don't print correctly Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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-2505.patch    

 Description   

Expected behavior:

$ clj -e '(identity "foo")'
"foo"

Observed behavior:

$ java -jar cljs.jar -e '(identity "foo")'
foo


 Comments   
Comment by David Nolen [ 12/Feb/18 12:45 PM ]

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

Comment by David Nolen [ 12/Feb/18 12:57 PM ]

We can't use prn because REPLs return strings. I'm reverting and fixing the Nashorn REPL.

Comment by Mike Fikes [ 12/Feb/18 2:07 PM ]

David, do you think that the use of pr-str might be a way to effectively achieve the same, as in the attached patch?

Comment by Mike Fikes [ 12/Feb/18 2:16 PM ]

Interestingly, the pr-str patch somehow addresses CLJS-2503 (not clear to me why yet). It also solves CLJS-2504 because of the when guard for "nil".

Comment by David Nolen [ 12/Feb/18 2:28 PM ]

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





[CLJS-2502] lein test failing with :nashorn target update Created: 11/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

With https://github.com/clojure/clojurescript/commit/d3b9fc0dacd64e26842feb882ad7dba43bcc8e7a made for CLJS-1076

lein test fails with

ERROR in (test-emit-node-requires-cljs-2213) (io.clj:288)
Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.IllegalArgumentException: Cannot open <nil> as a Reader.
 at clojure.java.io$fn__10992.invokeStatic (io.clj:288)
    clojure.java.io/fn (io.clj:288)
    clojure.java.io$fn__10894$G__10870__10901.invoke (io.clj:69)
    clojure.java.io$reader.invokeStatic (io.clj:102)
    clojure.java.io$reader.doInvoke (io.clj:86)
    clojure.lang.RestFn.invoke (RestFn.java:410)
    clojure.lang.AFn.applyToHelper (AFn.java:154)
    clojure.lang.RestFn.applyTo (RestFn.java:132)
    clojure.core$apply.invokeStatic (core.clj:659)
    clojure.core$slurp.invokeStatic (core.clj:6862)
    clojure.core$slurp.doInvoke (core.clj:6862)
    clojure.lang.RestFn.invoke (RestFn.java:410)
    cljs.closure$output_bootstrap.invokeStatic (closure.clj:2654)
    cljs.closure$build.invokeStatic (closure.clj:2789)
    cljs.build.api$build.invokeStatic (api.clj:204)
    cljs.build_api_tests$fn__11889$fn__11890$fn__11892.invoke (build_api_tests.clj:342)
    cljs.build_api_tests$fn__11889$fn__11890.invoke (build_api_tests.clj:342)
    cljs.build_api_tests$fn__11889.invokeStatic (build_api_tests.clj:327)
    cljs.build_api_tests/fn (build_api_tests.clj:324)
    clojure.test$test_var$fn__9209.invoke (test.clj:716)
    clojure.test$test_var.invokeStatic (test.clj:716)
    clojure.test$test_var.invoke (test.clj:707)
    clojure.test$test_vars$fn__9235$fn__9240.invoke (test.clj:734)
    clojure.test$default_fixture.invokeStatic (test.clj:686)
    clojure.test$default_fixture.invoke (test.clj:682)
    clojure.test$test_vars$fn__9235.invoke (test.clj:734)
    clojure.test$default_fixture.invokeStatic (test.clj:686)
    clojure.test$default_fixture.invoke (test.clj:682)
    clojure.test$test_vars.invokeStatic (test.clj:730)
    clojure.test$test_all_vars.invokeStatic (test.clj:736)
    clojure.test$test_ns.invokeStatic (test.clj:757)
    clojure.test$test_ns.invoke (test.clj:742)
    user$eval233$fn__294.invoke (form-init4614255697286350997.clj:1)
    clojure.lang.AFn.applyToHelper (AFn.java:156)
    clojure.lang.AFn.applyTo (AFn.java:144)
    clojure.core$apply.invokeStatic (core.clj:659)
    leiningen.core.injected$compose_hooks$fn__163.doInvoke (form-init4614255697286350997.clj:1)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invokeStatic (core.clj:657)
    leiningen.core.injected$run_hooks.invokeStatic (form-init4614255697286350997.clj:1)
    leiningen.core.injected$prepare_for_hooks$fn__168$fn__169.doInvoke (form-init4614255697286350997.clj:1)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.lang.AFunction$1.doInvoke (AFunction.java:29)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    clojure.core$map$fn__5587.invoke (core.clj:2747)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.next (RT.java:706)
    clojure.core$next__5108.invokeStatic (core.clj:64)
    clojure.core$reduce1.invokeStatic (core.clj:936)
    clojure.core$reduce1.invokeStatic (core.clj:926)
    clojure.core$merge_with.invokeStatic (core.clj:3051)
    clojure.core$merge_with.doInvoke (core.clj:3043)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invokeStatic (core.clj:659)
    clojure.test$run_tests.invokeStatic (test.clj:767)
    clojure.test$run_tests.doInvoke (test.clj:767)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.core$apply.invokeStatic (core.clj:657)
    user$eval233$fn__306$fn__339.invoke (form-init4614255697286350997.clj:1)
    user$eval233$fn__306$fn__307.invoke (form-init4614255697286350997.clj:1)
    user$eval233$fn__306.invoke (form-init4614255697286350997.clj:1)
    user$eval233.invokeStatic (form-init4614255697286350997.clj:1)
    user$eval233.invoke (form-init4614255697286350997.clj:1)
    clojure.lang.Compiler.eval (Compiler.java:7062)
    clojure.lang.Compiler.eval (Compiler.java:7052)
    clojure.lang.Compiler.load (Compiler.java:7514)
    clojure.lang.Compiler.loadFile (Compiler.java:7452)
    clojure.main$load_script.invokeStatic (main.clj:278)
    clojure.main$init_opt.invokeStatic (main.clj:280)
    clojure.main$init_opt.invoke (main.clj:280)
    clojure.main$initialize.invokeStatic (main.clj:311)
    clojure.main$null_opt.invokeStatic (main.clj:345)
    clojure.main$null_opt.invoke (main.clj:342)
    clojure.main$main.invokeStatic (main.clj:424)
    clojure.main$main.doInvoke (main.clj:387)
    clojure.lang.RestFn.applyTo (RestFn.java:137)
    clojure.lang.Var.applyTo (Var.java:702)
    clojure.main.main (main.java:37)


 Comments   
Comment by Mike Fikes [ 12/Feb/18 1:51 PM ]

No longer reproducible on master.





[CLJS-2510] Misspelling of ClojureScript in help output Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

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


 Description   
-v,  --verbose bool      if true, will enable ClojureScriptt verbose logging


 Comments   
Comment by David Nolen [ 12/Feb/18 12:44 PM ]

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





[CLJS-2515] NPE running Node REPL Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

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


 Description   

With uberjar built from master:

$ java -jar cljs.jar -m cljs.repl.node
Exception in thread "main" java.lang.NullPointerException
	at cljs.repl$load_file.invokeStatic(repl.cljc:553)
	at cljs.repl$load_file.invoke(repl.cljc:547)
	at cljs.repl$load_file.invokeStatic(repl.cljc:548)
	at cljs.cli$main_opt_STAR_$fn__7194.invoke(cli.clj:115)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:98)
	at cljs.cli$main_opt.invokeStatic(cli.clj:121)
	at cljs.cli$main_opt.invoke(cli.clj:121)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:204)
	at cljs.cli$main.doInvoke(cli.clj:162)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)


 Comments   
Comment by David Nolen [ 12/Feb/18 11:33 AM ]

This isn't a bug. -m is only for ClojureScript stuff in this case

java -cp cljs.jar clojure.main -m cljs.repl.node

Is what you want, nothing changed here.





[CLJS-2516] Build API fails targeting Node (QuickStart) Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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   

With uberjar built from master, try building targeting Node per QuickStart, in particular the steps here https://clojurescript.org/guides/quick-start#running-clojurescript-on-node.js

You will get a build failure:

$ java -cp cljs.jar:src clojure.main node.clj
Exception in thread "main" java.lang.IllegalArgumentException: Cannot open <nil> as a Reader., compiling:(/Users/mfikes/Desktop/hello_world/node.clj:3:1)
	at clojure.lang.Compiler.load(Compiler.java:7526)
	at clojure.lang.Compiler.loadFile(Compiler.java:7452)
	at clojure.main$load_script.invokeStatic(main.clj:278)
	at clojure.main$script_opt.invokeStatic(main.clj:338)
	at clojure.main$script_opt.invoke(main.clj:333)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: Cannot open <nil> as a Reader.
	at clojure.java.io$fn__10992.invokeStatic(io.clj:288)
	at clojure.java.io$fn__10992.invoke(io.clj:288)
	at clojure.java.io$fn__10894$G__10870__10901.invoke(io.clj:69)
	at clojure.java.io$reader.invokeStatic(io.clj:102)
	at clojure.java.io$reader.doInvoke(io.clj:86)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$slurp.invokeStatic(core.clj:6862)
	at clojure.core$slurp.doInvoke(core.clj:6862)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.closure$output_bootstrap.invokeStatic(closure.clj:2654)
	at cljs.closure$build.invokeStatic(closure.clj:2789)
	at cljs.build.api$build.invokeStatic(api.clj:204)
	at cljs.build.api$build.invoke(api.clj:189)
	at cljs.build.api$build.invokeStatic(api.clj:192)
	at cljs.build.api$build.invoke(api.clj:189)
	at user$eval34.invokeStatic(node.clj:3)
	at user$eval34.invoke(node.clj:3)
	at clojure.lang.Compiler.eval(Compiler.java:7062)
	at clojure.lang.Compiler.load(Compiler.java:7514)
	... 9 more


 Comments   
Comment by David Nolen [ 12/Feb/18 11:31 AM ]

fixed https://github.com/clojure/clojurescript/commit/578f732f19e36d39ed2b215e3fa67158424966d2





[CLJS-2376] Add support for ES6 default imports/exports Created: 02/Oct/17  Updated: 12/Feb/18

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

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: Juho Teperi
Resolution: Unresolved Votes: 8
Labels: npm-deps

Attachments: Text File CLJS-2376-1.patch    

 Description   

ES6 has special syntax for using "default" imports and there is currently no equivalent for CLJS when using imported ES6 code.

import * as name from "module-name";
(:require ["module-name" :as name])

import { export } from "module-name";
(:require ["module-name" :refer (export)])

import { export as alias } from "module-name";
(:require ["module-name" :refer (export) :rename {export alias}])

import defaultExport from "module-name";
import defaultExport, { export [ , [...] ] } from "module-name";
import defaultExport, * as name from "module-name";
;; no easy access to defaultExport

I'm proposing to add a :default to the ns :require.

(:require ["module-name" :as mod :default foo])

This makes it much more convenient to use rewritten ES6 code from CLJS. If "module-name" has a default export you currently have to write mod/default everywhere since they is no easy way to alias the default.

(:require ["material-ui/RaisedButton" :as RaisedButton])
;; :as is incorrect and the user must now use
RaisedButton/default

(:require ["material-ui/RaisedButton" :default RaisedButton])
;; would allow direct use of
RaisedButton

Internally the Closure compiler (and ES6 code rewritten by babel) will rewrite default exports to a .default property, so :default really is just a convenient way to access it.

The long version that already works is

(:require ["material-ui/RaisedButton" :refer (default) :rename {default RaisedButton}])

When ES6 becomes more widespread we should have convenient way to correctly refer to "default" exports.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import



 Comments   
Comment by Juho Teperi [ 10/Jan/18 2:25 PM ]

First take on the implementation. This implementation rewrites :default name to {{:refer [default] :rename {default name}}} at the lowest level (parse-require-spec), so I think other functions don't need to be changed.

I didn't update the require spec validation errors yet.

Comment by David Nolen [ 12/Feb/18 6:25 AM ]

It seems to me the most desirable syntax for ES6 default imports is the following:

(:require ["material-ui/RaisedButton" :refer [RaisedButton]])

I don't see why we couldn't just make this work?

Comment by Thomas Heller [ 12/Feb/18 6:55 AM ]

I don't understand that suggestion. The default export has no name, the .default property is only used in interop by transpilers. In actual JS a live reference is used. How would you map RaisedButton back to default?

["material-ui/RaisedButton" :default foo] would be valid. The JS module may also actually have an `export { RaisedButton }` in addition to `export default ...`, which would lead to a conflict if you "guess" the wrong name. Default exports are not aliases for the module itself, that is separate.

I made a reference translation table [1] and adding the :default alias is the only way to reliably map all ES6 import/export features. I don't see a way to make :refer work that would not be totally non-intuitive and unreliable.

import defaultExport, * as name from "module-name";

[1] https://shadow-cljs.github.io/docs/UsersGuide.html#_using_npm_packages

Comment by David Nolen [ 12/Feb/18 8:55 AM ]

Let's leave rhetoric out of this discussion please and focus on the technical bits. I just don't want to include another directive in the ns form and I would prefer an approach that avoids that. From my point of view there has not been enough exploration of that alternative.

This ticket is mostly about convenience, and I think we need to stew on this one for a bit longer before deciding on anything.





[CLJS-2509] Command line -v not properly handled as an init opt Created: 12/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

Observe how -e is properly handled here:

$ java -jar cljs.jar -e '(inc 1)' - foo bar <<INPUT
> (println *command-line-args*)
> INPUT
2
(foo bar)

But using -v instead causes things to derail with it not being treated as just another init opt.

$ java -jar cljs.jar -v - foo bar <<INPUT
> (println *command-line-args*)
> INPUT
Exception in thread "main" java.lang.IllegalArgumentException: Cannot open <nil> as a Reader.
	at clojure.java.io$fn__10992.invokeStatic(io.clj:288)
	at clojure.java.io$fn__10992.invoke(io.clj:288)
	at clojure.java.io$fn__10894$G__10870__10901.invoke(io.clj:69)
	at clojure.java.io$reader.invokeStatic(io.clj:102)
	at clojure.java.io$reader.doInvoke(io.clj:86)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.analyzer$parse_ns$fn__2827.invoke(analyzer.cljc:3762)
	at cljs.analyzer$parse_ns.invokeStatic(analyzer.cljc:3745)
	at cljs.analyzer$parse_ns.invoke(analyzer.cljc:3724)
	at cljs.analyzer$parse_ns.invokeStatic(analyzer.cljc:3735)
	at cljs.closure$src_file__GT_target_file.invokeStatic(closure.clj:2964)
	at cljs.closure$src_file__GT_target_file.invoke(closure.clj:2957)
	at cljs.closure$src_file__GT_target_file.invokeStatic(closure.clj:2959)
	at cljs.repl$load_file$fn__6908.invoke(repl.cljc:555)
	at cljs.repl$load_file.invokeStatic(repl.cljc:555)
	at cljs.repl$load_file.invoke(repl.cljc:547)
	at cljs.repl$load_file.invokeStatic(repl.cljc:548)
	at cljs.cli$main_opt_STAR_$fn__7194.invoke(cli.clj:112)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:98)
	at cljs.cli$script_opt.invokeStatic(cli.clj:141)
	at cljs.cli$script_opt.invoke(cli.clj:141)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:204)
	at cljs.cli$main.doInvoke(cli.clj:162)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)


 Comments   
Comment by Mike Fikes [ 12/Feb/18 8:52 AM ]

Invalid ticket. The -v option takes a Boolean argument.

$ java -jar cljs.jar -v true - foo bar <<INPUT
> (println *command-line-args*)
> INPUT
Reading analysis cache for jar:file:/Users/mfikes/Projects/planck/int-test/cljs.jar!/cljs/core.cljs
(foo bar)




[CLJS-2462] subvec on non-integral indexes fails Created: 09/Jan/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

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

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

 Description   

In Clojure, you can evidently pass non-integral indexes to count. For example (count (subvec [1 2] 0 1.5)) evaluates to 1 in Clojure. But, in ClojureScript, this evaluates to 1.5.

It is not clear whether passing non-integral indexes is legal, or if doing so leads to undefined behavior (in which case this ticket is invalid).



 Comments   
Comment by Mike Fikes [ 09/Jan/18 11:30 AM ]

We had discussed this in #cljs-dev, and concluded that subvec should round its arguments down to the nearest integer. Theoretically speaking, Math/floor is the correct operation, given that vector indexes can go up to 2^32 - 1, but pragmatically speaking, int is a much more compelling choice because it is faster (compiling down to a bit-or with zero). This choice would cause errors if the arguments happen to be greater than 2147483647.

Comment by David Nolen [ 11/Feb/18 4:49 PM ]

This patch needs a rebase.

Comment by Mike Fikes [ 11/Feb/18 5:58 PM ]

CLJS-2462-2.patch rebases.

Comment by David Nolen [ 12/Feb/18 5:59 AM ]

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





[CLJS-2474] with-meta on lazy-seq causes separate realization Created: 20/Jan/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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-2474-2.patch     Text File CLJS-2474-3.patch     Text File CLJS-2474-4.patch     Text File CLJS-2474.patch    
Patch: Code and Test

 Description   

Calling with-meta on a lazy-seq causes a separate realization, which is problematic if the lazy sequence is not deterministically realized. Observe this behavior in the following REPL interaction, which doesn't occur in Clojure:

cljs.user=> (defn rand-seq [] (lazy-seq (cons (rand) (rand-seq))))
#'cljs.user/rand-seq
cljs.user=> (def xs (rand-seq))
#'cljs.user/xs
cljs.user=> (def ys (with-meta xs {:foo 1}))
#'cljs.user/ys
cljs.user=> (take 3 xs)
(0.608952482736965 0.09053783024879025 0.3209446424968001)
cljs.user=> (take 3 ys)
(0.33162671415201395 0.5719320838932136 0.8606932935816556)

See CLJ-1800 for some insight into how Clojure avoids this, preserving the immutability of the return value of lazy-seq even in the face of adding meta.



 Comments   
Comment by Mike Fikes [ 20/Jan/18 4:51 PM ]

The attached patch is derived from the one in CLJ-1800. (Michael Blume has signed the CCA, so this derived patch should be fine from that perspective.)

This approach is important because in ClojureScript (unlike Clojure), calling with-meta on the return value of lazy-seq doesn't realize the first element. In other words, going with the CLJ-1800 approach preserves the existing laziness in ClojureScript.

The patch fundamentally works by tying the with-meta result back to the original sequence with a delayed call to seq on the original sequence. This is not part of included tests, but key to understanding how it achieves immutability is that while (not (identical? xs ys)), this is true: (identical? (rest xs) (rest ys)).

Comment by Mike Fikes [ 21/Jan/18 7:45 AM ]

The second attached patch makes a minor tweak to call -seq rather than to indirectly go through seq.

Comment by Mike Fikes [ 21/Jan/18 7:54 AM ]

Sorry for the churn: The third attached patch adds a test ensuring that the empty lazy-seq case works properly.

Comment by David Nolen [ 11/Feb/18 4:53 PM ]

Needs a rebase

Comment by Mike Fikes [ 11/Feb/18 5:34 PM ]

CLJS-2474-4.patch rebases against master

Comment by David Nolen [ 12/Feb/18 5:58 AM ]

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





[CLJS-2501] NPE in cljs.util/compiled-by-version Created: 11/Feb/18  Updated: 12/Feb/18  Resolved: 12/Feb/18

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

Type: Defect Priority: Minor
Reporter: Dieter Komendera Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2501.patch    

 Description   

We're sometimes running into null pointer exceptions with the ClojureScript compiler, which can only be resolved by cleaning
the build artifacts from the previous build.

java.lang.NullPointerException: null
 at java.util.regex.Matcher.getTextLength (Matcher.java:1283)
    java.util.regex.Matcher.reset (Matcher.java:309)
    java.util.regex.Matcher.<init> (Matcher.java:229)
    java.util.regex.Pattern.matcher (Pattern.java:1093)
    clojure.core$re_matcher.invokeStatic (core.clj:4796)
    clojure.core$re_matches.invokeStatic (core.clj:4826)
    clojure.core$re_matches.invoke (core.clj:4826)
    cljs.util$compiled_by_version.invokeStatic (util.cljc:42)
    cljs.util$compiled_by_version.invoke (util.cljc:39)
    cljs.closure$compile_from_jar.invokeStatic (closure.clj:586)
    cljs.closure$compile_from_jar.invoke (closure.clj:579)
    cljs.closure$eval53393$fn__53394.invoke (closure.clj:619)
    cljs.closure$eval53323$fn__53324$G__53312__53331.invoke (closure.clj:493)
    cljs.closure$compile_task$fn__53481.invoke (closure.clj:904)
    cljs.closure$compile_task.invokeStatic (closure.clj:902)
    cljs.closure$compile_task.invoke (closure.clj:894)
    cljs.closure$parallel_compile_sources$fn__53487.invoke (closure.clj:931)

These stem from empty .js files in the build artifacts, which when ClojureScript compiler tries to parse the compiler version and build options from, crashes. While I couldn't yet find the reason for those empty .js files, it seems reasonable to not crash and just consider the artifact stale.



 Comments   
Comment by Dieter Komendera [ 11/Feb/18 4:06 AM ]

Patch

Comment by David Nolen [ 12/Feb/18 5:57 AM ]

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





[CLJS-2475] Actual and expected on recur argument count mismatch Created: 21/Jan/18  Updated: 11/Feb/18

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

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

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

 Description   

When a recur argument count mismatch is detected, analysis fails with "recur argument count mismatch." In Clojure, the actual and expected number of arguments is indicated as well.

This additional information might be helpful to developers situations where the desired recur target is farther away than an enclosing target, e.g.:

(loop [x 1 y 2] #(when true (recur 3 4)))

or in cases where an enclosing target exists upon macroexpansion, as in:

(loop [x 1 y 2] (lazy-seq (recur 3 4)))

In both examples above, Clojure will indicate "Mismatched argument count to recur, expected: 0 args, got: 2."



 Comments   
Comment by David Nolen [ 11/Feb/18 5:30 PM ]

Needs rebase

Comment by Mike Fikes [ 11/Feb/18 5:45 PM ]

CLJS-2475-2.patch rebases against master.





[CLJS-2487] Unroll (list ...) constructs to List ctor calls Created: 30/Jan/18  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

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

Attachments: Text File CLJS-2487-2.patch     Text File CLJS-2487.patch     PNG File perf-curves.png    
Patch: Code
Approval: Accepted

 Description   

For small (list arg1 arg2 arg3) constructs, have the macro expand to code that directly uses the List ctor.

Here is an example of the perf benefit in the case of 2 arguments:

(defn bench-fn [] (list 17 41))
(goog/exportSymbol "indirect" bench-fn)
(simple-benchmark [] (js/indirect) 1e7)

produces

Benchmarking with V8
[], (js/indirect), 10000000 runs, 106 msecs
Benchmarking with SpiderMonkey
[], (js/indirect), 10000000 runs, 226 msecs
Benchmarking with JavaScriptCore
[], (js/indirect), 10000000 runs, 366 msecs
Benchmarking with Nashorn
[], (js/indirect), 10000000 runs, 4789 msecs
Benchmarking with ChakraCore
[], (js/indirect), 10000000 runs, 874 msecs

While

(defn bench-fn [] (List. nil 17 (List. nil 41 () 1 nil) 2 nil))
(goog/exportSymbol "indirect" bench-fn)
(simple-benchmark [] (js/indirect) 1e7)

produces

Benchmarking with V8
[], (js/indirect), 10000000 runs, 50 msecs
Benchmarking with SpiderMonkey
[], (js/indirect), 10000000 runs, 217 msecs
Benchmarking with JavaScriptCore
[], (js/indirect), 10000000 runs, 302 msecs
Benchmarking with Nashorn
[], (js/indirect), 10000000 runs, 4340 msecs
Benchmarking with ChakraCore
[], (js/indirect), 10000000 runs, 670 msecs

Perhaps this could be done for any number of list arguments, or, if perf becomes an issue for large numbers of arguments maybe it could be done only up to a certain size. (Maybe CLJS-910 factors in on older JavaScriptCore instances.)

CLJS-1617 is relevant.



 Comments   
Comment by Mike Fikes [ 30/Jan/18 2:51 PM ]

The perf curves show the effects on V8, SpiderMonkey, and JavaScriptCore for 1, 2, 3, 4 and 5 arguments.

Both non-const and const argument curves are shown (they appear to be essentially the same, but both are included for completeness).

In summary, you can see that the patch is a wash for SpiderMonkey, has a large benefit for V8 (speedup of 6 for 5 arguments), and decent improvement for JavaScriptCore (speedup of 1.2 for 5 arguments).

Comment by Mike Fikes [ 30/Jan/18 5:11 PM ]

Assigning back to me to deal with the aspect that the attached patch puts the empty list as the last element instead of nil. Using nil should terminate a little more quickly, and be more consistent with the way ClojureScript currently behaves.

Comment by Mike Fikes [ 30/Jan/18 5:47 PM ]

The attached CLJS-2487-2.patch adds handling for the 1-arity case so that nil is placed as the last item in the chain rather than the empty list. The performance characteristics are essentially the same as those attached for the CLJS-2487.patch

Comment by David Nolen [ 11/Feb/18 5:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/755924247bad8a539eadc08d024beb1b4d841c90





[CLJS-2095] Nashorn runner Created: 16/Jun/17  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

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


 Description   

Then we could provide a test runner out of the box. See CLJS-1076



 Comments   
Comment by David Nolen [ 11/Feb/18 5:22 PM ]

more or less resolved by the CLI oriented commits leading to this one https://github.com/clojure/clojurescript/commit/4b60d2e0eadbd5dffb975776cfc8b419c4e099ca





[CLJS-2486] Map entry much slower for first Created: 28/Jan/18  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

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

 Description   

If you do apply first to the result of getting an entry from a map, things are now slower.

Before: (https://github.com/clojure/clojurescript/commit/9ddd356d344aa1ebf9bd9443dd36a1911c92d32f)

Benchmarking with V8
[me (first {:a 1})], (first me), 10000000 runs, 267 msecs
Benchmarking with SpiderMonkey
[me (first {:a 1})], (first me), 10000000 runs, 378 msecs
Benchmarking with JavaScriptCore
[me (first {:a 1})], (first me), 10000000 runs, 406 msecs
Benchmarking with Nashorn
[me (first {:a 1})], (first me), 10000000 runs, 2232 msecs
Benchmarking with ChakraCore
[me (first {:a 1})], (first me), 10000000 runs, 1204 msecs

After: (https://github.com/clojure/clojurescript/commit/91431bd556f7a11db59319fcc082737a448f651e)

Benchmarking with V8
[me (first {:a 1})], (first me), 10000000 runs, 1106 msecs
Benchmarking with SpiderMonkey
[me (first {:a 1})], (first me), 10000000 runs, 720 msecs
Benchmarking with JavaScriptCore
[me (first {:a 1})], (first me), 10000000 runs, 689 msecs
Benchmarking with Nashorn
[me (first {:a 1})], (first me), 10000000 runs, 6711 msecs
Benchmarking with ChakraCore
[me (first {:a 1})], (first me), 10000000 runs, 1969 msecs


 Comments   
Comment by Mike Fikes [ 28/Jan/18 9:39 PM ]

By creating an IndexedSeq. on a 2-element JavaScript array in the -seq implementation, we can regain most of the perf that we had with the prior implementation where map entries were 2-element persistent vectors:

After the patch is applied:

Benchmarking with V8
[me (first {:a 1})], (first me), 10000000 runs, 294 msecs
Benchmarking with SpiderMonkey
[me (first {:a 1})], (first me), 10000000 runs, 503 msecs
Benchmarking with JavaScriptCore
[me (first {:a 1})], (first me), 10000000 runs, 491 msecs
Benchmarking with Nashorn
[me (first {:a 1})], (first me), 10000000 runs, 5304 msecs
Benchmarking with ChakraCore
[me (first {:a 1})], (first me), 10000000 runs, 1390 msecs

While this helps with seq, first, and second, this also really helps a lot if code happens to call nth on the resulting seq.

The patch applies the change to all of the map entry types and also to the rseq implementations in those types.

Comment by Mike Fikes [ 30/Jan/18 7:43 AM ]

I tried (List. nil key (List. nil val () 1 nil) 2 nil) as a replacement for {{(IndexedSeq. #js [key val] 0 nil)}} and for

(simple-benchmark [me (first {:a 1})] (first me) 10000000)

I get speedups of 0.88 for V8, 0.83 for SpiderMonkey, and 0.73 for JavaScriptCore relative to the IndexedSeq. version. (In other words, the List. construct is slower than the IndexedSeq. construct.)

Comment by David Nolen [ 11/Feb/18 5:10 PM ]

fixed https://github.com/clojure/clojurescript/commit/15027e1d558a42ae91939d5f7332c8a95bfd0070





[CLJS-2476] recur across try should fail compilation Created: 22/Jan/18  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

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

Attachments: Text File CLJS-2476.patch    
Patch: Code and Test
Approval: Accepted

 Description   

In Clojure: (loop [] (try (recur))) results in "Cannot recur across try."

In ClojureScript:

((fn foo [n] 
   (try (prn 'x n) 
     (when (pos? n) 
       (foo (dec n))) 
     (finally (prn 'f n)))) 
 5)

prints

x 5
x 4
x 3
x 2
x 1
x 0
f 0
f 1
f 2
f 3
f 4
f 5

while replacing the tail call with a recur

((fn foo [n] 
   (try (prn 'x n) 
     (when (pos? n) 
       (recur (dec n))) 
     (finally (prn 'f n)))) 
 5)

generates code that prints

x 5
f 4
x 4
f 3
x 3
f 2
x 2
f 1
x 1
f 0
x 0
f 0


 Comments   
Comment by Mike Fikes [ 22/Jan/18 2:50 PM ]

The attached patch also fixes a problem in the cljs.js namespace where recur was used across try. Instead of making direct recursive calls, which would grow the stack, the fixe there employs trampoline to achieve the same effect as recur.

Comment by David Nolen [ 11/Feb/18 5:06 PM ]

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





[CLJS-1195] generic reusable command line argument parsing for REPLs Created: 10/Apr/15  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Comment by David Nolen [ 11/Feb/18 3:37 PM ]

more or less covered by the work leading up to this commit https://github.com/clojure/clojurescript/commit/c7d800c4385c76385582f298ac802e4701cc10bf





[CLJS-1076] :nashorn target Created: 02/Mar/15  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Description   

To run well on Nashorn the target should supply CLOSURE_IMPORT_SCRIPT as well as setTimeout or setImmediate for core.async.



 Comments   
Comment by Mike Fikes [ 02/Feb/18 7:27 AM ]

Partially done with: https://github.com/clojure/clojurescript/commit/d95705b92fbdb04165a990382f27d865c152da43

Comment by David Nolen [ 11/Feb/18 2:49 PM ]

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

Comment by Mike Fikes [ 11/Feb/18 3:21 PM ]

See unit test regression CLJS-2502.





[CLJS-2497] Issue launching main program (src.substring) Created: 09/Feb/18  Updated: 11/Feb/18  Resolved: 11/Feb/18

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   

With src/foo/core.cljs containing

(ns foo.core)

(println "*command-line-args*" *command-line-args*)

(defn -main [& args]
  (println "args:" args))

and cljs.jar built from https://github.com/clojure/clojurescript/commit/2f9e50c230969c217e1465958a9480883a55961b

I get a failure:

$ java -cp cljs.jar:src clojure.main -m cljs.repl.node -o out -e "(+ 3 4)" -m foo.core 1 2
ClojureScript Node.js REPL server listening on 54886
7
Exception in thread "main" clojure.lang.ExceptionInfo: repl:65
        if(src.substring(0, 2) == "..") {
               ^

TypeError: Cannot read property 'substring' of undefined
    at global.CLOSURE_IMPORT_SCRIPT (repl:65:16)
    at Object.goog.require (repl:21:8)
    at repl:3:6
    at Script.runInThisContext (vm.js:65:33)
    at Object.runInThisContext (vm.js:199:38)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:317:14)
    at Socket.<anonymous> ([stdin]:49:25)
    at Socket.emit (events.js:160:13)
    at addChunk (_stream_readable.js:269:12) {:type :js-eval-exception, :error {:status :exception, :value "repl:65\n        if(src.substring(0, 2) == \"..\") {\n               ^\n\nTypeError: Cannot read property 'substring' of undefined\n    at global.CLOSURE_IMPORT_SCRIPT (repl:65:16)\n    at Object.goog.require (repl:21:8)\n    at repl:3:6\n    at Script.runInThisContext (vm.js:65:33)\n    at Object.runInThisContext (vm.js:199:38)\n    at Domain.<anonymous> ([stdin]:50:34)\n    at Domain.run (domain.js:317:14)\n    at Socket.<anonymous> ([stdin]:49:25)\n    at Socket.emit (events.js:160:13)\n    at addChunk (_stream_readable.js:269:12)"}, :repl-env #cljs.repl.node.NodeEnv{:host "localhost", :port 54886, :path nil, :socket #object[clojure.lang.Atom 0x5edf2821 {:status :ready, :val {:socket #object[java.net.Socket 0xe1e2e5e "Socket[addr=localhost/127.0.0.1,port=54886,localport=52437]"], :in #object[java.io.BufferedReader 0x661c46bc "java.io.BufferedReader@661c46bc"], :out #object[java.io.BufferedWriter 0x37864b77 "java.io.BufferedWriter@37864b77"]}}], :proc #object[clojure.lang.Atom 0x4dbad37 {:status :ready, :val #object[java.lang.UNIXProcess 0x2b98b3bb "java.lang.UNIXProcess@2b98b3bb"]}], :debug-port nil}, :form (do (set! clojure.core/*command-line-args* (clojure.core/list "1" "2")) (.require js/goog "foo.core") (foo.core/-main "1" "2")), :js "cljs.core._STAR_command_line_args_STAR_ = cljs.core._conj.call(null,cljs.core._conj.call(null,cljs.core.List.EMPTY,\"2\"),\"1\");\n\ngoog.require(\"foo.core\");\n\nfoo.core._main.call(null,\"1\",\"2\");\n"}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:532)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:464)
	at cljs.repl$evaluate_form.invoke(repl.cljc:457)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:462)
	at cljs.cli$main_opt$fn__7138.invoke(cli.clj:106)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1271)
	at cljs.cli$main_opt.invokeStatic(cli.clj:92)
	at cljs.cli$main_opt.invoke(cli.clj:70)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:191)
	at cljs.cli$main.doInvoke(cli.clj:145)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.repl.node$_main.invokeStatic(node.clj:235)
	at cljs.repl.node$_main.doInvoke(node.clj:235)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.main$main_opt.invokeStatic(main.clj:317)
	at clojure.main$main_opt.invoke(main.clj:313)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)

I'm only presuming this has landed in master only based on a comment that David made in #cljs-dev:

stuff like this works now
java -cp cljs.jar:src clojure.main -m cljs.repl.node -o out -e "(+ 3 4)" -m foo.core 1 2
outputs

ClojureScript Node.js REPL server listening on 53734
7
Analyzing file:/Users/davidnolen/development/clojure/main-cljs/src/foo/core.cljs
args: (1 2)
*command-line-args* (1 2)


 Comments   
Comment by Mike Fikes [ 10/Feb/18 12:07 PM ]

Revising to use instead `cljs.main` with https://github.com/clojure/clojurescript/commit/d9408cb07263252df5e752427bfdb78b90dd10cb results in an NPE:

$ java -cp cljs.jar:src cljs.main -m cljs.repl.node -o out -e "(+ 3 4)" -m foo.core 1 2
Exception in thread "main" java.lang.NullPointerException
	at java.util.regex.Matcher.getTextLength(Matcher.java:1283)
	at java.util.regex.Matcher.reset(Matcher.java:309)
	at java.util.regex.Matcher.<init>(Matcher.java:229)
	at java.util.regex.Pattern.matcher(Pattern.java:1093)
	at clojure.core$re_matcher.invokeStatic(core.clj:4796)
	at clojure.core$re_find.invokeStatic(core.clj:4838)
	at cljs.analyzer$analyze_file.invokeStatic(analyzer.cljc:4034)
	at cljs.analyzer$analyze_file.invoke(analyzer.cljc:4013)
	at cljs.analyzer$analyze_file.invokeStatic(analyzer.cljc:4027)
	at cljs.analyzer.api$analyze_file.invokeStatic(api.cljc:138)
	at cljs.analyzer.api$analyze_file.invoke(api.cljc:120)
	at cljs.analyzer.api$analyze_file.invokeStatic(api.cljc:131)
	at cljs.cli$main_opt_STAR_$fn__7149.invoke(cli.clj:104)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1285)
	at cljs.cli$main_opt_STAR_.invokeStatic(cli.clj:90)
	at cljs.cli$main_opt.invokeStatic(cli.clj:113)
	at cljs.cli$main_opt.invoke(cli.clj:113)
	at clojure.core$partial$fn__5561.invoke(core.clj:2617)
	at cljs.cli$main.invokeStatic(cli.clj:203)
	at cljs.cli$main.doInvoke(cli.clj:158)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at cljs.main$_main.invokeStatic(main.clj:35)
	at cljs.main$_main.doInvoke(main.clj:34)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at cljs.main.main(Unknown Source)
Comment by David Nolen [ 11/Feb/18 12:10 PM ]

Should be fixed by https://github.com/clojure/clojurescript/commit/4b60d2e0eadbd5dffb975776cfc8b419c4e099ca. Mike if this works for you, go ahead and close.

Comment by Mike Fikes [ 11/Feb/18 2:07 PM ]

Confirmed fixed.





[CLJS-2500] Process-js-modules has to be called after compiler restart Created: 10/Feb/18  Updated: 11/Feb/18  Resolved: 11/Feb/18

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

Type: Defect Priority: Major
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-2500.patch    
Approval: Accepted

 Description   

Since https://github.com/clojure/clojurescript/commit/245bdee2c35e19a9685b7a0849f26fce8bdaf7ca process-js-modules is only called if foreign-lib files have changed after last Cljs compilation. However, process-js-modules has to be called after restarting Cljs compiler to populate :js-namespaces and :js-module-index in compiler state.



 Comments   
Comment by David Nolen [ 11/Feb/18 8:32 AM ]

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





[CLJS-2382] circular dependency in node_modules prevents compilation Created: 13/Oct/17  Updated: 10/Feb/18

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

Type: Defect Priority: Minor
Reporter: Hendrik Poernama Assignee: Juho Teperi
Resolution: Unresolved Votes: 0
Labels: npm-deps


 Description   

Compiler complains of missing provides when there is circular dependency in node_modules.

Cause is the order of the 'goog.addDependency' statements in out/cljs_deps.js

goog.addDependency("../node_modules/lib1.js", ['lib1'], ['lib2']); // This line will fail since 'lib2' is not yet provided
goog.addDependency("../node_modules/lib2.js", ['lib2'], ['lib1']);

Example of affected node_modules: apollo-client 1.9.2

I'm not sure if this is a closure compiler limitation or explicitly unsupported, but it does reduce the number of node packages that can be included using node_modules.

Current workaround is to rewrite the library code to not have circular deps or to switch to cljsjs.



 Comments   
Comment by Juho Teperi [ 10/Feb/18 5:28 PM ]

I didn't notice circular dependencies when testing Apollo-client 1.9.2 with latest Cljs changes, but there is something else strange going on.

code
// Code from apollo-client references some modules by incorrect name:
goog.provide("module$home$juho$Source$reagent$test_environments$browser_node$node_modules$apollo_client$transport$networkInterface");
goog.require("module$whatwg_fetch");
goog.require("module$graphql$language$printer");

// From whatwg file:
goog.provide("module$home$juho$Source$reagent$test_environments$browser_node$node_modules$whatwg_fetch$fetch");
code

It is possible this is caused by Cljs bug, because provide name is created by us, and require names are provided by Closure.





[CLJS-2412] Consuming npm module Created: 22/Nov/17  Updated: 10/Feb/18  Resolved: 10/Feb/18

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

Type: Defect Priority: Minor
Reporter: Jan Břečka Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

When trying to :require the uuid (https://www.npmjs.com/package/uuid) npm package:

(:require ["uuid/v4" :as uuid-v4])

compiled javascript is failing on Reference error: require is not defined. It's caused because the library itself requiring two files:

var rng = require('./lib/rng');
var bytesToUuid = require('./lib/bytesToUuid');

While the require('./lib/bytesToUuid') call is properly replaced with "goog provide", the require('./lib/rng') is not touched at all. This is part of the package.json the library provides:

"browser": { "./lib/rng.js": "./lib/rng-browser.js", "./lib/sha1.js": "./lib/sha1-browser.js" }

The problem is that this "browser" alternative is ignored.



 Comments   
Comment by Thomas Heller [ 22/Nov/17 12:14 PM ]

There are 2 pending pull requests for the Closure Compiler related to this.

https://github.com/google/closure-compiler/pull/2622
https://github.com/google/closure-compiler/pull/2627

Comment by Juho Teperi [ 10/Feb/18 4:33 PM ]

Should be fixed by https://github.com/clojure/clojurescript/commit/72e2ab6e63b3341aa26abcbdd72dc291cbd0c462.

Just tested and require("./lib/rng.js") is resolved into ./lib/rng-browser.js.





[CLJS-2447] Ignore non-js (css) JS modules Created: 20/Dec/17  Updated: 10/Feb/18  Resolved: 22/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Juho Teperi Assignee: Juho Teperi
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2447-2.patch     Text File CLJS-2447.patch    

 Description   

Some Node packages (like firebase-web-react: https://github.com/firebase/firebaseui-web-react/blob/master/dist/FirebaseAuth.js#L35) try to require css files. Webpack can bundle the css files. Closure doesn't support this.

I think this could be solved by ignoring these, by just passing empty string as source for these files, so Closure doesn't fail when it tries to parse the file as JS.



 Comments   
Comment by Juho Teperi [ 20/Dec/17 1:38 PM ]

Patch to ignore contents of non .js/.json files in module processing. Existing tests pass, but I didn't have time to test this with real case.

Comment by David Nolen [ 22/Dec/17 2:15 PM ]

Patch has a typo

Comment by Juho Teperi [ 22/Dec/17 2:34 PM ]

Oh, sorry. Renamed that to url.

Comment by David Nolen [ 22/Dec/17 5:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/123e8f9aa59899c6886bbe392128b22ae9a0def3





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

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

Type: Enhancement Priority: Major
Reporter: Arne Brasseur Assignee: Juho Teperi
Resolution: Completed Votes: 1
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

Comment by Juho Teperi [ 04/Aug/17 4:41 PM ]

I'm working on improving UMD wrapper support on Closure (https://github.com/google/closure-compiler/pull/2597), and I think the d3 wrapper is in fact the same as Leaflet wrapper, which I have already made some progress with.

Comment by Juho Teperi [ 10/Feb/18 4:23 PM ]

D3 from NPM still doesn't work due to https://github.com/google/closure-compiler/issues/2005, but the UMD module as foreign-lib with :module-type :commonjs (like in the first comment) works with the latest changes (updated Closure with better UMD wrapper detection).

Comment by Juho Teperi [ 10/Feb/18 4:23 PM ]

Fixed by https://github.com/clojure/clojurescript/commit/72e2ab6e63b3341aa26abcbdd72dc291cbd0c462





[CLJS-2498] Unit tests failing after AMD module support removal Created: 09/Feb/18  Updated: 10/Feb/18  Resolved: 10/Feb/18

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: test


 Description   

With https://github.com/clojure/clojurescript/commit/333dddbfb890f11dc6a21987437552093c5b9511
script/test:

ERROR in (test-cljs-2286) (TypeError:NaN:NaN)
expected: (= 3 (es6_calc/calculator.add 1 2))
  actual: #object[TypeError TypeError: Cannot read property 'add' of undefined]


 Comments   
Comment by Mike Fikes [ 10/Feb/18 8:09 AM ]

No now longer reproducible on master.





[CLJS-2499] Implement ClojureScript pREPL Created: 10/Feb/18  Updated: 10/Feb/18

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

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





[CLJS-2495] Closure compilation errors should stop ClojureScript compilation Created: 08/Feb/18  Updated: 10/Feb/18  Resolved: 10/Feb/18

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

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

Attachments: Text File CLJS-2495.patch    

 Description   

If Closure reports error, it is printed by report-failure, but ClojureScript will continue. This can (often will) result in broken output.

This problem can happen with both Module Processing (some files not being processed, e.g. CommonJS code is present in output) and with optimization step. If optimization fails (Closure can't parse some files), optimize doesn't throw but returns nil, which is written to output-file.



 Comments   
Comment by David Nolen [ 10/Feb/18 6:10 AM ]

I think we want to log those errors right? Or do we already do that?

Comment by David Nolen [ 10/Feb/18 6:11 AM ]

Oops I see that we do. Looks good.

Comment by David Nolen [ 10/Feb/18 6:11 AM ]

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





[CLJS-2494] Add option to disable node_modules use Created: 08/Feb/18  Updated: 10/Feb/18  Resolved: 10/Feb/18

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

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

Attachments: Text File CLJS-2494.patch    

 Description   

There are four different use cases related to Node Modules and their use:

1. Project has `:npm-deps` and `:install-deps true`
2. Project uses libraries with `:npm-deps` and uses `:install-deps true`
3. Project has `package.json` and packages are installed by manually running `npm install` and Node packages should be usable from Cljs
4. Project has Node packages installed but `node_modules` shouldn't be used

Fourth option is not currently possible, because Cljs will read node_modules always if it exists.

Proposed fix: If :npm-deps value is false, node_modules wont be read.



 Comments   
Comment by David Nolen [ 10/Feb/18 6:08 AM ]

fixed https://github.com/clojure/clojurescript/commit/486de1a8b0836dbe3a622662a69f57aa92d232de





[CLJS-2389] Fix module processing after latest Closure changes Created: 27/Oct/17  Updated: 10/Feb/18  Resolved: 10/Feb/18

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

Type: Defect Priority: Major
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-2389-1.patch     Text File CLJS-2389-3.patch     Text File CLJS-2389-4.patch     Text File CLJS-2389-5.patch     Text File CLJS-2389-6.patch     Text File CLJS-2389-7.patch    
Approval: Accepted

 Description   

New Closure-compiler doesn't add goog.provide/require calls to processed modules:

https://github.com/google/closure-compiler/pull/2641

> ES6 and CommonJS modules no longer generate synthetic goog.require and goog.provide calls.

Currently {process-js-modules} uses {load-library}, which reads the goog.provide calls in the file, to determine the name for processed module, something like {module$absolute$path}.
Now that the file doesn't have goog.provide call, this breaks.

As the module name is based on file-path, we can determine the module name directly from filepath, Closure provides utility for this: {ModuleNames/fileToModuleName}.



 Comments   
Comment by Juho Teperi [ 27/Oct/17 5:53 AM ]

Attached patch fixes the first problem, which is updating {:js-module-index} mapping from Node name to processed module name.

Another problem is that analyzer/check-uses now fails when referring to symbols in processed modules:

ERROR in (test-module-name-substitution) (core.clj:4617)
expected: (= (compile (quote (ns my-calculator.core (:require [calculator :as calc :refer [subtract add] :rename {subtract sub}])))) (str "goog.provide('my_calculator.core');" crlf "goog.require('cljs.core');" crlf "goog.require('" (absolute-module-path "
src/test/cljs/calculator.js" true) "');" crlf))
  actual: clojure.lang.ExceptionInfo: Invalid :refer, var module$home$juho$Source$clojurescript$src$test$cljs$calculator/add does not exist

For some reason {missing-use?} doesn't work correctly.

Comment by Juho Teperi [ 06/Dec/17 12:47 PM ]

I'm looking into emitting those goof.provide/require calls from Cljs. I have provide working already and that fixes analyzer etc. But we also need require so that cljs_deps.js gets proper information for browser, but I haven't yet found a way to retrieve module requires from Closure.

Comment by Juho Teperi [ 12/Dec/17 12:20 PM ]

New patch should now emit goog.provide/require calls and tests pass, and I have tested this with Reagent and npm modules and foreign libs. Works with Closure v20171112 but v20171203 doesn't seem to process CommonJS at all, but doesn't give any errors.

One problem is that optmized build now prints bunch of these warnings:

Dec 12, 2017 8:10:28 PM com.google.javascript.jscomp.PhaseOptimizer$NamedPass process
WARNING: Skipping pass inlineTypeAliases
Dec 12, 2017 8:10:29 PM com.google.javascript.jscomp.PhaseOptimizer$NamedPass process
WARNING: Skipping pass j2clChecksPass

Reagent advanced test build size increased from 1.1M to 1.3M so it is possible (some) optimizations are not being applied.

Comment by Thomas Heller [ 12/Dec/17 12:53 PM ]

I think those warnings only appear when `:language-in` is NOT set. Setting it to `:ecmascript6` now does weird things to CLJS though which may explain your code size increase.

Comment by Juho Teperi [ 12/Dec/17 12:55 PM ]

Hmm, I looked into CompilerOptions and I thought it defaults to EcmaScript2017 (ES8_MODULES featureSet, which should enable everything), but probably I missed something.

Comment by Thomas Heller [ 12/Dec/17 4:56 PM ]

Could be that the default is too high. The warnings went away (in shadow-cljs) when setting :language-in to :ecmascript5 and I didn't look any further since that is fine for CLJS.

Comment by Juho Teperi [ 08/Jan/18 5:18 PM ]

Current patch status:

Latest v20180101 release processes CJS and ES6 OK.

The emitted JS code doesn't work, because all methods exported from CJS (and ES6 default exports) need to be accessed through default property, like:

module$path$node_modules$left_pad$index["default"](...)

module$path$node_modules$react$react["default"].cloneElement(...)

And still seeing warnings about skipping passes.

Comment by Juho Teperi [ 09/Jan/18 4:06 PM ]

Latest version modifies compiler to emit ["default"] when accessing anything in JS modules.

Still getting warnings during optimization. Setting language-in doesn't help.

Comment by Juho Teperi [ 09/Jan/18 4:34 PM ]

-4 now defaults language-in to EcmaScript 5 so all optimization passes get enabled. Optimized output size now matches 1.9.946 when tested with Reagent.

Comment by Juho Teperi [ 10/Jan/18 2:22 PM ]

-5 reads module type from Closure, adds it to new :js-namespaces compiler env property, and uses that on emit* :var to emit ["default"] for only CJS modules.

Comment by David Nolen [ 12/Jan/18 3:18 PM ]

I'm unable to get this patch working with `script/test`. Module processing fails on lodash package.json because we are parsing as JS source. I also modified `script/test` to invoke the new `clj` tool and leverage the new `deps.edn` file so we can use the unshaded Closure Compiler dependency and I observed no change - the same issue persists.

Comment by Juho Teperi [ 08/Feb/18 3:47 PM ]

Updated to february Closure release. script/test works if https://dev.clojure.org/jira/browse/CLJS-2375 has been applied first.

Pr-str test case had to be changed due to https://github.com/google/closure-compiler/commit/179b62cc4770fd6a9eb306d3cf529eb99e992026.

Comment by Juho Teperi [ 09/Feb/18 12:30 PM ]

Rebased

Comment by David Nolen [ 10/Feb/18 6:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/72e2ab6e63b3341aa26abcbdd72dc291cbd0c462





[CLJS-2375] Closure-Compiler: Intent to Remove AMD Module Support Created: 01/Oct/17  Updated: 09/Feb/18  Resolved: 09/Feb/18

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

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

Attachments: Text File CLJS-2375.patch    
Approval: Accepted

 Description   

Hi - I'm a maintainer of Closure-Compiler. The compiler currently has a pass to transform some types of AMD modules to CommonJS so that the CommonJS processing can rewrite the code. It's an old pass and nobody is maintaining it. I'd like to delete it.

I just wanted to check and make sure that Clojure isn't actively using it. If it is, we need to come up with some type of plan.

Thanks,

Chad Killingsworth



 Comments   
Comment by Juho Teperi [ 01/Oct/17 10:54 AM ]

We support AMD modules, but no, Cljs doesn't actively use them.

We'll need to be remove references to some options and classes when these are removed from Closure.

Comment by Juho Teperi [ 08/Feb/18 3:46 PM ]

Removed the AMD module support and test cases.

Comment by David Nolen [ 09/Feb/18 11:46 AM ]

fixed https://github.com/clojure/clojurescript/commit/333dddbfb890f11dc6a21987437552093c5b9511

Comment by Mike Fikes [ 09/Feb/18 5:14 PM ]

See test regression CLJS-2498





[CLJS-2496] PHM Seq and Iter should return MapEntry on nil key case. Created: 09/Feb/18  Updated: 09/Feb/18  Resolved: 09/Feb/18

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

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

Attachments: Text File CLJS-2496.patch    
Patch: Code and Test
Approval: Accepted

 Description   

When a PHM has a nil key, it still returns a PV rather than a ME in both the seq and iteration cases. This was an oversight in CLJS-2456.



 Comments   
Comment by David Nolen [ 09/Feb/18 12:00 PM ]

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





[CLJS-2270] Support AOT compilation of macro namespaces (bootstrap) Created: 24/Jul/17  Updated: 08/Feb/18

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

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

Approval: Vetted

 Comments   
Comment by Andrea Richiardi [ 08/Feb/18 11:18 AM ]

This is great! It is exactly what we were discussing at some point in lumo with Antonio. I am interested in what is the possible approach for solving transitive dependency issues: I really like the npm approach that inlines deps under the parent (each one gets its own).





[CLJS-2491] Inference warnings are not reported Created: 05/Feb/18  Updated: 05/Feb/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.655, 1.9.660, 1.9.671, 1.9.854, 1.9.908
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Oleh Palianytsia Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Consider the following file:

warn_on_infer_test/app.cljs
(ns warn-on-infer-test.app)

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

(defn wrap-baz [x]
  (.baz x))

On current latest version and all versions downwards to 1.9.655 inclusive, I'm not getting any warnings regarding inability to infer the type of x:

> (require '[cljs.build.api :as b])
nil

> (b/build "src"
           {:main 'warn-on-infer-test.app
            :output-to "out/main.js"
            :output-dir "out"
            :optimizations :none
            :infer-externs true})
nil

While on 1.9.562 these warnings are printed:

> (require '[cljs.build.api :as b])
nil

> (b/build "src"
           {:main 'warn-on-infer-test.app
            :output-to "out/main.js"
            :output-dir "out"
            :optimizations :none
            :infer-externs true})
WARNING: Cannot infer target type in expression (. x baz) at line 6 src/cljs/warn_on_infer_test/app.cljs
nil





[CLJS-2488] spec.alpha/conform should return MapEntry for tagged return values Created: 31/Jan/18  Updated: 02/Feb/18  Resolved: 02/Feb/18

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

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

Attachments: Text File CLJS-2488.patch    
Approval: Accepted

 Description   

Currently it returns a PersistentVector which has been specify!'d to include IMapEntry. Now that there is a MapEntry it can be used instead.



 Comments   
Comment by Thomas Mulvaney [ 31/Jan/18 4:27 AM ]

The attached patch also changes the signature of the private tagged-ret function to match that of Clojure version.

Comment by David Nolen [ 02/Feb/18 5:05 AM ]

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





[CLJS-2483] Cannot call browser.repl/connect before document.body exists Created: 25/Jan/18  Updated: 01/Feb/18

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

Type: Defect Priority: Minor
Reporter: Matthew Davidson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: browser-repl, repl
Environment:

Browsers - all



 Description   

(browser.repl/connect) calls (bootstrap), which monkey-patches goog.require to write scripts to js/document.body (line 149).

If you call this from <head>, before <body> exists, you will get an error.

Google Closure handles this situation by writing to head in goog.appendScriptSrcNode_, so perhaps that will work.



 Comments   
Comment by Matthew Davidson [ 26/Jan/18 4:06 PM ]

(Minor point: the version is the oldest version this affects, but it is in fact current as of 1.9.946)

Unfortunately, I can't seem to edit the version after submission.





[CLJS-2490] Nashorn repl cljs.repl.nashorn/eval-str should set the out and err writers to the currently bound *out* and *in* writers. Created: 31/Jan/18  Updated: 31/Jan/18

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

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


 Description   

Currently the instantiation of the Nashorn REPL captures the out and err writers at the time of creation.

Perhaps it would be better to set the engines writers to the "currently bound" values of out and err before each eval.

This would allow capturing eval output.






[CLJS-2481] Enhance CLJS module build times by skipping recompilation / regeneration of modules with no source changes Created: 25/Jan/18  Updated: 31/Jan/18

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

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


 Description   

I am trying to improve my auto build times for an use case were we cannot use optimizations:none (it is a chrome extension and chrome's security policy prohibits dynamic loading JS for the cases where it is injected into web pages). So when using the next quickest optimization optimizations:whitespace it was taking a lot of time (~30sec for even small change) as the app pulls in a lot of dependencies. I tried to use the new released modules feature to split the output into 2 files one with all the dependencies and the other with my own code and observed that clojurescript compiler does not skip re-generation of modules where the source files have not changed at all.

Digging into the code it looks like it is a minor change in the closure.clj in the function in the optimize-modules function where we could skip emitting the module whose all the source files are not changed at all.



 Comments   
Comment by Deepankar [ 25/Jan/18 1:54 AM ]

Hi,
I was unsure about the community's best practices around issues and hence created a JIRA, if you want me to do a preliminary discussion about this in clojurescript google group, I will move this to the google group.

Thank you

Comment by Thomas Heller [ 25/Jan/18 5:18 AM ]

:modules via the Closure Compiler only work when used on the entire program. It is not possible to feed in partial code and re-use files from another compile run. The compiler will otherwise inject a conflicting goog/base.js and maybe other things.

It is however easily possible to combine the output without the Closure Compiler at all. Just concatenate all sources from a given module into a single file without any processing at all. Unfortunately there is no easy hook for this in CLJS.

Comment by Deepankar [ 25/Jan/18 11:27 AM ]

Thomas Heller Yeah I agree, what I observed is from the timestamps of the files generated is that, the closure compiler is not the step that is taking time, rather generating the source-mappings is, if we disable the source-mappings we cut down the time to ~ 5 sec (can't do without them during development)and observing the timestamps of the generated files for the cljs-base module after the js file is generated the js.map file generation was taking around more than 15 sec of the overall 30 sec compilation time. It is this part I wanted to see if we can skip

Comment by Deepankar [ 31/Jan/18 3:44 AM ]

Thomas Heller any comments on the above ?

Comment by Thomas Heller [ 31/Jan/18 3:53 AM ]

Source maps should not take that much time. What kind of hardware/OS are you on? Are you maybe writing to a network mounted drive or something similar? Source maps can get quite big so it may be slow IO?

Comment by Deepankar [ 31/Jan/18 10:24 AM ]

This is a normal 2013 Macbook pro, not NAS involved. The size of the generated source map is around 6MB, not too big for a SSD, let me see if I can profile a build and get you something that is the more concrete about the bottlenecks.





[CLJS-2489] empty should return nil for non-IEmptyableCollection values Created: 31/Jan/18  Updated: 31/Jan/18  Resolved: 31/Jan/18

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: Declined Votes: 0
Labels: None

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

 Description   

It appears that the semantics of Clojure's empty is that nil is returned for values which do not support clojure.lang.IPersistentCollection's empty method.

Presuming these semantics, ClojureScript's current semantic mismatch could be addressed via a definition along the lines of

(defn empty [coll] 
  (when (satisfies? IEmptyableCollection coll) 
    (-empty coll)))

Then programs like (empty "abc") would match Clojure, while also supporting client code that does things like:

(extend-type array 
  IEmptyableCollection 
  (-empty [coll] #js []))


 Comments   
Comment by Mike Fikes [ 31/Jan/18 9:46 AM ]

If approved, CLJS-2489.patch would address the issue with an implementation employing the implements? ^not-native / native-satisfies? pattern instead of satisfies?.

Comment by Mike Fikes [ 31/Jan/18 9:56 AM ]

The attached patch fails to match Clojure behavior for records, viz:

user=> (defrecord Foo [a b])
user.Foo
user=> (empty (->Foo 1 2))
UnsupportedOperationException Can't create empty: user.Foo  user.Foo (NO_SOURCE_FILE:1)
cljs.user=> (defrecord Foo [a b])
cljs.user/Foo
cljs.user=> (empty (->Foo 1 2))
nil
Comment by Mike Fikes [ 31/Jan/18 10:17 AM ]

I'm going to retract this for now:

Alex Miller speculated that the spec for empty might be (s/fdef clojure.core/empty :args (s/cat :c (s/nilable coll?)) :ret (s/nilable coll?)).

If that ends up being the case then programs like (empty "abc") fail to meet the spec. In other words, the semantics presumed in the ticket description would be off, and it seems premature to attempt to revise the implementation IMHO.





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

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: Declined Votes: 0
Labels: None
Environment:

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


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

 Description   

If I evaluate

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

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

(lldb) bt

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

(Full stack trace attached as stacktrace.txt)

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

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



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

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

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

The construct

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

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

In r2014, the emitted JavaScript is:

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

while in r2024 the emitted JavaScript is:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

This JavaScriptCore perf problem is easily reproduced purely with JavaScript:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

For example:

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

Or maybe cheat and emit a ChunkedCons instead?

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

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

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

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

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

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

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

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

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

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

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

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

Activity is occurring with the WebKit ticket:

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

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

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

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

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

Mike this is true only for cljs.core.

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

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

Comment by Timothy Pratley [ 24/Jul/15 8:38 AM ]

Just for reference: https://github.com/google/closure-compiler/issues/1049

Comment by David Nolen [ 10/Nov/15 9:57 AM ]

this is an upstream bug that we are not going to fix that has known solutions, closing for now.

Comment by Mike Fikes [ 13/Nov/15 6:41 AM ]

Collected summary:

For certain forms like (list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18), the generated JavaScript under :none involves nested call constructs provoking a known upstream O(2^n) perf issue in JavaScriptCore:
https://bugs.webkit.org/show_bug.cgi?id=139847

This can be worked around by passing the compiler option :static-fns true

Comment by Yehonathan Sharvit [ 14/Apr/16 1:23 AM ]

Why not making :static-fns true by default?

Comment by Mike Fikes [ 18/Apr/17 7:10 AM ]

Patch has landed upstream https://trac.webkit.org/changeset/215453/webkit and associated ticket closed as fixed.

Comment by Mike Fikes [ 08/Jun/17 9:57 PM ]

Confirmed with WebKit Nightly WebKit-SVN-r217930 that the issue is indeed resolved for ClojureScript.

This form evaluates instantly:

(list 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30)

Tested the above form using http://clojurescript.io directly in WebKit Nightly and in Planck (by setting DYLD_FRAMEWORK_PATH per https://clojurescript.org/community/running-tests)

Comment by Mike Fikes [ 30/Jan/18 12:25 PM ]

Confirmed that this issue is resolved with the version of JavaScriptCore that ships with iOS 11.2.5. (It is not fixed on iOS 10.3.3).

Also confirmed that this issue is resolved with the version of JavaScriptCore that ships with macOS 10.13.3.





[CLJS-2484] Need rseq for map entry Created: 26/Jan/18  Updated: 28/Jan/18  Resolved: 28/Jan/18

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

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

Attachments: Text File CLJS-2484.patch    
Patch: Code and Test
Approval: Accepted

 Description   

(rseq (first {:a 1})) fails with Error: No protocol method IReversible.-rseq defined for type cljs.core/MapEntry



 Comments   
Comment by Mike Fikes [ 26/Jan/18 9:09 PM ]

Also, same is true for sorted maps:

(rseq (first (sorted-map :a 1)))
No protocol method IReversible.-rseq defined for type cljs.core/BlackNode
Comment by Mike Fikes [ 26/Jan/18 9:26 PM ]

Note: Strictly speaking, Tthe RedNode / BlackNode defect exists in currently shipping versions of ClojureScript, and while the MapEntry defect also exists, it is not yet surfaced via the (rseq (first {:a 1})) example because maps do not yet return map entries in the shipping ClojureScript.

Comment by David Nolen [ 28/Jan/18 9:32 AM ]

fixed https://github.com/clojure/clojurescript/commit/91431bd556f7a11db59319fcc082737a448f651e





[CLJS-2460] Print MapEntry as vector Created: 08/Jan/18  Updated: 28/Jan/18  Resolved: 28/Jan/18

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: print, printing

Attachments: Text File CLJS-2460-2.patch     Text File CLJS-2460.patch    
Patch: Code and Test
Approval: Accepted

 Description   

(pr-str (->MapEntry :a 1 nil)) yields {{"#object[cljs.core.MapEntry]"}} instead of "[:a 1]".



 Comments   
Comment by Mike Fikes [ 08/Jan/18 8:25 AM ]

The attached patch also tests that printing matches Clojure in the case that *print-length* happens to be less than 2.

Comment by Mike Fikes [ 25/Jan/18 9:42 AM ]

The attached CLJS-2460-2.patch contains the same content but moves the test code around a little so that the patch doesn't conflict with other map entry patches.

Comment by David Nolen [ 28/Jan/18 9:16 AM ]

fixed https://github.com/clojure/clojurescript/commit/15343585996c1266c15bc54d20c176f8a7b789c7





[CLJS-2482] Return MapEntry for seq on defrecord Created: 25/Jan/18  Updated: 28/Jan/18  Resolved: 26/Jan/18

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

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

Attachments: Text File CLJS-2482.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Related to CLJS-2456: We need to have a seq on a defrecord return map entry instances.



 Comments   
Comment by David Nolen [ 26/Jan/18 12:48 PM ]

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





[CLJS-2478] map-entry? should return true for sorted-map entries. Created: 24/Jan/18  Updated: 28/Jan/18  Resolved: 28/Jan/18

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-2478-2.patch     Text File CLJS-2478-3.patch     Text File CLJS-2478.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Sorted maps provided by PersistentTreeMap have there own entry types RedNode and BlackNode. map-entry? should return true when given one.

For example (map-entry? (first (sorted-map :a 1))) should return true.



 Comments   
Comment by Thomas Mulvaney [ 24/Jan/18 5:21 AM ]

The attached patch introduces a new marker protocol AMapEntry which map-entry? uses to identify map entries. By using a marker protocol, new or user defined map entry types could participate in map-entry? checks.

Comment by Thomas Mulvaney [ 24/Jan/18 5:48 AM ]

Actually a better approach might be to wait for CLJS-2456 and then remove IMapEntry from vectors as they will no longer be used as a substitute. map-entry? could then just check if the its argument implements IMapEntry.

Comment by Mike Fikes [ 25/Jan/18 9:29 AM ]

Yes, per Thomas's comment above, this approach would cause (map-entry? [:a 1]) to return false, consistent with Clojure.

The attached CLJS-2478-2.patch pursues that approach, fixing up a few areas where vectors are treated as map entries, and ensuring that -conj! can work with vectors (needed for programs like (into (hash-map :a 1) [[:b 2] [:c 3]]).

This patch is designed to be applied after the patches in CLJS-2456 and CLJS-2482.

An interesting consequence is that this change makes treating vectors as map entries no longer work as is so in Clojure, as in programs like (key [:a 1]), or (val [:a 1]), but code like this was never correct in the first place. I suspect that this particular aspect of the change would ferret out some incorrect code in the wild.

Comment by Mike Fikes [ 26/Jan/18 2:32 PM ]

CLJS-2478-3.patch is identical to CLJS-2478-2.patch except that it replaces the satisfies? call in the map-entry? predicate with implements?.

Comment by David Nolen [ 28/Jan/18 8:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/620ac16d93a91786e1c636fac1269ed66087aa03





[CLJS-2485] The macro cljs.analyzer/no-warn triggers a WARNING when compiled in self-host Created: 27/Jan/18  Updated: 27/Jan/18  Resolved: 27/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None

Attachments: Text File CLJS-2485-0.patch    

 Description   

This bug report is more of a descriptive one and for sure probably low priority but highlights maybe some need for refactoring of some functionality from cljs.analyzer.
I am very open to better approaches.

It all starts from compiling a piece of code of mine that uses the following in its body:

(ns serverless-lumo.build
  (require [cljs.analyzer :as ana]

(defn exit-on-warning!
  [warning-type env extra]
  (when (warning-type ana/*cljs-warnings*)
    (when-let [s (ana/error-message warning-type extra)]
      (binding [*print-fn* *print-err-fn*]
        (println "lumo error:" (ana/message env s)))
      (lumo.core/exit 1))))

When I compile the above with lumo, which has now a self-hosted compiler, I get:

WARNING: Use of undeclared Var cljs.analyzer/no-warn at line 855 
WARNING: Use of undeclared Var cljs.analyzer/no-warn at line 873 
WARNING: Use of undeclared Var cljs.analyzer/no-warn at line 2495 
WARNING: Use of undeclared Var cljs.analyzer/no-warn at line 2516 
WARNING: Use of undeclared Var cljs.analyzer/no-warn at line 2536

This is because the no-warn function is refer-ed but included in a :clj conditional.

The patch shows what solves for self-host. Does it break the JVM compiler though? All tests pass locally.

A good solution would be to refactor the warning helpers out of cljs.analyzer, so that client code can use them without requiring it.



 Comments   
Comment by Andrea Richiardi [ 27/Jan/18 9:14 PM ]

Attached a patch just as example, not intended for merging. Please advice on how to proceed.

Comment by Mike Fikes [ 27/Jan/18 9:27 PM ]

A minimal repro will be needed without any downstream tooling. If this is a self-hosted issue, see https://clojurescript.org/community/reporting-bootstrap-issues

Having said that, there already is a version of no-warn meant for self-hosted use.

The one modified by your patch needs to be conditionalized for :clj because only on Clojure can you define a macro and use it directly in the same namespace.

The one for self-hosted is referred here https://github.com/clojure/clojurescript/blob/47cd1cef8f7ed5b16cffc7e2ae4d223328091ab7/src/main/clojure/cljs/analyzer.cljc#L14

The definition is here https://github.com/clojure/clojurescript/blob/47cd1cef8f7ed5b16cffc7e2ae4d223328091ab7/src/main/clojure/cljs/analyzer/macros.clj#L16

Comment by Andrea Richiardi [ 27/Jan/18 9:30 PM ]

Yeah the premise is that probably this is so cutting edge that nobody will ever look at it. So first of all thanks!

No downstream tooling is a difficult task to achieve given that lumo uses the GCC compiler port to Javascript.

Comment by Mike Fikes [ 27/Jan/18 9:40 PM ]

So, by cutting edge, you might be referring to the ability of self-hosted to actually compile the compiler (2nd stage). Right, that is not possible.

But in this case, I would recommend making sure that *load-fn* first loads artifacts that were produced by JVM ClojureScript. (In other words, completely avoid having self-hosted ClojureScript attempt to compile the cljs.analyzer namespace by ensuring that *load-fn* doesn't return the source for that namespace, but instead returns the compiled JavaScript along with its analysis cache.)

I suppose on the other hand, by cutting edge you are referring to Lumo's running the compiler. Yeah, perhaps in that case there is no *load-fn*, and everything is cutting edge and it will be impossible to create minimal repros using cljs.js because that's not what Lumo is even making use of here.

Comment by Andrea Richiardi [ 27/Jan/18 10:27 PM ]

In any case thanks Mike, I will need to explore more myself. According to your references above though, this is a non-issue for now and I am going to close it. Sorry for the noise.





[CLJS-2477] empty map entry should be nil Created: 23/Jan/18  Updated: 26/Jan/18  Resolved: 26/Jan/18

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

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

Attachments: Text File CLJS-2477-2.patch     Text File CLJS-2477-3.patch     Text File CLJS-2477.patch    
Patch: Code and Test

 Description   

With CLJS-2456, (empty (first {:a 1})) should return nil, not []. (This is because map entries must have 2 elements.)

To repro without CLJS-2456: (empty (->MapEntry 1 2 nil)).



 Comments   
Comment by Mike Fikes [ 23/Jan/18 2:50 PM ]

Note that in the patch, the (generic) test case is special-cased for MapEntry.

Comment by Thomas Mulvaney [ 24/Jan/18 2:22 AM ]

It looks like the sorted-maps entries, `RedNode` & `BlackNode` need their respective `-empty` methods updated as well.

Comment by Thomas Mulvaney [ 24/Jan/18 4:50 AM ]

To expand on the above, the test case provided in the patch currently passes because map-entry? only returns true for MapEntry which has had its -empty method updated in the patch. But, map-entry? should actually return true for {Red,Black}Node as well. So all those types should be updated to have -empty return nil.

I've opened CLJS-2478 for sorting out the map-entry? check on the other entry types.

Comment by Mike Fikes [ 24/Jan/18 9:37 PM ]

Thanks @tmulvaney for the review!

Patch 2 addresses the fact that the sorted map entries (RedNode, BlackNode) also need to return nil in their -empty implementations. The test case has been revised to no longer special case on map-entry? but to instead special case on (instance? PersistentVector e). (This is consistent with the potential strategy outlined in CLJS-2478, and if carried out, this special casing, as well as the test for PersistentVector can be removed entirely when done.)

This also addresses a consequence of nil for empty on map entries: since clojure.walk/walk employs empty, it needs to be updated to handle map entries. The case added is a port of that case in the Clojure implementation of clojure.walk/walk.

Comment by Mike Fikes [ 25/Jan/18 10:09 AM ]

The attached CLJS-24773.patch fixes a defect in handling the case when a map entry is passed to js>clj (without the extra case, empty is applied to the map entry in the coll? case, thus producing nil and creating a reversed list with the converted key and value instead of a map entry with the converted key and value.

Comment by David Nolen [ 26/Jan/18 12:41 PM ]

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





[CLJS-2456] Have map types return MapEntry instead of vector Created: 03/Jan/18  Updated: 26/Jan/18  Resolved: 26/Jan/18

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

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

Attachments: Text File CLJS-2456.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Groundwork for this has been done.

CLJS-2013 adds the MapEntry type, but indicates "PAMs and PHMs don't emit them when you seq or iterate over them yet."
CLJS-2068 fixes an issue with MapEntry
CLJS-2001 adds a map-entry? predicate

So this ticket entails updating the PHM and PAM implementations so that, for example, (map-entry? (first {:a 1})) evaluates to true.

Note: One additional bit of groundwork that should be done prior to this ticket is CLJS-2460.



 Comments   
Comment by Thomas Mulvaney [ 06/Jan/18 6:46 PM ]

I've attached a patch for consideration. Besides updating `Seq` and `Iterator` types for PAM and PHM, it also updates implementers of `IFind` to return `MapEntry` rather than vectors. I can move the `IFind` changes into another patch though if that is prefered.

Comment by Mike Fikes [ 23/Jan/18 2:18 PM ]

LGTM

Comment by David Nolen [ 26/Jan/18 12:27 PM ]

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





[CLJS-2479] compare should accept objects with valueOf function Created: 24/Jan/18  Updated: 25/Jan/18

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

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

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

 Description   

Currently compare (for any non-CLJS IComparable) types only accepts primitives (numbers, bool, string, array) and forwards them to goog.array.defaultCompare (which is just a {{ return a > b ? 1 : a < b ? -1 : 0 }} ).

Though, the standard defines anything with a valueOf can be compared:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comparison_Operators#Relational_operators

since valueOf should return one of these primitves. Ie this works:

(let [x0 #js{:x 1, :valueOf #(-> 1)}
      x1 #js{:x 2, :valueOf #(-> 2)}
      dc garr/defaultCompare] 
  (list (dc x0 x1) (dc x1 x0) (dc x1 x1)))

We should allow this, ie. instead of:

:else
(if (and (or (string? x) (array? x) (true? x) (false? x))
         (identical? (type x) (type y)))
     (garray/defaultCompare x y)
     (throw (js/Error. (str "Cannot compare " x " to " y))))

do a:

(if (and (or (string? x) (array? x) (true? x) (false? x)
             (and (js-in "valueOf" x) (js-in "valueOf" y)))
         (identical? (type x) (type y)))
      (garr/defaultCompare x y)
      (throw (js/Error. (str "Cannot compare " x " to " y))))

We should also give js-in a boolean tag.






[CLJS-2480] Periods at end of analyzer warnings Created: 24/Jan/18  Updated: 24/Jan/18

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

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

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

 Description   

Analyzer warnings have "at line xxx" suffixed, and therefore are not standalone sentences. For example:

cljs.user=> (fn ([]) ([]))
WARNING: : Can't have 2 overloads with same arity at line 1 <cljs repl>

There are a couple of warnings that have periods and thus look odd:

cljs.user=> (+ 1 "a")
WARNING: cljs.core/+, all arguments must be numbers, got [number string] instead. at line 1 <cljs repl>
cljs.user=> (defn ^:deprecated foo [])
#'cljs.user/foo
cljs.user=> (foo)
WARNING: cljs.user/foo is deprecated. at line 1 <cljs repl>





[CLJS-2469] ChunkedCons chunk-next returns empty seq instead of nil Created: 13/Jan/18  Updated: 21/Jan/18

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

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

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

 Description   

There is a bug in ChunkedCons. In Clojure ChunkedCons (correctly) always calls seq in chunked-next. In CLJS it's not done. But since ChunkedCons has to be lazy it pretty much always gets an (empty) lazy seq as the "more" part.

Bug:

(-> (map inc (vec (range 64)))
    seq
    chunk-next
    seq
    chunk-next)

Returns and empty sequence instead of nil. This hasn't surfaced yet since nothing calls chunk-next on a ChunkedCons (yet).



 Comments   
Comment by A. R [ 13/Jan/18 7:26 AM ]

Found another bug that surfaced: Current implementation calls -seq on more which could be nil and this would blow up. So the patch also make a tiny change to -next also just calling seq on more. Pretty straight forward.

Comment by David Nolen [ 19/Jan/18 3:20 PM ]

This patch needs a test.

Comment by A. R [ 20/Jan/18 12:27 AM ]

Test's added.





[CLJS-1800] Defer to tools.reader for cljs.reader functionality Created: 30/Sep/16  Updated: 20/Jan/18  Resolved: 19/Jan/18

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

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

Approval: Vetted

 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

Comment by David Nolen [ 08/Jul/17 4:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/07ee2250af02b25f232111890c0f40f23150768d

Comment by Sam Umbach [ 12/Jan/18 9:18 PM ]

I believe this was fixed in the cljs 1.9.854 release but the "Fix Version/s" currently shows 1.9.671. I don't have the power to fix the metadata on this JIRA, but I'm sure someone does... @dnolen?

Comment by Sam Umbach [ 20/Jan/18 2:12 PM ]

Thanks, David!!





[CLJS-2171] Non deterministic compilation failure Created: 05/Jul/17  Updated: 19/Jan/18  Resolved: 19/Jan/18

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: David Nolen
Resolution: Completed Votes: 2
Labels: None

Attachments: Text File CLJS-2171-2.patch     Text File CLJS-2171.patch    
Patch: Code
Approval: Accepted

 Description   

Since updating to 1.9.655, we're randomly seeing exception thrown during build, using the following compiler options:

:optimizations :advanced
:infer-externs true
:cache-analysis true
:parallel-build true
:recompile-dependents false

The stracktrace is as follows:

10:50:04 Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs {:file #object[java.io.File 0x697fc544 "target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs"]}, compiling:(/tmp/form-init936318712789593796.clj:1:72)
10:50:04 	at clojure.lang.Compiler.load(Compiler.java:7441)
10:50:04 	at clojure.lang.Compiler.loadFile(Compiler.java:7367)
10:50:04 	at clojure.main$load_script.invokeStatic(main.clj:277)
10:50:04 	at clojure.main$init_opt.invokeStatic(main.clj:279)
10:50:04 	at clojure.main$init_opt.invoke(main.clj:279)
10:50:04 	at clojure.main$initialize.invokeStatic(main.clj:310)
10:50:04 	at clojure.main$null_opt.invokeStatic(main.clj:344)
10:50:04 	at clojure.main$null_opt.invoke(main.clj:341)
10:50:04 	at clojure.main$main.invokeStatic(main.clj:423)
10:50:04 	at clojure.main$main.doInvoke(main.clj:386)
10:50:04 	at clojure.lang.RestFn.applyTo(RestFn.java:137)
10:50:04 	at clojure.lang.Var.applyTo(Var.java:702)
10:50:04 	at clojure.main.main(main.java:37)
10:50:04 Caused by: clojure.lang.ExceptionInfo: failed compiling file:target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs {:file #object[java.io.File 0x697fc544 "target/dist/public/s/static/js/auth/deps/cljs/core/async/impl/buffers.cljs"]}
10:50:04 	at clojure.core$ex_info.invokeStatic(core.clj:4725)
10:50:04 	at clojure.core$ex_info.invoke(core.clj:4725)
10:50:04 	at cljs.compiler$compile_file$fn__6002.invoke(compiler.cljc:1471)
10:50:04 	at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1436)
10:50:04 	at cljs.compiler$compile_file.invoke(compiler.cljc:1412)
10:50:04 	at cljs.closure$compile_file.invokeStatic(closure.clj:497)
10:50:04 	at cljs.closure$compile_file.invoke(closure.clj:488)
10:50:04 	at cljs.closure$eval8017$fn__8018.invoke(closure.clj:566)
10:50:04 	at cljs.closure$eval7953$fn__7954$G__7942__7961.invoke(closure.clj:450)
10:50:04 	at cljs.closure$compile_from_jar.invokeStatic(closure.clj:548)
10:50:04 	at cljs.closure$compile_from_jar.invoke(closure.clj:536)
10:50:04 	at cljs.closure$eval8023$fn__8024.invoke(closure.clj:576)
10:50:04 	at cljs.closure$eval7953$fn__7954$G__7942__7961.invoke(closure.clj:450)
10:50:04 	at cljs.closure$compile_task$fn__8111.invoke(closure.clj:862)
10:50:04 	at cljs.closure$compile_task.invokeStatic(closure.clj:858)
10:50:04 	at cljs.closure$compile_task.invoke(closure.clj:850)
10:50:04 	at cljs.closure$parallel_compile_sources$fn__8121.invoke(closure.clj:889)
10:50:04 	at clojure.lang.AFn.applyToHelper(AFn.java:152)
10:50:04 	at clojure.lang.AFn.applyTo(AFn.java:144)
10:50:04 	at clojure.core$apply.invokeStatic(core.clj:657)
10:50:04 	at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1963)
10:50:04 	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1963)
10:50:04 	at clojure.lang.RestFn.invoke(RestFn.java:425)
10:50:04 	at clojure.lang.AFn.applyToHelper(AFn.java:156)
10:50:04 	at clojure.lang.RestFn.applyTo(RestFn.java:132)
10:50:04 	at clojure.core$apply.invokeStatic(core.clj:661)
10:50:04 	at clojure.core$bound_fn_STAR_$fn__6752.doInvoke(core.clj:1993)
10:50:04 	at clojure.lang.RestFn.invoke(RestFn.java:397)
10:50:04 	at clojure.lang.AFn.run(AFn.java:22)
10:50:04 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
10:50:04 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
10:50:04 	at java.lang.Thread.run(Thread.java:745)
10:50:04 Caused by: java.lang.ClassCastException: clojure.lang.Var$Unbound cannot be cast to java.util.concurrent.Future
10:50:04 	at clojure.core$deref_future.invokeStatic(core.clj:2288)
10:50:04 	at clojure.core$deref.invokeStatic(core.clj:2310)
10:50:04 	at clojure.core$deref.invoke(core.clj:2296)
10:50:04 	at cljs.analyzer$dump_specs.invokeStatic(analyzer.cljc:3620)
10:50:04 	at cljs.analyzer$dump_specs.invoke(analyzer.cljc:3611)
10:50:04 	at cljs.analyzer$write_analysis_cache.invokeStatic(analyzer.cljc:3644)
10:50:04 	at cljs.analyzer$write_analysis_cache.invoke(analyzer.cljc:3639)
10:50:04 	at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1357)
10:50:04 	at cljs.compiler$emit_source.invoke(compiler.cljc:1287)
10:50:04 	at cljs.compiler$compile_file_STAR_$fn__5979.invoke(compiler.cljc:1381)
10:50:04 	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1206)
10:50:04 	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1195)
10:50:04 	at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1370)
10:50:04 	at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1363)
10:50:04 	at cljs.compiler$compile_file$fn__6002.invoke(compiler.cljc:1459)
10:50:04 	... 29 more

The error originates from this line https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L3697



 Comments   
Comment by Mike Fikes [ 19/Jan/18 2:28 PM ]

This seems to be a race between using Vars in the cljs.spec.alpha namespace and the namespace being initialized when compiling with parallel build true.

To see that this can happen, define a function

(defn try-it []
  (try
    (when-let [spec-ns (find-ns 'cljs.spec.alpha)]
      @@(ns-resolve spec-ns 'registry-ref))
    (catch Throwable t
      (.printStackTrace t)
      false)))

and start calling it in a tight loop in another thread

(.start (Thread. #(while (not (try-it)))))

Then

(require 'cljs.spec.alpha)

this can result in

java.lang.NullPointerException
	at clojure.core$deref_future.invokeStatic(core.clj:2208)
	at clojure.core$deref.invokeStatic(core.clj:2228)
	at clojure.core$deref.invoke(core.clj:2214)
	at user$try_it.invokeStatic(NO_SOURCE_FILE:3)
	at user$try_it.invoke(NO_SOURCE_FILE:1)
	at user$eval3$fn__4.invoke(NO_SOURCE_FILE:8)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.lang.Thread.run(Thread.java:748)

followed by a repro of the Unbound issue:

java.lang.ClassCastException: clojure.lang.Var$Unbound cannot be cast to java.util.concurrent.Future
	at clojure.core$deref_future.invokeStatic(core.clj:2206)
	at clojure.core$deref.invokeStatic(core.clj:2228)
	at clojure.core$deref.invoke(core.clj:2214)
	at user$try_it.invokeStatic(NO_SOURCE_FILE:3)
	at user$try_it.invoke(NO_SOURCE_FILE:1)
	at user$eval3$fn__4.invoke(NO_SOURCE_FILE:8)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.lang.Thread.run(Thread.java:748)

The race can be avoided by polling. Define

(defn- polling-ns-resolve
  "Repeatedly calls ns-resolve until the var returned is non-nil and bound,
  waiting between calls by supplied delay."
  [ns sym delay]
  (let [var (ns-resolve ns sym)]
    (if (and var (bound? var))
      var
      (do
        (Thread/sleep delay)
        (recur ns sym delay)))))

and with this there is no issue

(defn try-it' []
  (try
    (when-let [spec-ns (find-ns 'cljs.spec.alpha)]
      @@(polling-ns-resolve spec-ns 'registry-ref 10))
    (catch Throwable t
      (.printStackTrace t)
      false)))

(.start (Thread. #(while (not (try-it')))))

(require 'cljs.spec.alpha)

The attached patch adds a polling-ns-resolve and calls it at the problematic call site as a presumptive fix.

Comment by Mike Fikes [ 19/Jan/18 2:39 PM ]

Note: Dan Sutton reported in #clojurescript Slack that they saw a build failure with a similar stack trace and that they are using parallel build and that he recently added the first use of core spec alpha in their codebase.

Comment by David Nolen [ 19/Jan/18 3:16 PM ]

Let's try a patch with `load-mutex` which allows us to lock around requires. It seems to me it might work sinc this looks like a race where the ns object is present but no vars have been bound yet.

Comment by Mike Fikes [ 19/Jan/18 3:30 PM ]

I agree, this is much simpler and seems like it would work. In an attempt to avoid contention, the attached CLJS-2171-2.patch acquires the mutex only after it is known that the cljs.spec.alpha has been (or is being) loaded. Importantly, the ns-resolve calls only occur after the mutex has been acquired. Unable to thoroughly test this apart from running unit tests and trying it on a few projects (which I plan to do).

Comment by Mike Fikes [ 19/Jan/18 3:43 PM ]

I've tried this on two downstream projects that make use of spec with :parallel-build true with the 2nd patch, without any obvious issues (the compiler didn't lock up, etc.)

Comment by David Nolen [ 19/Jan/18 3:58 PM ]

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





[CLJS-2473] Infer character literals to have string type Created: 16/Jan/18  Updated: 19/Jan/18  Resolved: 19/Jan/18

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

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

Attachments: Text File CLJS-2473.patch    
Patch: Code and Test
Approval: Accepted

 Description   

In JVM ClojureScript, (+ \a 1) will not trigger an analysis error, but this will under self-hosted ClojureScript owing to the fact that character literals are read in as strings before analysis.

By revising the analyzer to include an additional (instance? Character form) in analyze-form we can improve type inference for JVM ClojureScript.



 Comments   
Comment by A. R [ 17/Jan/18 1:20 AM ]

By coercing the character to to a string afterwards we could also solve CLJS-2087 :

#{"a" \a}
Comment by David Nolen [ 19/Jan/18 3:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/56da266bcd5357e437f12d4b36e41b0fbaae230f





[CLJS-2395] (str (goog.date.Date. 2017 11 8)) behaves differently with no optimizations than with :simple or :advanced Created: 08/Nov/17  Updated: 19/Jan/18

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

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

Attachments: Zip Archive cljs-goog-date-str-bug.zip    

 Description   

With :optimizations :none, (str (goog.date.Date. 2017 11 8)) returns 20171208, but with :simple or :advanced, it returns 1512684000000.

I attached a simple test case with a README.



 Comments   
Comment by A. R [ 08/Nov/17 6:41 AM ]

Closure compiler changed at some point:

$cljs$core$str$$.$cljs$core$IFn$_invoke$arity$1$ = function $Qd($x$jscomp$279$$) {
  return null == $x$jscomp$279$$ ? "" : "" + $x$jscomp$279$$;
};

It used to keep our [x].join("") code.

Workaround could be to emit a [x, ""].join("")

See CLJS-890 for details

Comment by A. R [ 19/Jan/18 1:05 AM ]

Reported it a bit ago: https://github.com/google/closure-compiler/issues/2782

They're now rewriting x.join("") to String:

https://github.com/google/closure-compiler/commit/b6973ec7b37f3890c8b0e11456afa95d79aaffab

which would also fix CLJS-1631 .





[CLJS-2344] de-dupe externs Created: 31/Aug/17  Updated: 16/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Sameer Rahmani
Resolution: Unresolved Votes: 1
Labels: closure

Attachments: Text File CLJS-2344.patch    
Patch: Code
Approval: Vetted

 Description   

We could possibly call distinct on externs to remove dupes in the common case where externs may appear multiple times accidentally on the classpath.



 Comments   
Comment by David Nolen [ 16/Jan/18 8:09 AM ]

I don't think we want to silently dedupe. We probably want to also warn so users can fix the issue. The only reason to even do this ticket is because the Closure warning is so unfriendly and fails the build.

Comment by Sameer Rahmani [ 16/Jan/18 8:33 AM ]

got it, I'll improve the patch





[CLJS-2472] ES6 Iterators should use IIterable if possible Created: 15/Jan/18  Updated: 15/Jan/18

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

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


 Description   

Motivation:

ES6 Iterables: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

Use in React: https://github.com/facebook/react/blob/30dac4e78de02fb427ee82013160ae875128d7a2/packages/react/src/ReactElementValidator.js#L195-L204

(defn es6-iterator**
  [coll]
  (if (implements? IIterable coll)
    (let [it (-iterator ^not-native coll)]
      #js{:next (fn []
                  (if ^boolean (.hasNext it)
                    #js{:value (.next it), :done false}
                    #js{:value nil, :done true}))})
    ;; Fallback to naive first/next iterator:
    (ES6Iterator. (seq coll))))

;; Tests can use round trip:
(es6-iterator-seq (es6-iterator (hash-map 0 1 2 3)))

(defn es6-iter-consume
  [it]
  (while (not (.-done (.next it)))))

(dotimes [_ 3]
  (let [xs (vec (range 3000))
        runs 1000]
    (simple-benchmark []
      (es6-iter-consume (es6-iterator xs)) runs)
    (simple-benchmark []
      (es6-iter-consume (es6-iterator** xs)) runs)))

About a 4x speedup in Chrome. Also note that much less garbage is produced.






[CLJS-2471] ChunkedSeq should implemented ICounted Created: 15/Jan/18  Updated: 15/Jan/18

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

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


 Description   

ChunkedSeq in clojure implements Counted, should do the same in CLJS. Implementation is straight forward.






[CLJS-2470] ArrayChunk.reduce doesn't honor end param Created: 15/Jan/18  Updated: 15/Jan/18

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

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


 Description   

Ex:

(reduce max (array-chunk #js[0 1 2 3] 0 1))
;; => 3

Currently not an issue since end is always arr.length for our usage of ArrayChunk, but since it's a public API users might pass a different end param.

This can easily be fixed after CLJS-2466 which properly implements the reduce for ArrayChunk.






[CLJS-2466] Faster reduce for lazy-seqs Created: 12/Jan/18  Updated: 13/Jan/18

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

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

Attachments: Text File CLJS-2466.patch    

 Description   

Lazy seqs that are built from vectors/chunked-seqs can sometimes take a slow first/next reduce.

Observation:

(def xs (vec (range 3000000)))

(time (reduce max xs)) ;; #1: 130ms, (reference)
(time (reduce max (lazy-cat xs))) ;; #2: 130ms, just as fast
(time (reduce max 0 (lazy-cat xs))) ;; #3: 500ms, 4x slower!!

;; Double them with concat and they're not 2x slower but 10x slower:
(time (reduce max (lazy-cat xs xs))) ;; #4: 1200ms
(time (reduce max 0 (lazy-cat xs xs))) ;; #5: 1200ms

Explanation for #3: The problem is that seq-reduce when called without init will properly call reduce again and take a fast path. With init given it won't ever escape to a faster reduce but will stick to first/next.

Note: In Clojure they scale "properly" (first 3 are ~45ms, last 2 are ~110ms).

The reason is that Clojure properly escapes to a fast path:

https://github.com/clojure/clojure/blob/2b242f943b9a74e753b7ee1b951a8699966ea560/src/clj/clojure/core/protocols.clj#L131-L143

An improved seq-reduce could look like this:

(defn seq-reduce
  ([f coll]
   (if-let [s (seq coll)]
     (reduce f (first s) (next s))
     (f)))
  ([f val coll]
   (loop [val val, coll (seq coll)]
     (if coll
       (if (chunked-seq? coll)
         (if (implements? IReduce coll)
           (reduce f val coll)
           (let [nval (reduce f val (chunk-first coll))]
             (if (reduced? nval)
               @nval
               (recur nval (chunk-next coll)))))
         (let [nval (f val (first coll))]
           (if (reduced? nval)
             @nval
             (recur nval (next coll)))))
       val))))

This reduces the times for #3: 160ms, #4: 380ms, #5: 380ms.

This is an RFC, since I'm not 100% certain about the implemenation. Need to be careful to not blow the stack here...

Questions:
1. Should ChunkedCons implement IReduce? I think so.



 Comments   
Comment by A. R [ 12/Jan/18 1:59 AM ]

Timings with advanced compilation:

CHROME:
"OLD"
#1: "Elapsed time: 21.870000 msecs"
#2: "Elapsed time: 25.485000 msecs"
#3: "Elapsed time: 134.900000 msecs"
#4: "Elapsed time: 317.155000 msecs"
#5: "Elapsed time: 313.225000 msecs"
"NEW"
#1: "Elapsed time: 23.290000 msecs"
#2: "Elapsed time: 19.445000 msecs"
#3: "Elapsed time: 20.075000 msecs"
#4: "Elapsed time: 102.895000 msecs"
#5: "Elapsed time: 100.430000 msecs"
"OLD"
#1: "Elapsed time: 19.585000 msecs"
#2: "Elapsed time: 19.475000 msecs"
#3: "Elapsed time: 87.805000 msecs"
#4: "Elapsed time: 303.340000 msecs"
#5: "Elapsed time: 291.680000 msecs"
"NEW"
#1: "Elapsed time: 20.760000 msecs"
#2: "Elapsed time: 19.690000 msecs"
#3: "Elapsed time: 18.960000 msecs"
#4: "Elapsed time: 101.385000 msecs"
#5: "Elapsed time: 101.290000 msecs"

FIREFOX:

"OLD"
#1: "Elapsed time: 7.880000 msecs"
#2: "Elapsed time: 7.820000 msecs"
#3: "Elapsed time: 69.460000 msecs"
#4: "Elapsed time: 197.800000 msecs"
#5: "Elapsed time: 209.300000 msecs"
"NEW"
#1: "Elapsed time: 7.380000 msecs"
#2: "Elapsed time: 7.360000 msecs"
#3: "Elapsed time: 8.100000 msecs"
#4: "Elapsed time: 110.640000 msecs"
#5: "Elapsed time: 89.960000 msecs"
"OLD"
#1: "Elapsed time: 6.520000 msecs"
#2: "Elapsed time: 7.360000 msecs"
#3: "Elapsed time: 106.920000 msecs"
#4: "Elapsed time: 252.300000 msecs"
#5: "Elapsed time: 249.800000 msecs"
"NEW"
#1: "Elapsed time: 7.360000 msecs"
#2: "Elapsed time: 6.940000 msecs"
#3: "Elapsed time: 6.880000 msecs"
#4: "Elapsed time: 99.380000 msecs"
#5: "Elapsed time: 53.220000 msecs"
Comment by A. R [ 12/Jan/18 2:14 AM ]

Impact on truly (unchunked) lazy-seqs is neglibile (hard to measure due to garbage collection causing a lot of variance).

Comment by A. R [ 12/Jan/18 10:22 AM ]

New ticket for reduce for ChunkedCons: CLJS-2468

Comment by A. R [ 13/Jan/18 7:00 AM ]

Depends on CLJS-2469 for tests to pass. Patch is attached.





[CLJS-2467] Small PVs are never chunked Created: 12/Jan/18  Updated: 13/Jan/18

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

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

Attachments: Text File CLJS-2467.patch    

 Description   

A PersistenVector that has <=32 elements and is seq'ed will return an IndexedSeq. Though IndexedSeq always fails the chunked-seq? check.

This means a:

(def xv (vec (range 32)))
(reduce + (map inc xv))

is about 4x slower than a map over a vector with 33 elements.

Options:
1. Return a ChunkedCons with the "rest" set to nil in PersistentVector.seq()
2. Implement IChunkedSeq for IndexedSeq:

(extend-type IndexedSeq
    IChunkedSeq
    (-chunked-first [x] x)
    (-chunked-rest [x] ())
    IChunkedNext
    (-chunked-next [x] nil)
    IChunk
    (-drop-first [coll]
      (if-some [n (-next coll)]
        n
        (throw (js/Error. "-drop-first of empty chunk")))))

I think option #2 is better since IndexedSeq is used quite a bunch throughout the code base, so the chunking will also kick in for many other code paths.



 Comments   
Comment by A. R [ 12/Jan/18 10:20 AM ]

Note:

This is different from Clojure (which does not consider an IndexedSeq a ChunkedSeq) since we use IndexedSeq a lot more where Clojure often uses a ChunkedSeq. For instance:

(apply (fn [& a] (type a)) [1 2 3 4 8])

will return IndexedSeq in CLJS but ChunkedCons in CLJ.

Since these IndexedSeq's will be passed around wildly in a normal CLJS app it makes sense to extend them as a IChunkedSeq since otherwise all these will never be chunked and get a slow first/next treatment.





[CLJS-2468] Implement reduce for ChunkedCons Created: 12/Jan/18  Updated: 12/Jan/18

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

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


 Description   

Forked from CLJS-2466

Title says it all.






[CLJS-2465] Using var-args as arguments name in variadic function overrides first argument Created: 11/Jan/18  Updated: 11/Jan/18

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

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


 Description   

The following returns (nil "b"):

((fn [& var-args]
   var-args) "a" "b")

var-args should to be shadowed by default.






[CLJS-2463] js calls are mistakenly treated as higher order invokes Created: 10/Jan/18  Updated: 10/Jan/18

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

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


 Description   

Code that invokes js/ a lot (like Sablono) will currently treat the JS invokes as higher order invokes and bind all arguments in a let beforehand. This generates a ton of IIFEs which are all unnecessary.

Fix:

HO-invoke? (and (boolean *cljs-static-fns*)
                        (not fn-var?)
                        (not (js-tag? f))
                        (not kw?)
                        (not (analyzed? f)))


 Comments   
Comment by A. R [ 10/Jan/18 2:11 PM ]

Actually this has a much larger scope. Currently also Math,goog (and more?) are treated like HO invokes. This also affects compile time when using these a lot since it generates a lot of new {{let}}s. Will probably also be affected by JS modules? So we should do a more careful check here to avoid this.

Comment by A. R [ 10/Jan/18 2:45 PM ]

Another one: all-values predicate should also include keyword? check, no reason to bind the argument when passing keywords.

To illustrate what the current code gen looks like:

(defn foooooooo
  [x y]
  (x :fooo :bar)
  (goog/mixin (js/fooo x) (js/bar y))
  (Math/floor (js/fooo x) (js/fooo x))
  (goog.mixin (js/fooo x) (js/fooo x)))

Generated:

foooooooo = (function (x,y){

var G__21625_21633 = cljs.core.cst$kw$fooo;
var G__21626_21634 = cljs.core.cst$kw$bar;
(x.cljs$core$IFn$_invoke$arity$2 ? x.cljs$core$IFn$_invoke$arity$2(G__21625_21633,G__21626_21634) : x(G__21625_21633,G__21626_21634));

var G__21627_21635 = fooo(x);
var G__21628_21636 = bar(y);
goog.mixin(G__21627_21635,G__21628_21636);

var G__21629_21637 = fooo(x);
var G__21630_21638 = fooo(x);
Math.floor(G__21629_21637,G__21630_21638);

var G__21631 = fooo(x);
var G__21632 = fooo(x);
return goog.mixin(G__21631,G__21632);

});

All of them don't need to bind the passed arguments.

Obviously in this simple case GCC will just inline everything and the cost is 0. But if any of these calls happen inside a function, like {{let [x (the-call...] ...}} then an IIFE is created that isn't broken up by GCC.





[CLJS-2464] Directly reducible tree-seq Created: 10/Jan/18  Updated: 10/Jan/18  Resolved: 10/Jan/18

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: Declined Votes: 0
Labels: None

Attachments: Text File CLJS-2464-1.patch    

 Description   

Introduce a directly reducible tree-seq implementation.

In a sense, tree-seq is akin to iterate in that both are sequence generator functions which accept higher order functions that are used to generate the sequence: While iterate takes a single function that operates on the previous value to produce the subsequent value, tree-seq takes two functions, and has some interesting recursive nature that makes it differ from iterate. Additionally, tree-seq can produce infinite as well as finite sequences, while iterate can only produce infinite sequences. Regardless, tree-seq is amenable to similar optimizations that were done in CLJS-2445 for iterate.

This high-level sketch shows how a directly-reducible tree-seq can be built upon the existing directly-reducible iterate. While this code has great perf characteristics, it lacks IPending support, but that can be addressed via a production patch that adds a new TreeSeq deftype.

(defn tree-seq
  [branch? children root]
    (eduction (take-while some?) (map first)
      (iterate (fn [[node pair]]
                 (when-some [[[node' & r] cont] (if (branch? node)
                                                  (if-some [cs (not-empty (children node))]
                                                    [cs pair]
                                                    pair)
                                                  pair)]
                   (if (some? r)
                     [node' [r cont]]
                     [node' cont])))
        [root nil])))


 Comments   
Comment by Mike Fikes [ 10/Jan/18 11:18 AM ]

Attaching CLJS-2464-1.patch for feedback. I'd still like to flesh out the functional tests, and perhaps add more perf tests, especially perf tests that don't exercise the "directly reducible" aspect, in order to ensure no perf regressions.

Here are the results for the two benchmarks that are included:

Speedup summaries:

            V8: 4.4, 9.9
  SpiderMonkey: 2.3, 9.5
JavaScriptCore: 2.8, 6.0

Before:

Benchmarking with V8
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 14381 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 17217 msecs
Benchmarking with SpiderMonkey
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 16855 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 13921 msecs
Benchmarking with JavaScriptCore
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 11075 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 9598 msecs

tree-seq branch

Benchmarking with V8
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 3293 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 1742 msecs
Benchmarking with SpiderMonkey
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 7308 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 1469 msecs
Benchmarking with JavaScriptCore
[], (into [] (tree-seq seq? identity (quote ((1 2 (3)) (4))))), 1000000 runs, 3928 msecs
[], (nth' (tree-seq seq? identity (repeat 10 (repeat 100 [1 2 (repeat 100 3)]))) 1000), 10000 runs, 1610 msecs
Comment by Mike Fikes [ 10/Jan/18 11:48 AM ]

We can't pursue this because the functions passed to tree-seq are not constrained to be side-effect free. In other words, tree-seq is akin to repeatedly and not akin to iterate. The fundamental problem is that you could reduce across a tree-seq and have it change things (via side effects) followed by simply mapping across the tree-seq (realizing it) and get different results. In other words, the fact that branch? and children could have side effects would mean that the suggested patch would cause tree-seq to fail to produce an immutable sequence.





[CLJS-2461] Minor shadowing intricacies Created: 09/Jan/18  Updated: 09/Jan/18

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler


 Description   

Examples that currently fail:

(ns a-b.core)
(defn doit [])

(ns a-b.other
  (:require [a-b.core :as core]))

(let [a_b 2]
  (core/doit)) ;; FAILS, a_b isn't shadowed properly

;; Related:
(let [a_b 1, a-b 2] a_b) ;; Collide, returns 2!
(let [a-b 1, a_b 2] a-b) ;; Collide, returns 2!
(let [goog "2"] (js-obj goog 2) ;; Fails. Overrides goog!

Given the shadowing logic of the compiler is pretty expensive and further dynamically computing the munged name of each "first namespace segment" could potentially slow down the compiler by a factor of 2x (or more) we should probably re-introduce this approach:

https://github.com/clojure/clojurescript/commit/1c71ab3bff27dc4a099b06e122ec3fdf5a2a8ba8

Ie, maintaining a set of first ns segments and do s simple set lookup in shadow-depth. We should probably only store munged names and do the lookup after munging to fix the above bugs. We should also add "goog".






[CLJS-2369] Undefined nameToPath for bootstrap` when using Twitter's Bootstrap (twbs) Created: 24/Sep/17  Updated: 08/Jan/18

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

Type: Defect Priority: Minor
Reporter: Corin Lawson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: npm-deps


 Description   

How to reproduce problem

  1. {{git clone https://github.com/au-phiware/cljsbuild-bootstrap4.git}}
  2. cd cljsbuild-bootstrap4
  3. make CLJS=path/to/cljs.jar
  4. open index.html
  5. Observe console messages.

Expected result

  1. Console should show the following messages:
    Error: Bootstrap dropdown require Popper.js (https://popper.js.org)
    Hello world!
  2. The Bootstrap Alert component should fade from the page resulting in a blank page.
  3. Compiler should produce:
    out/cljs_deps.js
    goog.addDependency("base.js", ['goog'], []);
    goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
    goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
    goog.addDependency("../node_modules/bootstrap/dist/js/bootstrap.js", ['bootstrap'], []); 
    goog.addDependency("../cljsbuild_bootstrap4/core.js", ['cljsbuild_bootstrap4.core'], ['cljs.core', 'bootstrap']);

Actual result

  1. Console shows
    Error: Undefined nameToPath for bootstrap
  2. The Bootstrap Alert component fails to fade from the page and remains on the page.
  3. Compiler produces:
    out/cljs_deps.js
    goog.addDependency("base.js", ['goog'], []);
    goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
    goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
    goog.addDependency("../cljsbuild_bootstrap4/core.js", ['cljsbuild_bootstrap4.core'], ['cljs.core', 'bootstrap']);


 Comments   
Comment by Hendrik Poernama [ 28/Sep/17 7:44 PM ]

I came across the same problem and did some tracing. It looks like 'load-foreign-library' correctly populated the 'provides' key, but 'library-graph-node' failed to get the 'provides'. I also observed that the output of library-graph-node is the one that gets passed to cljs_deps.js

load-foreign-library: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/dist/js/bootstrap.js
provides: ["bootstrap" "bootstrap/dist/js/bootstrap.js" "bootstrap/dist/js/bootstrap"]

load-foreign-library: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/package.json
provides: []

Copying file:/Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/dist/js/bootstrap.js to out/node_modules/bootstrap/dist/js/bootstrap.js

load-library: out/node_modules/bootstrap/dist/js/bootstrap.js

library-graph-node: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/out/node_modules/bootstrap/dist/js/bootstrap.js
{:requires [], :provides [],
:url #object[java.net.URL 0x6a0659ac "file:/<snip>/cljsbuild-bootstrap4/out/node_modules/bootstrap/dist/js/bootstrap.js"],
:closure-lib true, :lib-path "out/node_modules/bootstrap/dist/js/bootstrap.js"}

Comment by Hendrik Poernama [ 29/Sep/17 3:31 AM ]

I am also getting the same error when trying to use 'apollo-client'. 'cljs_deps.js' would be missing the 'whatwg-fetch' dependency.

I suspect this is because 'whatwg-fetch' is a polyfill that does not explicitly export anything. I was able to workaround the issue by appending the following lines to the bottom of '<projectdir>/node_modules/whatwg-fetch/fetch.js'

const whatwg_fetch = 1;
export { whatwg_fetch };

I don't know enough to say that this is the same problem, other than the error message being identical. I have not tried adding the same hack to bootstrap.js

Comment by Derek Chiang [ 08/Jan/18 2:03 AM ]

Getting the same error for `ethereumjs-abi` too.

Log from console:

```
base.js:1357 Uncaught Error: Undefined nameToPath for ethereumjs_abi
at visitNode (base.js:1357)
at visitNode (base.js:1355)
at visitNode (base.js:1355)
at Object.goog.writeScripts_ (base.js:1369)
at Object.goog.require (base.js:706)
at (index):46
```





[CLJS-2459] Compiler should emit warning about namespace and/or non-existent var Created: 05/Jan/18  Updated: 06/Jan/18

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

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

Approval: Vetted

 Description   

Made project as described in quickstart.

hello-world/core.cljs

(ns hello-world.core
  (:require [hello-world.foo]))

(enable-console-print!)

(println "Hello world!")
(hello-world.foo/foo)
;; no warning

hello_world/foo.cljs

(ns hello-world.foo2)
(defn foo []
  (println "o no"))

Note that `foo.cljs` has a namespace name that is not consistent with the file structure.

I expect
1) a warning about `(:require [hello-world.foo])` not being able to find the namespace, and/or
2) a warning about `(hello-world.foo/foo)` being a non-existent var.






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

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

Type: Defect Priority: Minor
Reporter: Jeremy R Sellars Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None
Environment:

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


Attachments: Text File CLJS-1965-failing-tests.patch     Text File cljs-1965_wrap_letfn_statements.patch    
Patch: Code

 Description   

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

one.cljs
(ns hello-world.one)

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

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

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

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



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

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

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

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

Comment by Mike Fikes [ 16/Jun/17 10:54 AM ]

Confirmed that this fixes things downstream in self-hosted ClojureScript (Planck):

Without the patch:

cljs.user=> (require 'hello-world.core)
one => 2
two => 2
nil

With the patch:

cljs.user=> (require 'hello-world.core)
one => 1
two => 2
nil
Comment by daniel sutton [ 29/Dec/17 3:18 PM ]

This doesn't require different namespaces. The bug is that `let-fn` is putting its binding as a global variable.

And easy reproduction is
1. `lein new mies letfn-bug`,
2. update cljs version to `[org.clojure/clojurescript "1.9.946"]`
3. and then

(ns letfn-bug.core
  (:require [clojure.browser.repl :as repl]))

(enable-console-print!)

(letfn [(non-unique-name [] 4)]
  (defn f1 [] (non-unique-name)))

(letfn [(non-unique-name [] 5)]
  (defn f2 [] (non-unique-name)))

(println "should be 4-> " (f1))
(println "should be 5-> " (f2))

then `scripts/repl`.

results in:

cljs.user=> (load-file "letfn_bug/core.cljs")
should be 4->  5
should be 5->  5
nil
cljs.user=>

With the generated js:

// Compiled by ClojureScript 1.9.946 {}
goog.provide('letfn_bug.core');
goog.require('cljs.core');
goog.require('clojure.browser.repl');
cljs.core.enable_console_print_BANG_.call(null);
var non_unique_name = (function letfn_bug$core$non_unique_name(){
return (4);
});
letfn_bug.core.f1 = (function letfn_bug$core$f1(){
return non_unique_name.call(null);
});
var non_unique_name = (function letfn_bug$core$non_unique_name(){
return (5);
});
letfn_bug.core.f2 = (function letfn_bug$core$f2(){
return non_unique_name.call(null);
});
cljs.core.println.call(null,"should be 4-> ",letfn_bug.core.f1.call(null));
cljs.core.println.call(null,"should be 5-> ",letfn_bug.core.f2.call(null));
Comment by David Nolen [ 05/Jan/18 10:42 AM ]

Quick review of the patch, instead of always wrapping in a statement context it might be better if we only did it at the top-level where this is actually a problem.





[CLJS-2455] nth fails on eduction Created: 03/Jan/18  Updated: 04/Jan/18  Resolved: 04/Jan/18

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

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

Attachments: Text File CLJS-2455-2.patch     Text File CLJS-2455.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Clojure:

user=> (nth (eduction [:x]) 0)
:x

ClojureScript:

cljs.user=> (nth (eduction [:x]) 0)
repl:13
throw e__6225__auto__;
^

Error: nth not supported on this type cljs.core/Eduction
    at Function.cljs.core.nth.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Downloads/out/.cljs_node_repl/cljs/core.js:6184:8)
    at cljs$core$nth (/Users/mfikes/Downloads/out/.cljs_node_repl/cljs/core.js:6141:22)
    at repl:1:93
    at repl:9:3
    at repl:14:4
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:152:38)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:227:14)
    at Socket.<anonymous> ([stdin]:49:25)


 Comments   
Comment by Mike Fikes [ 03/Jan/18 11:22 AM ]

Blocked on CLJS-2457.

The attached CLJS-2455 patch fixes the issue by making ClojureScript's nth behave like Clojure's, in particular resorting to coercion via seq for types which satisfy ISequential.

The aspect is blocked is test code that validates that the ClojureScript implementation behaves like Clojure in this case:

user=> (deftype Foo []
clojure.lang.Sequential)
user.Foo
user=> (nth (->Foo) 0)
IllegalArgumentException Don't know how to create ISeq from: user.Foo  clojure.lang.RT.seqFrom (RT.java:550)

The implementation in the patch works fine at the REPL, but owing to CLJS-2457, this assertion

(is (thrown-with-msg? js/Error #".* is not ISeqable" (nth (->Foo2455) 0)))

currently fails when running the tests via script/test or script/test-simple.

Comment by David Nolen [ 03/Jan/18 8:33 PM ]

Looks like this needs a rebase on master now.

Comment by Mike Fikes [ 03/Jan/18 8:36 PM ]

Attached CLJS-2455-2.patch rebases against master.

Comment by David Nolen [ 04/Jan/18 6:58 AM ]

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





[CLJS-2458] MapEntry test is passing wrong number of args Created: 03/Jan/18  Updated: 04/Jan/18  Resolved: 04/Jan/18

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: test

Attachments: Text File CLJS-2458.patch    
Patch: Code and Test
Approval: Accepted

 Description   

When running script/test:

WARNING: Wrong number of args (2) passed to MapEntry at line 1499 src/test/cljs/cljs/core_test.cljs


 Comments   
Comment by David Nolen [ 03/Jan/18 8:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/983b7fd6e17728b5947a398176bc60aa48ed04b8





[CLJS-2457] For non-seqable types, seq evals to object info when running tests Created: 03/Jan/18  Updated: 04/Jan/18  Resolved: 04/Jan/18

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

Attachments: Text File CLJS-2457.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Observing on master and 1.9.946, and as far back as 1.7.28:

(require '[clojure.test :refer [deftest]])

(deftype FooSeq [])

(deftype BarSeq []
  ASeq)

(deftype BazSeq []
  ICounted (-count [_] 0)
  ISequential)

(defprotocol IFooSeq)

(deftype QuuxSeq []
  IFooSeq)

(deftest test-seq
  (prn (seqable? (->FooSeq)))
  (prn (seq (->FooSeq)))
  (prn (seqable? (->BarSeq)))
  (prn (seq (->BarSeq)))
  (prn (seqable? (->BazSeq)))
  (prn (seq (->BazSeq)))
  (prn (seqable? (->QuuxSeq)))
  (prn (seq (->QuuxSeq))))

If you do this in a REPL and call (test-seq) you will get the second test form throwing as expected:

cljs.user=> (test-seq)
false

ERROR in (test-seq) (Error:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[Error Error: [object Object] is not ISeqable]
nil

Note that this also behaves properly if you are in a REPL and put this code in a namespace and load it via require while in the REPL:

Have src/foo/core.cljs containing

(ns foo.core
 (:require [clojure.test :refer [deftest]]))

(deftype FooSeq [])

(deftype BarSeq []
  ASeq)

(deftype BazSeq []
  ICounted (-count [_] 0)
  ISequential)

(defprotocol IFooSeq)

(deftype QuuxSeq []
  IFooSeq)

(deftest test-seq
  (prn (seqable? (->FooSeq)))
  (prn (seq (->FooSeq)))
  (prn (seqable? (->BarSeq)))
  (prn (seq (->BarSeq)))
  (prn (seqable? (->BazSeq)))
  (prn (seq (->BazSeq)))
  (prn (seqable? (->QuuxSeq)))
  (prn (seq (->QuuxSeq))))

and then try it at the REPL:

$ java -cp cljs.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 51614
To quit, type: :cljs/quit
cljs.user=> (require 'foo.core)
nil
cljs.user=> (foo.core/test-seq)
false

ERROR in (test-seq) (Error:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[Error Error: [object Object] is not ISeqable]
nil

If instead you drop the code (apart from the require form) into the bottom of the cljs.core-test namespace (or, the top of cljs.primitives-test) and run script/test-simple or script/test-self-parity you will see some concerning output. Also note the odd true and false return values.

true
()
false
(["cljs$lang$protocol_mask$partition0$" 32] ["cljs$lang$protocol_mask$partition1$" 0])
false
(["cljs$lang$protocol_mask$partition0$" 16777218] ["cljs$lang$protocol_mask$partition1$" 0] ["cljs$core$ICounted$_count$arity$1" #object[Function]])
true
(["cljs$core_test$IFooSeq$" #js {}])

A simpler case is (prn (seq #js {:a 1 :b 2})) which will print

(["a" 1] ["b" 2])
.



 Comments   
Comment by Mike Fikes [ 03/Jan/18 2:27 PM ]

Root cause is this test intermixing with other things https://github.com/clojure/clojurescript/commit/009db3c7dc45d2a99afd5d89720bca7d2047219d

The reason I wrote this ticket was a result of one assertion in CLJS-2455 failing.

Comment by Mike Fikes [ 03/Jan/18 3:18 PM ]

Attached patch moves the manipulation of object to a separate namespace that is loaded last, and makes the manipulation occur at runtime.

Comment by David Nolen [ 03/Jan/18 8:22 PM ]

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





[CLJS-2363] Warn user when a namespace requires itself Created: 16/Sep/17  Updated: 04/Jan/18

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

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


 Description   

When a namespace requires itself, the compilation hangs without an error message.
It would be nice to either throw an error, or at least print a warning message to the user.



 Comments   
Comment by Mike Fikes [ 20/Nov/17 3:28 PM ]

Cannot repro with 1.9.908 nor 1.9.946:

src/itself/core.cljs:

(ns itself.core
  (:require itself.core))
$ java -cp cljs.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 54877
To quit, type: :cljs/quit
cljs.user=> (require 'itself.core)
clojure.lang.ExceptionInfo: Assert failed: Circular dependency detected, cljs.user -> itself.core -> itself.core
Comment by Frozenlock [ 04/Dec/17 11:21 AM ]

I still get the same behavior with 1.9.946.
Tried with figwheel and uberjar.
Both hang without an error message.

Could it be a problem with the toolchain, say Leiningen?

Comment by Mike Fikes [ 04/Dec/17 11:25 AM ]

Yes. With Figwheel, I'd try disabling :autoload (see https://github.com/bhauman/lein-figwheel/wiki/Configuration-Options#client)

If that allows you to isolate it to Figwheel, perhaps this JIRA can be closed.

Comment by Frozenlock [ 03/Jan/18 4:05 PM ]

This wouldn't change much in this case : it's hanging when running Figwheel or Uberjar because it can't complete the cljs compilation.

I've tested with lein-cljsbuild and I have the same issue.
I'll check with the cljsbuild team if the problem is on their side.





[CLJS-2001] Add map-entry? predicate Created: 06/Apr/17  Updated: 03/Jan/18  Resolved: 03/Jan/18

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

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

Attachments: Text File CLJS-2001.1.patch     Text File CLJS-2001.2.patch     Text File CLJS-2001.patch    
Patch: Code and Test
Approval: Accepted

 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

Comment by David Nolen [ 16/Jun/17 9:48 AM ]

Now that we have MapEntry we can do this one correctly.

Comment by Thomas Mulvaney [ 13/Jul/17 5:40 AM ]

Updated patch attached which checks if x is an instance of the recently added MapEntry type.

Comment by Mike Fikes [ 19/Nov/17 7:33 PM ]

Patch no longer applies; needs re-baseline.

Comment by Thomas Mulvaney [ 30/Dec/17 1:46 AM ]

Updated patch attached.

Comment by David Nolen [ 03/Jan/18 9:04 AM ]

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





[CLJS-2454] Inconsistent macro require behaviour Created: 02/Jan/18  Updated: 03/Jan/18

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   

It is my understanding that a namespace has to :require-macros itself to have its macros accessible automatically but this no longer seems to be accurate.

Given this minimal example

;; src/demo/lib.cljs
(ns demo.lib)

;; src/demo/lib.clj
(ns demo.lib)

(defmacro foo [& args]
  `(prn ::macro))

;; src/demo/test.cljs
(ns demo.test
  (:require [demo.lib :as lib]))

(lib/foo 1 2 3)

I would expect a warning for lib/foo since demo.lib has no :require-macros for its macros and the demo.test did not use :include-macros or :refer-macros.

Compiling this via the build API does in fact produce the expected warning but only iff the demo.lib CLJ namespace was not required anywhere else.

WARNING: Use of undeclared Var demo.lib/foo at line 5 src/demo/test.cljs
;; build.clj

(require '[cljs.build.api :as cljs])
;; (require 'demo.lib) ;; requiring this here removes the warning

(cljs/build
  "src"
  {:output-dir "out"
   :optimizations :none
   :verbose true})

Enabling the (require 'demo.lib) in the build.clj causes the warning to disappear and the code uses the macro properly.

Some popular libraries (eg. devcards) do not self-require but still work if the macro file was at least required somewhere else.

Is this intended behaviour or just accidental?



 Comments   
Comment by David Nolen [ 03/Jan/18 8:44 AM ]

It's accidental in a sense - the macro resolution is simple to the point of being a bit naive. I'm not sure if we should do anything about it at this point. It would probably cause quite a bit of breakage and little benefit.





[CLJS-2339] Significant code reload slowdown with :npm-deps Created: 29/Aug/17  Updated: 31/Dec/17  Resolved: 24/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Petter Eriksson Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: performance

Attachments: Text File CLJS-2339-2.patch     Text File CLJS-2339.patch    
Patch: Code
Approval: Accepted

 Description   

After migrating our dependencies from cljsjs to node_modules we noticed that figwheel took a lot longer to reload our code.

I asked in #clojurescript if anyone had an idea of what could be done and @anmonteiro wrote:
@petterik might just be because we're processing all your node dependencies through Closure on every reload feel free to open a JIRA ticket, we could probably employ a caching strategy somewhere

Versions used:
clojurescript "1.9.908"
lein-figwheel "0.5.13"

npm-depm deps:
:react-helmet "5.1.3"
:react "15.6.1"
:react-dom "15.6.1"
as well as some devDependencies in package.json.



 Comments   
Comment by Petter Eriksson [ 29/Aug/17 1:23 PM ]

If needed to repro, here are the additional devDependencies:
"phantomjs": "1.9.19",
"foundation-cli": "2.2.3",
"bower": "1.8.0"

Comment by David Nolen [ 15/Sep/17 3:32 PM ]

This ticket will need more information. It might just be a Figwheel issue.

Comment by Tony Kay [ 17/Sep/17 9:58 PM ]

OK, so I have some additional interesting facts. It does compile and work (where "work" is defined as "renders a thing from the npm module").

cljs: 1.9.908
cljsbuild: 1.1.7 via lein
Heap size: 2g
npm-deps: react, react-dom, and @blueprintjs/core
cljs deps: Om Next
verbose: true

Project is an om-next app, so cljsjs.react is in the classpath

Building the application without the npm deps and no code that refers to them: 46 seconds (CPU time)
Adding the NPM deps (with install true, but no code that uses them) increases this just be the amount of the install: 59 seconds
using the npm deps increases compile time to: 3 minutes 50 seconds

Critically: with verbose on, I can see that om/next.cljs takes about 15s in the first two cases, and 3 minutes in the final one. In other words: the thing that is slow is compiling next.cljc. Nothing else gets slower.

I'm not sure if this is even going to work "right" when it does compile, since I'm not sure how the cljsjs.react and npm react can co-exist (this is where I assume Om Next probably just needs to be made to use npm instead of cljsjs?)

But, since I saw that Petter was pulling in similar deps, he might be hitting a similar problem with cljsjs.react and npm react both being "in scope" so to speak.

When I use it from figwheel, the time between reloads becomes unusable. I assume it is related, but I have no data to back that up.

EDIT: I had missed the new doc on :global-exports. I'm going to try that and add my results.

Comment by Tony Kay [ 17/Sep/17 10:51 PM ]

So, I fixed the build to be "correct" with `:global-exports so that I only have the NPM version of react and react-dom around (excluded cljsjs/react and react-dom from Om Next). The compile time for next.cljc is still about 3 minutes (compared to the "normal" 15s)

BUT, I then removed blueprint from my `:npm-deps` (and usage), but kept react, react-dom, and a use of react (the NPM one) The time to compile next.cljc dropped to about a minute! Still 4x slower than "normal", but 4x faster than with blueprint in the npm deps. It seems that a file using an npm dep see a significant slowdown that is somehow proportional to the amount of total npm deps code "in view".

Obviously, Om Next uses React, but not blueprint. Yet, it's compile time is significantly affected.

What Antonio said about processing them all through Closure certainly sounds relevant. Kind of a let down to go from recent speed-ups in compiler speed to this sudden jolt of a performance hit when we finally get a better interface to libraries

Comment by António Nuno Monteiro [ 17/Sep/17 11:35 PM ]

Patch attached.

Comment by Tony Kay [ 17/Sep/17 11:48 PM ]

That patch fixes the build issue on plain cljsbuild.

Figwheel now starts quickly (first complete compile), but a simple file change on a small project takes 12s to hot code reload, making it pretty unusable.

Comment by António Nuno Monteiro [ 18/Sep/17 12:01 AM ]

So it looks like this is a 2-part problem.

The first one, which my patch solved, is related to CLJS compilation (where we were performing unnecessary computations on every symbol analysis – which slowed down proportionally to the number of JS modules processed).

The second problem is that we process every JS module on every code reload: the solution for this one is implementing a strategy for figuring out if we need to process JS modules again (e.g. based on last modified timestamps of source files, just like `cljs.compiler/requires-compilation?`).

Comment by António Nuno Monteiro [ 18/Sep/17 10:42 PM ]

The attached CLJS-2339-2.patch contains the changes in the previous patch and also solves the issues around reloading, only running the foreign libraries that are modules through Closure if any of the source files in the dependency graph have changed.

I'd appreciate if people who are seeing the issue can try out the patch and report back.

Comment by Tony Kay [ 19/Sep/17 11:01 PM ]

So, I did some profiling with visualvm, and gave the results to Antonio. They were showing the vast majority of the time being consumed by `pipe`, under the direction of the node module discovery functions. His new version of the second patch definitely improves reload speed considerably. My hot code reload went from about 14 seconds (via patch 1) to 2 seconds with the new version of patch 2. This is on a project with Om Next, and some largish node libraries.

Comment by David Nolen [ 24/Sep/17 4:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/245bdee2c35e19a9685b7a0849f26fce8bdaf7ca

Comment by Alexis Vincent [ 31/Dec/17 4:42 PM ]

I'm still getting this with 1.9.946. Oddly enough it happens every other reload/eval. On a toy project, including react, with an empty file I get alternating reload/eval times of +-0.8 seconds and +-9 seconds. Removing react from npm-deps brings all reloads to +-0.8s.





[CLJS-2131] Calling empty on a ChunkedSeq returns a vector not an empty list Created: 27/Jun/17  Updated: 29/Dec/17  Resolved: 29/Dec/17

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

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

 Description   

Calling empty on a ChunkedSeq returns an empty vector rather than an empty list.



 Comments   
Comment by Thomas Mulvaney [ 27/Jun/17 3:56 AM ]

Metadata was also being carried on to the empty seq which is not expected behaviour for seqs.

Comment by Mike Fikes [ 19/Nov/17 7:28 PM ]

The attached patch no longer applies. Needs to be re-baselined.

Comment by Thomas Mulvaney [ 28/Dec/17 12:56 AM ]

Attached rebased patch. Test case was moved into seq-test namespace.

Comment by David Nolen [ 29/Dec/17 4:17 PM ]

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





[CLJS-2453] Improved re-seq termination for empty matches Created: 28/Dec/17  Updated: 29/Dec/17  Resolved: 29/Dec/17

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

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

Attachments: Text File CLJS-2453-2.patch     Text File CLJS-2453.patch    
Patch: Code and Test
Approval: Accepted

 Description   

CLJS-608 improved re-seq in one case where an infinite sequence is produced for an empty match.

Here are two other cases that are not quite handled correctly:

For

(re-seq #"[Bc]?" "aBcD")
, Clojure produces ("" "B" "c" "" ""), while ClojureScript produces an infinite sequence of emtpy strings.

For

(re-seq #"[BcD]?$" "aBcD")
, Clojure produces ("D" ""), while ClojureScript terminates early, producing only ("D").

It is possible to handle both of these cases with slightly improved termination handling over what was added in CLJS-608.



 Comments   
Comment by Mike Fikes [ 29/Dec/17 10:03 AM ]

The attached CLJS-2453-2.patch is semantically equivalent to the first, but simplified slightly (it uses <= instead of < and inc).

Comment by David Nolen [ 29/Dec/17 3:37 PM ]

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





[CLJS-1793] Few misplaced docstrings Created: 24/Sep/16  Updated: 29/Dec/17  Resolved: 29/Dec/17

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

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

Attachments: Text File CLJS-1793.1.patch     Text File CLJS-1793.patch    
Patch: Code
Approval: Accepted

 Description   

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



 Comments   
Comment by Mike Fikes [ 19/Nov/17 7:44 PM ]

Patch no longer applies; needs re-baseline.

Comment by Thomas Mulvaney [ 28/Dec/17 1:10 AM ]

Rebased patch attached.

Comment by David Nolen [ 29/Dec/17 3:28 PM ]

fixed https://github.com/clojure/clojurescript/commit/11e645aef91a46adcf03ad22ce8398df26c2eeee





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

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

Type: Enhancement Priority: Major
Reporter: Thomas Mulvaney Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1743.1.patch     Text File CLJS-1743.2.patch     Text File CLJS-1743.patch    
Patch: Code and Test
Approval: Accepted

 Description   

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



 Comments   
Comment by David Nolen [ 16/Jun/17 3:05 PM ]

Patch no longer applies

Comment by Thomas Mulvaney [ 13/Jul/17 5:46 AM ]

Rebased patch attached

Comment by David Nolen [ 22/Dec/17 5:34 PM ]

Need another rebased patch. To avoid another conflict maybe move the tests to the cljs.collections-test.cljs namespace.

Comment by Thomas Mulvaney [ 28/Dec/17 12:13 AM ]

Attached rebased patch with tests moved to collections-test namespace

Comment by David Nolen [ 29/Dec/17 3:23 PM ]

fixed https://github.com/clojure/clojurescript/commit/994069152cd8e8f1338602dd5e17fa63b2694bd9





[CLJS-2247] Warn when overwriting protocol method Created: 15/Jul/17  Updated: 29/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: errormsgs

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

 Description   

Warn when a protocol overwrites a method in another protocol.

Observe the warning in this Clojure REPL session:

user=> (defprotocol IAlpha
  #_=>   (-foo [this]))
IAlpha
user=> (defrecord Alpha []
  #_=>   IAlpha
  #_=>  (-foo [this] :alpha))
user.Alpha
user=> (defprotocol IBeta
  #_=>   (-foo [this]))
Warning: protocol #'user/IBeta is overwriting method -foo of protocol IAlpha
IBeta
user=> (defrecord Beta []
  #_=>   IBeta
  #_=>   (-foo [this] :beta))
user.Beta
user=> (-foo (->Alpha))

IllegalArgumentException No implementation of method: :-foo of protocol: #'user/IBeta found for class: user.Alpha  clojure.core/-cache-protocol-fn (core_deftype.clj:568)

Here is the same in ClojureScript:

To quit, type: :cljs/quit
cljs.user=> (defprotocol IAlpha
(-foo [this]))
nil
cljs.user=> (defrecord Alpha []
IAlpha
(-foo [this] :alpha))
cljs.user/Alpha
cljs.user=> (defprotocol IBeta
(-foo [this]))
nil
cljs.user=> (defrecord Beta []
IBeta
(-foo [this] :beta))
cljs.user/Beta
cljs.user=> (-foo (->Alpha))
repl:13
throw e__5612__auto__;
^

Error: No protocol method IBeta.-foo defined for type : [object Object]
    at cljs$core$missing_protocol (/Users/mfikes/.cljs_node_repl/.cljs_node_repl/cljs/core.js:317:9)
    at cljs$user$_foo (repl:25:34)
    at repl:1:94
    at repl:9:3
    at repl:14:4
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:242:14)
    at Socket.<anonymous> ([stdin]:49:25)

Note that they both result in the same runtime behavior, but Clojure emits a nice diagnostic when IBeta is defined.



 Comments   
Comment by Mike Fikes [ 16/Jul/17 9:28 AM ]

This is an interesting one: While the attached patch produces nice warnings, even without it the :redef-in-file warning triggers if the forms involved happen to reside in files, and in that case you'd get two slightly different warnings being emitted:

WARNING: Protocol IDelta is overwriting method -bar of protocol IGamma in file /Users/mfikes/Desktop/src/foo/core.cljc
WARNING: -bar at line 6 is being replaced at line 7 /Users/mfikes/Desktop/src/foo/core.cljc

Each of the warnings above has their own strengths; but having both seems to be a negative. Here are my initial thoughts on variants:

  1. Leave both: Could be perceived to be noisy, albeit useful if it occurs.
  2. Have the warning disabled, but enabled for the REPL: Odd to have a warning that only happens while in the REPL (and you can still get two when loading files)
  3. Have the warning only be emitted for REPL-entered forms: This perhaps directly addresses the issue: (No double warnings, useful for people learning the language). But this seems even odder to have a warning limited to REPL-entered forms.
  4. Figure out a way to suppress the :redef-in-file warning in this case: Maybe cleaner.
  5. Same as above, but find a way to get the line numbers into the "overwrite" warning: Perhaps cleanest, but difficult to do?
  6. Do nothing, decline the ticket: Perhaps this leaves users learning the language at a loss if they encounter this while in the REPL where redefining forms is "normal", but not overwriting via protocol definitions.
Comment by Mike Fikes [ 29/Dec/17 10:19 AM ]

The attached CLJS-2247-2.patch rebaselines against master.





[CLJS-2452] reverse empty vector returns nil Created: 23/Dec/17  Updated: 27/Dec/17  Resolved: 27/Dec/17

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

Approval: Accepted

 Description   

In Clojure, (reverse []) returns the empty list (), but in ClojureScript, this is returning nil.

This appears to have changed with https://github.com/clojure/clojurescript/commit/944264365091ccc7071e9b5302d95bc58af089db



 Comments   
Comment by David Nolen [ 27/Dec/17 9:18 AM ]

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

Comment by Mike Fikes [ 27/Dec/17 9:24 AM ]

Hey David, in Clojure (rseq []) is actually nil (It is documented as being this way—perhaps it is imitating seq.)

Perhaps the implementation of reverse could patch the problematic case? Something like:

(defn reverse
   [coll]
   (if (reversible? coll)
     (or (rseq coll) ())
     (reduce conj () coll)))
Comment by Mike Fikes [ 27/Dec/17 11:29 AM ]

The previous comment was addressed via David via
https://github.com/clojure/clojurescript/commit/92cc9f34d88b3aa31eff86c1f570af1162f44e4d
and
https://github.com/clojure/clojurescript/commit/4a24d18ca86ba9f41856cc37314cfa4d4797a3b1





[CLJS-2433] HTTP server in cljs.browser.repl doesn't serve files with extensions other than specified in ext->mime-type Created: 04/Dec/17  Updated: 26/Dec/17

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

Type: Defect Priority: Minor
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-CLJS-2433-fallback-to-send-static-use-host-to-determ.patch     Text File 0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch     Text File 0002-CLJ-2433-send-fils-via-send-and-close.patch    

 Description   
$ cat deps.edn
{:deps {org.clojure/clojure {:mvn/version "1.9.0-RC2"}
        org.clojure/clojurescript {:mvn/version "1.9.946"}}
$ touch hello.xml
$ clojure -m cljs.browser.repl &
$ curl localhost:9000/hello.xml
<html><body><h2>Page not found</h2>No page /hello.xml found on this server.</body></html>

Adding an entry to cljs.repl.browser/ext->mime-type superficially fixes that.

1) Currently used MIME type identification is very simplistic and doesn't return correct MIME types for most files. Instead, this should be delegated to the host:

(ns cljs.repl.browser
  (:import [java.nio.file Files Paths]))

(defn get-content-type [path]
  (Files/probeContentType (Paths/get path (into-array String []))))

2) Since both files and sockets are abstractions over bytes, cljs.repl.server/send-and-close should be agnostic to file encoding and just send whatever is in the file. Currently this is handled via cljs.repl.browser/mime-type->encoding.

I will prepare a patch by tomorrow.



 Comments   
Comment by Yegor Timoshenko [ 25/Dec/17 10:16 AM ]

I disagree with this being a minor defect. Currently most files are not served by the built-in HTTP server, which means that it's impossible to develop an application that has files other than HTML/CSS/JS.

Comment by David Nolen [ 26/Dec/17 7:03 AM ]

The builtin webserver is not an important component of ClojureScript. It exists to aid tutorials.

Comment by Yegor Timoshenko [ 26/Dec/17 7:10 AM ]

I like it much more than figwheel because it's simpler, and it seems to be feature-complete other than this bug. Also, this is the first in-browser REPL that comes to mind when using new `clojure` tooling.

Also, previous version of this patch didn't correctly count Content-Length, changed arity of send-and-close function, and didn't work for favicons. I'm attaching a new patch that this time should be complete and cause zero breakage.

New patch is called 0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch in the attachments above. Link: https://dev.clojure.org/jira/secure/attachment/17586/0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch

Here's a GitHub link (for code review/explanations): https://github.com/hackberryhike/clojurescript/commit/ea8336da3a779cb5982875d243dc6d40abd0d3ba

Comment by Yegor Timoshenko [ 26/Dec/17 7:18 AM ]

Also, if you don't like something about this patch, do tell me and I'll fix it right away. I'm very interested in getting this merged.





[CLJS-2451] Unable to require scoped dependency Created: 23/Dec/17  Updated: 23/Dec/17

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

Type: Defect Priority: Major
Reporter: Kurt Harriger Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I've been trying to get @atlaskit/editor-core to work in clojurescript, awhile back I tried ran into issue with npm-deps required keywords that seems to be fixed, but now having what seems to be the same issue with require.

Assert failed: cljs.analyzer/foreign-dep? expected symbol got "@atlaskit/editor-core"
(symbol? dep)

I pushed a project to github that reproduces the issue:
https://github.com/kurtharriger/cljs-npm-dep-issue






[CLJS-2450] Improve JS Module file extension ignore logic Created: 23/Dec/17  Updated: 23/Dec/17  Resolved: 23/Dec/17

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

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

Attachments: Text File CLJS-2450-2.patch     Text File CLJS-2450-3.patch     Text File CLJS-2450.patch    
Patch: Code
Approval: Accepted

 Description   

Some additional file types (.jsx) are supported when using :preprocess. Antonio suggested the best solution would be to ignore .css now and make the ignored extensions list configurable.



 Comments   
Comment by Juho Teperi [ 23/Dec/17 8:08 AM ]

Added debug-prn under verbose build.

Comment by Juho Teperi [ 23/Dec/17 11:05 AM ]

Rebased against master and removed once unncessary line.

Comment by David Nolen [ 23/Dec/17 11:37 AM ]

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





[CLJS-2417] Inter-ns s/fdef expansion side effect fails when load cached source Created: 24/Nov/17  Updated: 22/Dec/17  Resolved: 22/Dec/17

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

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


 Description   

Like CLJS-1989, but for inter-ns s/fdef as opposed to intra-ns. When loading from cache, if an s/fdef specs a function in another namespace, the load can fail. The root cause is that analysis cache writing and reading is scoped by namespace, and one failure mode is as follows: When analysis cache is written for a given namespace A, specs set up by a different namespace B may not have yet been loaded. When that namespace B is subsequently loaded, if any cache is written for B, it will, due to the scoping, only write out information relevant to B.

To repro, the setup is similar to that in CLJS-1989, but involves splitting the defn and s/fdef across two namespaces:

Have src/foo/core.cljs with:

(ns foo.core)

(defn bar [x])

Have src/baz/core.cljs with:

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

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

Then:

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

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

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

Note, the above was reproduced using ClojureScript 1.9.655 as well, where CLJS-1989 landed, to be sure that this wasn't a subsequent regression.

As an aside, the motivating example where this inter-ns pattern arises is porting of clojure.core.specs.alpha to cljs.core.specs.alpha (CLJS-2413) where in each case, the specs are in a separate *...specs namespace.



 Comments   
Comment by David Nolen [ 22/Dec/17 7:10 PM ]

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





[CLJS-2390] Metadata `def` and attr-map in `defn` are not evaluated Created: 28/Oct/17  Updated: 22/Dec/17  Resolved: 22/Dec/17

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

Type: Defect Priority: Major
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Unlike in clojure metadata is not evalled on def:

(def ^{:aa #'+} tt [])
(:aa (meta #'tt))
;;=> (var +)

(defn tt {:aa #'+} [])
(:aa (meta #'tt))
;;=> (var +)

Probably related to CLJS-2388.



 Comments   
Comment by Vitalie Spinu [ 28/Oct/17 1:28 PM ]

After some digging I see that vars are created at compile time, so there might not be a fix for this given the current model. This is a major difference with clojure but it's is not documented anywhere. Not in the doc string, nor in the "differences with clojure" document.

I think the compiler could do a better job by unquoting vars and symbols: #'xyz) and 'xyz. Incidentally, that is the only use case which I need for my package.

Comment by David Nolen [ 22/Dec/17 11:29 AM ]