<< Back to previous view

[CLJS-2369] Undefined nameToPath for bootstrap` when using Twitter's Bootstrap (twbs) Created: 24/Sep/17  Updated: 24/Sep/17

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

Type: Defect Priority: Major
Reporter: Corin Lawson Assignee: Unassigned
Resolution: Unresolved Votes: 0
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']);





[CLJS-2368] Self-host: Never compile macro namespaces with `:optimize-constants true` Created: 23/Sep/17  Updated: 24/Sep/17

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: António Nuno Monteiro
Resolution: Unresolved Votes: 0
Labels: bootstrap

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

 Description   

Macro namespaces may be evaluated in a compiler environment that is different from the one which has emitted the constants table, which leads to cryptic errors because the constants are not available in the evaluating environment.



 Comments   
Comment by David Nolen [ 24/Sep/17 4:43 AM ]

Looks like this one needs a rebase





[CLJS-2367] Self-host: :def-emits-var leaks into loaded namespace processing Created: 20/Sep/17  Updated: 24/Sep/17  Resolved: 24/Sep/17

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: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

If you require a namespace, any defs in that namespace should return the value, not the Var. On the other hand, defs evaluated at the REPL return Vars because :def-emits-var is set to true.

In self-hosted ClojureScript, defs in loaded namespaces produce Vars if :def-emits-var is set to true.

Here is a minimal repro:

(require 'cljs.js)

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

(cljs.js/eval st
  '(require (quote foo.core))
  {:context :expr
   :def-emits-var true
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns foo.core) (def b (def a 3))"}))}
  prn)

(cljs.js/eval st
  'foo.core/b
  {:context :expr
   :eval cljs.js/js-eval}
  prn)

The last form should cause

{:value 3}

to be printed, but instead you get

{:value #'foo.core/a}
.

Similarly, for a macros namespace, this minimal repro

(cljs.js/eval st
  '(require-macros (quote bar.core))
  {:context :expr
   :def-emits-var true
   :eval cljs.js/js-eval
   :load (fn [_ cb]
           (cb {:lang :clj
                :source "(ns bar.core) (def b (def a 3)) (defmacro c [] b)"}))}
  prn)

(cljs.js/eval st
  '(bar.core/c)
  {:context :expr
   :eval cljs.js/js-eval}
  prn)

Should cause

{:value 3}

to be printed, but currently this will cause things to really derail with

Error: failed compiling constant: #'bar.core$macros/a; ...


 Comments   
Comment by David Nolen [ 24/Sep/17 4:41 AM ]

fixed https://github.com/clojure/clojurescript/commit/4607affd1f399839b44eef1aa01e0645cee539d4





[CLJS-2366] :arglists inconsistency in cljs Created: 18/Sep/17  Updated: 24/Sep/17

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

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

n/a



 Description   

ClojureScript does not seem to fully support setting :arglists meta-data to a function. In particular, it seems to fail when the real parameter list contains an '&'.

In Clojure,

(:arglists (meta (defn f {:arglists '([x])} [& a] a)))

returns ([x]). But, in ClojureScript, it returns ([& a])

Note that simpler forms do work correctly:

(:arglists (meta (defn f {:arglists '([x])} [a] a)))

returns ([x]) in both environments.

(Tested in in ClojureScript 1.9.908 and Clojure 1.9.0-alpha17)



 Comments   
Comment by David Goldfarb [ 24/Sep/17 3:57 AM ]

Looks like this is a duplicate of https://dev.clojure.org/jira/browse/CLJS-2351





[CLJS-2365] Self-host: Unable to reload namespace while in it Created: 18/Sep/17  Updated: 20/Sep/17

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

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


 Description   

There is an unreleased self-host regression where requiring a namespace while in that namespace triggers circular dependency detection.

As a concrete example, let's say you are in a REPL, and you require a namespace, go into that namespace (using in-ns), exercise it a little, and then change the code to fix something and then reload it. This now fails on master for self-hosted code.

A repro following this example is the following:

(require 'cljs.js)

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

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

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

This causes the following on master, but not with the 1.9.908 release:

{:error #error {:message "Circular dependency detected foo.core -> foo.core", :data {:tag :cljs/analysis-error}}}

(Strictly speaking, the above example is not minimal in that :reload is not required in order to reproduce it: It will happen if you simply attempt to require the namespace while "in" it.)



 Comments   
Comment by Mike Fikes [ 18/Sep/17 10:19 AM ]

Git bisect result implicates CLJS-2356:

238028ccc51afe45de98fb853be4396a71afb602 is the first bad commit
commit 238028ccc51afe45de98fb853be4396a71afb602
Author: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Date:   Sun Sep 10 21:24:22 2017 -0700

    CLJS-2356: Self-host: circular dependency detection is not correct

:040000 040000 a93d466e3f1ea042e730fb78ca911f92319880d0 29108cab4b45247e641d430d85dda85c436e95fe M	src
Comment by Mike Fikes [ 18/Sep/17 3:22 PM ]

Here is the result from some analysis by António and me: There are a few places in the self-host compiler code which make use of :def-emits-var to deduce that self-host is being used to implement a REPL. In particular, the code being exercised in the ticket description works if you include :def-emits-var true in the options passed. But, if you are evaluating a "load" form, such as require and others, if :def-emits-var is set, then it can be seen that code inside the loaded namespaces gets compiled with def (and derived) forms producing Vars. (This can be seen if you make use of the self-hosted caching option.) Perhaps this is incorrect behavior (I believe it doesn't occur with JVM ClojureScript, for example). With the current behavior, if a REPL chooses to not set :def-emits-var true for "load" forms, then you encounter the issue described in the ticket description. So, in short, the underlying issue in this ticket could either be fixed or deemed OK, and the other issue mentioned here regarding def forms inside required namespaces could be split off as a separate ticket that could be pursued on its own.

Comment by Mike Fikes [ 20/Sep/17 1:10 PM ]

With CLJS-2367, I suspect this ticket becomes a non-issue, because REPLs can unconditionally (always) set :def-emits-var.





[CLJS-2364] Bump Closure Compiler to the Sep 2017 version Created: 16/Sep/17  Updated: 24/Sep/17

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

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

Attachments: Text File CLJS-2364.patch    




[CLJS-2363] Warn user when a namespace requires itself Created: 16/Sep/17  Updated: 16/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Christian Fortin 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.






[CLJS-2362] Make node REPL debuggable/inspectable again Created: 15/Sep/17  Updated: 17/Sep/17

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

Type: Enhancement Priority: Minor
Reporter: Andrea Richiardi Assignee: Andrea Richiardi
Resolution: Unresolved Votes: 0
Labels: nodejs

Attachments: File cljs-2362.diff    
Approval: Screened

 Description   

Currently, when trying to enable debug mode a warning is thrown:

Clojure 1.9.0-alpha17
user=> (require '[cljs.repl :as repl])
nil
user=> (require '[cljs.repl.node :as node])         
nil
user=> (repl/repl (node/repl-env :debug-port 5002))
(node:2152) [DEP0062] DeprecationWarning: `node --debug` and `node --debug-brk` are invalid. Please use `node --inspect` or `node --inspect-brk` instead.

I can take care of this easy patch.



 Comments   
Comment by Andrea Richiardi [ 15/Sep/17 12:24 AM ]

Patch attached, a bit rusty , hopefully all good

Comment by David Nolen [ 15/Sep/17 7:36 AM ]

Is this going to work for older version of Node?

Comment by Andrea Richiardi [ 15/Sep/17 1:46 PM ]

Should work for node >= 6.3, do you want me to branch on the version?

Comment by Thomas Heller [ 15/Sep/17 2:42 PM ]

Given that CLJS itself does not use this info and just passes it along it might be a better solution to provide a generic option that takes a seq of strings to use when launching the node process.

In shadow-cljs I have

(shadow/node-repl {:node-args ["--inspect" "--inspect-brk" "--whatever-you-need"]})

This is more flexible and doesn't have to deal with versioning issues since it is left to the user.

Comment by Andrea Richiardi [ 17/Sep/17 1:59 PM ]

I like Thomas' approach way better, branching on the version would need to execute node version,not really pretty.

Many boot tasks have a :options key for that (converting {:key} to --key). Thomas version is even simpler, the only difficulty would be to avoid breaking :debug-port as it is now.





[CLJS-2361] Self-host: circular dependency detection doesn't handle REPL self-require Created: 13/Sep/17  Updated: 15/Sep/17  Resolved: 15/Sep/17

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

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

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

 Comments   
Comment by David Nolen [ 15/Sep/17 3:22 PM ]

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





[CLJS-2360] ClojureScript ignores first two arguments passed to a macro when using vargs Created: 13/Sep/17  Updated: 15/Sep/17

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

Type: Defect Priority: Minor
Reporter: Ethan McCue Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   

The following code produces different results in clojure and in clojurescript

(defmacro beep [& args]
  (cons 'list args))
(print (beep 0 1 2 3))

In clojure that code outputs (0 1 2 3)
In clojurescript that code outputs (2 3)






[CLJS-2359] Improve cljs.test runtime error reporting Created: 13/Sep/17  Updated: 13/Sep/17

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

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


 Description   

It would be an awesome addition to improve the reporter so that

ERROR in (test-hydrate-saladman) (TypeError:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[TypeError TypeError: Cannot read property 'call' of null]

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

would display the full stack trace of the runtime error whenever it happens.
At the moment a user is left on its own with no location, no hints whatsoever.






[CLJS-2358] two (inc) calls give different result (1 and NaN) even though argument evaluates to the same value (nil) Created: 11/Sep/17  Updated: 11/Sep/17  Resolved: 11/Sep/17

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

Type: Defect Priority: Major
Reporter: Markus Bertheau Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Chromium Version 60.0.3112.113 (Developer Build) Built on Ubuntu , running on Ubuntu 16.04 (64-bit)



 Description   

cljs.user=> (apply max [])
nil
cljs.user=> (inc nil)
1
cljs.user=> (inc (apply max []))
NaN

I expect (inc (apply max [])) to evaluate to 1, since (inc nil) evaluates to 1 and (apply max []) evaluates to nil.



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

This is not a bug. (apply max []) in Clojure throws because that's an invalid arity.





[CLJS-2357] Self-host: run all async tests Created: 10/Sep/17  Updated: 11/Sep/17  Resolved: 11/Sep/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap, test

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

 Comments   
Comment by David Nolen [ 11/Sep/17 6:37 AM ]

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





[CLJS-2356] Self-host: circular dependency detection is not correct Created: 10/Sep/17  Updated: 18/Sep/17  Resolved: 11/Sep/17

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

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

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

 Description   

Patch should be applied after CLJS-2354



 Comments   
Comment by David Nolen [ 11/Sep/17 6:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/238028ccc51afe45de98fb853be4396a71afb602

Comment by Mike Fikes [ 18/Sep/17 10:20 AM ]

See regression in CLJS-2365.





[CLJS-2355] Fix tests after CLJS-2349 Created: 10/Sep/17  Updated: 15/Sep/17  Resolved: 15/Sep/17

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

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

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

 Description   

warn-on-reflection doesn't exist in CLJS



 Comments   
Comment by David Nolen [ 15/Sep/17 3:29 PM ]

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





[CLJS-2354] Self-host: `compile-str` doesn't handle `clojure` -> `cljs` aliasing Created: 10/Sep/17  Updated: 11/Sep/17  Resolved: 11/Sep/17

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

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

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

 Comments   
Comment by David Nolen [ 11/Sep/17 6:35 AM ]

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





[CLJS-2353] use portable `node-module-dep?` function in analyze-deps Created: 10/Sep/17  Updated: 11/Sep/17  Resolved: 11/Sep/17

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

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

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

 Comments   
Comment by David Nolen [ 11/Sep/17 6:33 AM ]

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





[CLJS-2352] Compiler emits invalid JS code for NaN/Inf/-Inf if used with CLJ >= 1.9.0-alpha20 Created: 09/Sep/17  Updated: 24/Sep/17  Resolved: 24/Sep/17

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

Type: Defect Priority: Major
Reporter: Shogo Ohta Assignee: David Nolen
Resolution: Completed Votes: 3
Labels: None

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

 Description   

Since 1.9.0-alpha20, Clojure has its own external representations for NaN/Inf/-Inf: ##NaN / ##Inf / ##-Inf, respectively.

The current CLJS compiler emits code as is for those values, so if it is used with the latest Clojure, it will end up emitting invalid JS code:

=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##NaN)))
"##NaN"
=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##Inf)))
"##Inf"
=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##-Inf)))
"##-Inf"


 Comments   
Comment by Shogo Ohta [ 12/Sep/17 12:43 AM ]

Added a patch.
I tested the code on my local env and saw the things worked fine, but I'm not sure I should have added CLJ 1.9.0-alpha for test profile.

Comment by David Nolen [ 15/Sep/17 3:26 PM ]

Patch looks fine but could we please add a test case.

Comment by Shogo Ohta [ 17/Sep/17 12:54 AM ]

Updated the patch.

Is it OK to directly use the raw Java constants for NaN/Infinity/-Infinity in test cases since we can't use new literals for those values yet until the dependent tools.reader is updated to 1.1.0?

Comment by David Nolen [ 24/Sep/17 4:40 AM ]

fixed https://github.com/clojure/clojurescript/commit/89914d2ead964122f99e638edda0cd96d330cb66





[CLJS-2351] Setting :arglists metadata when vararg is present Created: 08/Sep/17  Updated: 15/Sep/17

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

Type: Defect Priority: Minor
Reporter: Hlöðver Sigurðsson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: vars
Environment:

I'm running cljs 1.9.909 with figwheel and lumo 1.7, bug is present in both environments.


Approval: Vetted

 Description   

consider function with no parameters

(defn aarg {:arglists '([fake])} [])
=> {:ns cljs.user,
 :name aarg,
 :file nil,
 :end-column 11,
 :column 1,
 :line 1,
 :end-line 1,
 :arglists ([fake]),
 :doc nil,
 :test nil}

All works as expected, but with the introduction of a vararg

(defn aarg {:arglists '([fake])} [& env])
(meta #'aarg)
=> {:ns cljs.user,
 :name aarg,
 :file nil,
 :end-column 11,
 :top-fn {:variadic true,
          :max-fixed-arity 0,
          :method-params [(env)],
          :arglists ([& env]),
          :arglists-meta (nil)},
 :column 1,
 :line 1,
 :end-line 1,
 :arglists ([& env]),
 :doc nil,
 :test nil}

:arglists does not get affected.






[CLJS-2350] Fix circular dependencies test Created: 08/Sep/17  Updated: 09/Sep/17  Resolved: 09/Sep/17

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

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

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

 Comments   
Comment by David Nolen [ 09/Sep/17 9:28 AM ]

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





[CLJS-2349] Port reset-vals! and swap-vals! over from Clojure Created: 07/Sep/17  Updated: 09/Sep/17  Resolved: 09/Sep/17

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

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

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

 Description   

same as https://github.com/clojure/clojure/commit/9fdbd8cd56524911d120e4631cc53c572ebdd33d



 Comments   
Comment by David Nolen [ 09/Sep/17 9:15 AM ]

fixed https://github.com/clojure/clojurescript/commit/35a360943ffade5fc319203512010458c054e241





[CLJS-2348] Node ns require doesn't create top-level object Created: 04/Sep/17  Updated: 04/Sep/17  Resolved: 04/Sep/17

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

Type: Defect Priority: Major
Reporter: Leon Grapenthin Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: nodejs


 Description   

Some node packages export an object directly which in turn can not be instantiated.

(ns demo.core (:require [web3 :as Web3]))

(new Web3)

ReferenceError: Web3 is not defined



 Comments   
Comment by Leon Grapenthin [ 04/Sep/17 3:53 PM ]

misreport





[CLJS-2347] NPE on :infer-externs true Created: 03/Sep/17  Updated: 08/Sep/17

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

Type: Defect Priority: Major
Reporter: Kanwei Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When :infer-externs is true in 1.9.908, an NPE will result from the Closure compiler. Also reported here with a stacktrace:

https://github.com/google/closure-compiler/issues/2629



 Comments   
Comment by Juho Teperi [ 08/Sep/17 9:00 AM ]

Well, this is interesting. I haven't been able to reproduce this without Boot-cljs completely, but I can reproduce this with Cljs by manually providing extern with single line: `var COMPILED;`. For some reason, inferred extern with Boot-cljs includes this line, but not when calling compiler directly.
`





[CLJS-2346] Make (:require foo/bar) work for JS modules Created: 01/Sep/17  Updated: 05/Sep/17

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

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

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

 Description   

the string require only really becomes necessary when there are multiple slashes. We can support this style as a symbol for dependencies that only have 1 slash because it's a valid symbol.



 Comments   
Comment by Thomas Heller [ 02/Sep/17 2:43 AM ]

Me again ... what is so bad about using string requires? Qualified symbols are illegal for normal CLJS code and using a string already solves all potential issues. Adding a case where you can use a symbol if there is one slash but not if there are two is just going to confuse new users with no additional benefit.

Comment by Juho Teperi [ 05/Sep/17 3:40 AM ]

I can see the point of only allowing slashes with strings.

However, if we do this, it might be good to check the warnings we give when trying to do this:

(ns example.core
  (:require [react-dom/server :as sdf]))

  No such namespace: react-dom/server, could not locate react_dom_SLASH_server.cljs, react_dom_SLASH_server.cljc, or JavaScript source providing "server"

Notice how it says "server" instead of react-dom/server. This is because everything allows slashes, but foreign lib code only uses name part instead of namespace. (This patch doesn't change the warning.)

Comment by Thomas Heller [ 05/Sep/17 4:50 AM ]

There are other ambiguities when it comes to JS requires which is why I'm still advocating for just using strings in all circumstances.

Technically we could make {{(:require [@scoped/thing :as thing])}} work as well but shouldn't.

Also I'm pretty sure I saw several JS packages that either used "-" or "_" in their names. For CLJS we always convert to an underscore which would add more confusion if we don't do this for JS.

Yes, the warning should be fixed but wouldn't even be an issue if a string was used.





[CLJS-2345] File paths emitted as args to cljs.core.load_file without escaping fails on Windows Created: 31/Aug/17  Updated: 09/Sep/17  Resolved: 09/Sep/17

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: windows

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

 Description   

(Context: https://github.com/funcool/promesa/issues/52)

The compiler currently doesn't escape file paths when emitting calls to cljs.core.load_file. This is bad in cases where a library has a JS dependency with a path segment whose first character corresponds to an ASCII control code (a, b, t, n, v, f, r, e); this can result in a control code being where a backslash and letter should be, e.g.:

// Compiled by ClojureScript 1.9.908 {:target :nodejs}
goog.provide('promesa.impl');
goog.require('cljs.core');
goog.require('promesa.protocols');
cljs.core.load_file(".cljs_node_repl\bluebird\bluebird.js");

(Note the unescaped backslashes, and thus the backspace char.)

Patch incoming.



 Comments   
Comment by David Nolen [ 09/Sep/17 9:46 AM ]

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





[CLJS-2344] de-dupe externs Created: 31/Aug/17  Updated: 21/Sep/17

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: 0
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.






[CLJS-2343] Double require guard bypassed if :refer-macros Created: 30/Aug/17  Updated: 30/Aug/17

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

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

Approval: Vetted

 Description   

If you have this code,

(ns foo.core
  (:require [cljs.test])
  (:require [clojure.string]))

it will trigger the "Only one :require form is allowed per namespace definition" diagnostic.

But this diagnostic is bypassed if you use inline macro spec sugar:

(ns foo.core
  (:require [cljs.test :refer-macros [deftest]])
  (:require [clojure.string]))

This causes the compiler to derail with this:

clojure.lang.ExceptionInfo: Only :as, :refer and :rename options supported in :require / :require-macros; offending spec: [cljs.test :refer-macros [deftest]] at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (ns foo.core (:require [cljs.test :refer-macros [deftest]]) (:require [clojure.string]))}, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4617)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:693)
	at cljs.analyzer$error.invoke(analyzer.cljc:693)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:695)
	at cljs.analyzer$basic_validate_ns_spec.invokeStatic(analyzer.cljc:2256)
	at cljs.analyzer$parse_require_spec.invokeStatic(analyzer.cljc:2344)
	at cljs.analyzer$parse_require_spec.invoke(analyzer.cljc:2343)
	at clojure.lang.AFn.applyToHelper(AFn.java:171)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invokeStatic(core.clj:652)
	at clojure.core$partial$fn__4765.doInvoke(core.clj:2534)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$map$fn__4785.invoke(core.clj:2644)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.RT.boundedLength(RT.java:1749)
	at clojure.lang.RestFn.applyTo(RestFn.java:130)
	at clojure.core$apply.invokeStatic(core.clj:650)
	at cljs.analyzer$fn__2052$fn__2062.invoke(analyzer.cljc:2622)
	at clojure.core.protocols$fn__6755.invokeStatic(protocols.clj:167)
	at clojure.core.protocols$fn__6755.invoke(protocols.clj:124)
	at clojure.core.protocols$fn__6710$G__6705__6719.invoke(protocols.clj:19)
	at clojure.core.protocols$seq_reduce.invokeStatic(protocols.clj:31)
	at clojure.core.protocols$fn__6738.invokeStatic(protocols.clj:75)
	at clojure.core.protocols$fn__6738.invoke(protocols.clj:75)
	at clojure.core.protocols$fn__6684$G__6679__6697.invoke(protocols.clj:13)
	at clojure.core$reduce.invokeStatic(core.clj:6545)
	at cljs.analyzer$fn__2052.invokeStatic(analyzer.cljc:2575)
	at cljs.analyzer$fn__2052.invoke(analyzer.cljc:2556)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3333)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3336)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3361)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3530)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3577)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3603)
	at cljs.repl$evaluate_form$__GT_ast__6169.invoke(repl.cljc:471)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:472)
	at cljs.repl$eval_cljs.invokeStatic(repl.cljc:631)
	at cljs.repl$eval_cljs.invoke(repl.cljc:618)
	at cljs.repl$repl_STAR_$read_eval_print__6300.invoke(repl.cljc:880)
	at cljs.repl$repl_STAR_$fn__6306$fn__6315.invoke(repl.cljc:922)
	at cljs.repl$repl_STAR_$fn__6306.invoke(repl.cljc:921)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1252)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:882)
	at cljs.repl$repl.invokeStatic(repl.cljc:1001)
	at cljs.repl$repl.doInvoke(repl.cljc:933)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.repl.node$_main.invokeStatic(node.clj:234)
	at cljs.repl.node$_main.invoke(node.clj:233)
	at clojure.lang.Var.invoke(Var.java:375)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.core$apply.invokeStatic(core.clj:646)
	at clojure.main$main_opt.invokeStatic(main.clj:314)
	at clojure.main$main_opt.invoke(main.clj:310)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)





[CLJS-2342] Speed up printing of js object by using forEach and regex optimizations Created: 30/Aug/17  Updated: 30/Aug/17

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

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

 Description   

By using goog.object/forEach, along with direct interop for regex checking it is possible to speed up the printing of JavaScript objects anywhere between 1.14 and 1.77



 Comments   
Comment by Mike Fikes [ 30/Aug/17 10:50 AM ]
Speedup Summary:

        Engine  small, medium, with sub-object
            V8: 1.42, 1.31, 1.19
  SpiderMonkey: 1.14, 1.48, 1.41
JavaScriptCore: 1.48, 1.58, 1.62
       Nashorn: 1.27, 1.36, 1.20
    ChakraCore: 1.49, 1.77, 1.77


Before:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 74 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 84 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 75 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 95 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 92 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 134 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 46 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 41 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 55 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 1228 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 1048 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 620 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 55 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 76 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 129 msecs


After:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 52 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 64 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 63 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 83 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 62 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 95 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 31 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 26 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 34 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 970 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 769 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 518 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 37 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 43 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 73 msecs




[CLJS-2341] Speed up js->clj on objs using forEach and transient volatile Created: 30/Aug/17  Updated: 31/Aug/17

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

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

 Description   

It is possible to speed up js->clj on JavaScript objects by revising the implementation to employ goog.object/forEach, accumulating by bashing on a transient volatile map.

The speedups range from 1.5 to 2.1 over the current implementation.

Note: The current implementation uses js-keys. The optimization in CLJS-2340 could help js->clj, but it doesn't appear to provide much speedup in itself (perhaps 1.1?) compared to the change in implementation described above.



 Comments   
Comment by Mike Fikes [ 30/Aug/17 9:19 AM ]
Speedup Summary:

        Engine  small, medium, with sub-object
            V8: 1.62, 1.50, 1.13
  SpiderMonkey: 1.91, 1.74, 1.59
JavaScriptCore: 1.67, 1.74, 2.10
       Nashorn: 1.54, 2.13, 1.51
    ChakraCore: 1.71, 2.10, 1.95


Before:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 55 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 63 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 93 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 155 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 87 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 94 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 45 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 33 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 42 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 1123 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 1195 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 773 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 65 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 44 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 78 msecs


After:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 34 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 42 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 82 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 81 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 50 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 59 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 27 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 19 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 20 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 728 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 561 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 513 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 38 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 21 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 40 msecs




[CLJS-2340] Have js-keys delegate directly to good.object/getKeys Created: 29/Aug/17  Updated: 24/Sep/17  Resolved: 24/Sep/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-2340.patch    
Patch: Code
Approval: Accepted

 Description   

js-keys is implemented using goog.object/forEach. This essentially results in JavaScript that does a for over each key in the object, (just like goog.object/getKeys), but instead of directly accumulating the keys in a JavaScript array, using forEach ends up incurring unnecessary overhead extracting (and ultimately discarding) the value associated with each key, and making a callback for each key. In :none mode, you can see that goog.object/getKeys can be twice as fast as js-keys at times (at least in JavaScriptCore—work for this ticket should probably include a new benchmark.)

This ticket asks that js-keys be implemented by directly delegating to good.object/getKeys. There are a few places, in printing, and in js->clj that could benefit from this speedup (and with the simple delegation, good.object/getKeys could even be inlined by hand at call sites if it improved perf).



 Comments   
Comment by Mike Fikes [ 29/Aug/17 7:06 PM ]
Speedup summary:

            V8:  7.2,  8.7
  SpiderMonkey: 29.5, 29.2
JavaScriptCore:  7.8,  4.8
       Nashorn:  6.2,  5.5
    ChakraCore:  3.6,  5.8


Before:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 72 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 96 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 177 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 146 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 133 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 78 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 804 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 749 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 91 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 129 msecs


After:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 10 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 11 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 6 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 5 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 17 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 16 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 128 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 136 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2) f js-keys], (f obj), 400000 runs, 25 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6) f js-keys], (f obj), 400000 runs, 22 msecs
Comment by David Nolen [ 24/Sep/17 4:44 AM ]

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





[CLJS-2339] Significant code reload slowdown with :npm-deps Created: 29/Aug/17  Updated: 24/Sep/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





[CLJS-2338] Add support for renamePrefix and renamePrefixNamespace closure compiler options Created: 29/Aug/17  Updated: 15/Sep/17  Resolved: 15/Sep/17

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

Type: Enhancement Priority: Major
Reporter: Pieter du Toit Assignee: Pieter du Toit
Resolution: Completed Votes: 1
Labels: closure

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

 Description   

Adding support for these closure compiler options would allow for a mechanism to avoid name collisions



 Comments   
Comment by Pieter du Toit [ 29/Aug/17 6:01 AM ]

Patch attached

Comment by David Nolen [ 29/Aug/17 6:35 AM ]

Pieter, thanks for the patch. Have you submitted your contributor agreement?

Comment by David Nolen [ 29/Aug/17 6:37 AM ]

One question I have about the patch, is there a default prefix name if not supplied by the user?

Comment by Pieter du Toit [ 29/Aug/17 7:05 AM ]

Yes, I have submitted my contributor agreement. The patch does not specify defaults, if the user does not supply :rename-prefix or :rename-prefix-namespace, then the compiler options passed to the closure compiler would be unchanged.

Comment by David Nolen [ 29/Aug/17 8:05 AM ]

Pieter sorry for being unclear, what happens if :rename-prefix-namespace is specified but :rename-prefix is not specified?

Comment by Pieter du Toit [ 29/Aug/17 9:40 AM ]

AFAICT, the options can be specified independently, the only documentation I could find on it is in Closure CommandLineRunner and here.

For my own use case, I only need to set :rename-prefix, which I have tested both with and without :modules and works as expected.

I did some further testing now with only specifying :rename-prefix-namespace (which calls setRenamePrefixNamespace), it has no impact on a build without :modules, and results in an error "{prefix} is not defined" when using with :modules. Specifying both :rename-prefix and :rename-prefix-namespace for a build with :modules results in the same "{prefix} is not defined" error.

I am less confident in :rename-prefix-namespace so I could modify the patch to remove the option ? I also noticed that I would need to modify the patch to add any new options to known-opts

Comment by Thomas Heller [ 29/Aug/17 11:26 AM ]

FWIW I'm using setRenamePrefixNamespace for a feature in shadow-cljs [1]. The effect is that only one global variable will be created and everything else will be a property of that variable. I'm using $CLJS as the prefix namespace and where :advanced mode would otherwise create a xY variable (just an example, name doesn't matter), it would now create a $CLJS.xY property instead.

I have never used :rename-prefix but my understanding is that if would just turn xY into $CLJSxY so you'd still end up with a bunch of global variables.

In my case I use it combined with :modules and only setRenamePrefixNamespace needs to be set and is otherwise unrelated to :rename-prefix.

[1] https://github.com/thheller/shadow-cljs/blob/21e9a3a279ed7a34239cc3c7cc65715f22289163/src/main/shadow/cljs/closure.clj#L481

Comment by David Nolen [ 30/Aug/17 6:21 AM ]

Ok so they sound like independent options then. Pieter, I'm fine with the patch granted it's updated to augment known-opts.

Comment by Pieter du Toit [ 30/Aug/17 9:01 AM ]

David, thanks, I have attached the updated patch

Comment by Pieter du Toit [ 30/Aug/17 9:17 AM ]

Thomas, thanks, I get the same result (no prefix namespace) with a shadow-cljs release build as with cljs (when using the new :rename-prefix-namespace and no :modules)

shadow-cljs.edn
{:dependencies
 []

 :source-paths
 ["src"]

 :builds
 {:app
  {:target :browser
   :output-dir "shadow/js"
   :modules
   {:main {:entries [moduleb]}}}}}
moduleb.cljs
(ns moduleb)

(def env "dev")

(defn init []
  (js/console.log (str "Module B init for env: " env)))

(init)
Comment by Thomas Heller [ 31/Aug/17 3:45 AM ]

The option itself is not supported as of now as I'm using the default cljs.closure/set-options function in shadow-cljs. When it is added to CLJS it will become available in shadow-cljs.

I'm currently using the feature internally for the :npm-module target which compiles code so it can be directly consumed by JS tools (or node). I'm setting that manually though so the :rename-prefix-namespace option is not recognized.

Comment by David Nolen [ 15/Sep/17 3:46 PM ]

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





[CLJS-2337] Missing renamePrefixNamespace compiler option Created: 28/Aug/17  Updated: 29/Aug/17  Resolved: 29/Aug/17

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

Type: Enhancement Priority: Major
Reporter: Xiaomin Wu Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: compiler
Environment:

any



 Description   

right now if we use the ":modules" option with ":optimization :advanced" flag, compile will expose a lot of global functions/vars. From documentation, if we have "renamePrefixNamespace", compile will put all the globale function/vars under the "renamePrefixNamespace". However Clojure Script compiler is missing this flag.



 Comments   
Comment by David Nolen [ 29/Aug/17 6:37 AM ]

dupes CLJS-2338 which has a patch.





[CLJS-2336] Call alength once in areduce and amap Created: 26/Aug/17  Updated: 27/Aug/17  Resolved: 27/Aug/17

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

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

 Description   

Make a single call to alength in areduce and amap, essentially porting relevant aspects of CLJ-1765 and CLJ-1901 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 26/Aug/17 5:32 PM ]
Before:

Benchmarking with V8
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 63 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 83 msecs

Benchmarking with SpiderMonkey
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 26 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 796 msecs

Benchmarking with JavaScriptCore
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 92 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 98 msecs

Benchmarking with Nashorn
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 1258 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 1433 msecs

Benchmarking with ChakraCore
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 9 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 67 msecs


After:

Benchmarking with V8
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 56 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 79 msecs

Benchmarking with SpiderMonkey
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 23 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 787 msecs

Benchmarking with JavaScriptCore
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 93 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 104 msecs

Benchmarking with Nashorn
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 991 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 1252 msecs

Benchmarking with ChakraCore
[arr (to-array (range 1000000))], (reset! x (areduce arr i ret 0 (+ ret (aget arr i)))), 1 runs, 9 msecs
[arr (to-array (range 1000000))], (amap arr i ret (* 10 (aget arr i))), 1 runs, 71 msecs
Comment by David Nolen [ 27/Aug/17 4:55 PM ]

fixed https://github.com/clojure/clojurescript/commit/998933f5090254611b46a2b86626fb17cabc994a





[CLJS-2335] Avoid alength on strings Created: 26/Aug/17  Updated: 26/Aug/17  Resolved: 26/Aug/17

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: David Nolen
Resolution: Completed Votes: 0
Labels: None

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

 Description   

There are a handful of places where alength is called (improperly) on string instances. While preserving identical JavaScript codegen .-length interop could be used instead.



 Comments   
Comment by David Nolen [ 26/Aug/17 6:14 PM ]

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





[CLJS-2334] Also gather dependencies from foreign-lib modules Created: 22/Aug/17  Updated: 25/Aug/17  Resolved: 25/Aug/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: js-modules

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

 Description   

Currently dependencies of foreign libs passed as modules are not gathered and processed in Closure.

It would be nice to make npm-deps and foreign libs passed as modules just work.

(b/build "src"
  {:output-dir "out"
   :main 'foo.core
   :output-to "out/main.js"
   :asset-path "/out"
   :foreign-libs [{:file "src/foreign/lib.js"
                   :module-type :es6
                   :provides ["js.lib"]}]
   :npm-deps {:react "15.6.1"
              :react-dom "15.6.1"}
   :install-deps true
   :optimizations :none
   :compiler-stats true
   :verbose true})
import React from 'react';

export var main = function() {
    console.log("This is my JSX component: " + React.DOM.div(null, 'hi'));
};


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

Patch attached with fix.

Comment by David Nolen [ 25/Aug/17 2:45 PM ]

fixed https://github.com/clojure/clojurescript/commit/79041d10ce11e9e2f15c261a9a4174c6a7066834





[CLJS-2333] module-deps.js doesn't correctly compute `main` if aliased in browser field Created: 21/Aug/17  Updated: 25/Aug/17  Resolved: 25/Aug/17

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

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

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

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

fixed https://github.com/clojure/clojurescript/commit/88e1f39d5653f154da6b6362bced3c3cb5c15e3b





[CLJS-2332] module_deps.js doesn't process `export from` correctly Created: 18/Aug/17  Updated: 19/Aug/17  Resolved: 19/Aug/17

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

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

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

 Comments   
Comment by David Nolen [ 19/Aug/17 5:42 AM ]

fixed https://github.com/clojure/clojurescript/commit/98656d305e6447c62e36849ee615532d53211fdd





[CLJS-2331] Extend :global-exports to auto-alias and rewrite occurrences of declared globals Created: 18/Aug/17  Updated: 18/Aug/17

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: foreign-libs, global-exports

Approval: Accepted

 Description   

In order to lower the barrier to adopting `:npm-deps` we could push `:global-exports` a bit further. Instead of just using it to support foreign-libs, we can also use it to automatically make libraries node_modules compatible. This can be done by auto generating a namespace alias if not provided and rewriting global access for matching symbols. Some libs may refer to globals without explicit requires and we should warn in that case.






[CLJS-2330] Don't set `"browser"` field for Closure if target is :nodejs Created: 18/Aug/17  Updated: 18/Aug/17  Resolved: 18/Aug/17

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

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

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

 Comments   
Comment by David Nolen [ 18/Aug/17 1:32 PM ]

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





[CLJS-2329] REPL should not error out if quote is missing on string require Created: 18/Aug/17  Updated: 18/Aug/17

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

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


 Description   

At the REPL, the following should not happen I think, cause it feels unidiomatic:

(require "something") ;; there is no quote before the apices

----  Could not Analyze  <cljs form>   line:1  column:1  ----

  Library name must be specified as a symbol in :require / :require-macros; offending spec: ["something"] at line 1 <cljs repl>

  1  (require '"something")
     ^--- 

----  Analysis Error  ----

Minor, solvable with:

(require '"something") ;; quote before the apices





[CLJS-2328] Args are not provided to *main-cli-fn* with optimizations Created: 17/Aug/17  Updated: 18/Aug/17  Resolved: 18/Aug/17

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

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

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

 Description   
(ns example.core)

(defn -main [& argv]
  (enable-console-print)
  (println argv))

(set! *main-cli-fn* -main)

This works when compiled with simple optimizations, and doesn't with advanced optimizations.

Proposed fix is to use nodejs/process instead of js/process here:
https://github.com/clojure/clojurescript/blob/fb90282f41799fe509b143e7870eaf4f9885de41/src/main/cljs/cljs/nodejscli.cljs#L21



 Comments   
Comment by António Nuno Monteiro [ 17/Aug/17 4:22 PM ]

Patch attached with fix.

Comment by Yegor Timoshenko [ 17/Aug/17 4:56 PM ]

Antonio, why not just use cljs.nodejs/process? It uses externs.

Comment by António Nuno Monteiro [ 17/Aug/17 4:59 PM ]

`cljs.nodejs/process` is the same as `js/process`. That wasn't the problem.

`argv` was being munged so we need to access it as a string. Or provide externs for it.

Comment by Yegor Timoshenko [ 17/Aug/17 5:22 PM ]

I see. Thanks!

Comment by David Nolen [ 18/Aug/17 7:58 AM ]

fixed https://github.com/clojure/clojurescript/commit/799325ac84a7cc3acdd68ce98df61328779c63d7





[CLJS-2327] module_deps.js doesn't know about browser field advanced usage Created: 17/Aug/17  Updated: 18/Aug/17  Resolved: 18/Aug/17

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

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

 Comments   
Comment by David Nolen [ 18/Aug/17 7:46 AM ]

fixed https://github.com/clojure/clojurescript/commit/5180e7e2891a92d56b7f1a3f1268e615a96dfd33





[CLJS-2326] Indexing node_modules can't find `main` when it doesn't have an extension Created: 17/Aug/17  Updated: 18/Aug/17  Resolved: 18/Aug/17

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

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

 Description   

This problem is present in both module-deps and cljs.closure/index-node-modules-dir

A notorious example is the `bootstrap` package, which lists `dist/js/bootstrap` in the main entry, where bootstrap is boostrap.js



 Comments   
Comment by David Nolen [ 18/Aug/17 7:46 AM ]

Needs a rebase to master.

Comment by António Nuno Monteiro [ 18/Aug/17 10:57 AM ]

Replaced patch with a rebased version.

Comment by David Nolen [ 18/Aug/17 12:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/694a623018ff2918eba88a9570dbfa685019c6f3





[CLJS-2325] konan cannot handle `export ... from` imports Created: 16/Aug/17  Updated: 16/Aug/17  Resolved: 16/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: modules, npm-deps

Approval: Vetted

 Description   

We use a very small dependency Node.js `konan` to supply import detection. Unfortunately `konan` only handles `require` and `import`, it doesn't check for `export ... from`. Given how trivial this code is we should just probably write our own logic for this in `module_deps.js`.



 Comments   
Comment by David Nolen [ 16/Aug/17 1:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/56a880cb09d57e287a3eba4839c4f5688958850f





[CLJS-2324] module-graph doesn't munge :requires when indexing inputs Created: 15/Aug/17  Updated: 15/Aug/17  Resolved: 15/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: modules

Approval: Accepted

 Description   

This means lookups will fail and deps will get dropped



 Comments   
Comment by David Nolen [ 15/Aug/17 3:33 PM ]

appears fixed with https://github.com/clojure/clojurescript/commit/86b482a0390b2d90d8bee1c88ef248e339e35fcc, but needs test case.

Comment by David Nolen [ 15/Aug/17 3:51 PM ]

added test case https://github.com/clojure/clojurescript/commit/3567cc0155f436617978eb75be08f17ec2811472





[CLJS-2323] Wrong return value for data readers with records Created: 15/Aug/17  Updated: 15/Aug/17  Resolved: 15/Aug/17

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

Type: Defect Priority: Critical
Reporter: Andrew Rudenko Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Patch: Code
Approval: Accepted

 Description   

ClojureScript generates wrong code for data readers which returns records.

Minimal case to reproduce:

dr.cljc
(ns dr)

(defrecord Position [line col])

#?(:cljs (.log js/console #dr/pos {:line 1 :col 5}))
data_readers.cljc
{dr/pos dr/map->Position}
build.clj
(require 'cljs.build.api)

(cljs.build.api/build "." {:output-to "main.js"})

To compile:

java -cp ~/Downloads/cljs.jar:. clojure.main build.clj

It generates

console.log(new cljs.core.PersistentArrayMap(null, 2, [new cljs.core.Keyword(null,"line","line",212345235),(1),new cljs.core.Keyword(null,"col","col",-1959363084),(5)], null));

So, it just ignores record and emit code only for underlying map.



 Comments   
Comment by David Nolen [ 15/Aug/17 12:34 PM ]

This works now in master as of https://github.com/clojure/clojurescript/commit/f5af28f814896d3a538cba717331b651a5ef9239.

We should provide a test case before closing.

Comment by David Nolen [ 15/Aug/17 12:40 PM ]

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





[CLJS-2322] Require only `@cljs-oss/module-deps` to be installed to figure out Node.js dep graph Created: 14/Aug/17  Updated: 15/Aug/17  Resolved: 15/Aug/17

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

Type: Enhancement Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

 Comments   
Comment by David Nolen [ 15/Aug/17 3:51 PM ]

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





[CLJS-2321] Do not automatically call `set-loaded!` on the user's behalf Created: 13/Aug/17  Updated: 13/Aug/17  Resolved: 13/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: cljs.loader, modules

Approval: Vetted

 Description   

The current design prevents users from building functionality on top of `cljs.loader` and introduces many composition issues.



 Comments   
Comment by David Nolen [ 13/Aug/17 3:08 PM ]

Need to update the documentation. Possibly fix some tests, we should also add a docstring to `cljs.loader` marking the alpha status.

Comment by António Nuno Monteiro [ 13/Aug/17 3:18 PM ]

This is probably as easy as reverting CLJS-2157 (commit https://github.com/clojure/clojurescript/commit/15c1eb2d511c2f494400183bab1d9eca611777d6)

Comment by David Nolen [ 13/Aug/17 4:58 PM ]

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





[CLJS-2320] Warn if attempting to load a module which has no entry points which require `cljs.loader` Created: 13/Aug/17  Updated: 13/Aug/17  Resolved: 13/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Declined Votes: 0
Labels: cljs.loader, modules

Approval: Vetted

 Comments   
Comment by David Nolen [ 13/Aug/17 4:44 PM ]

Declined due to CLJS-2321





[CLJS-2319] cljs.core/mod handling of floats inconsistent with Clojure & JavaScript Created: 13/Aug/17  Updated: 15/Aug/17

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

Type: Defect Priority: Minor
Reporter: André Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2319-Fix-cljs.core-mod-handling-of-floats_INLINED.patch     Text File CLJS-2319-Fix-cljs.core-mod-handling-of-floats.patch    

 Description   

The workaround for negative numbers (https://dev.clojure.org/jira/browse/CLJS-417) results in annoying behavior for floats:

(mod 2.1 3) ; => 2.0999999999999996

Both Clojure and the standard JavaScript modulo return the expected 2.1 here.

Two possible solutions:

  • only do the double-mod workaround when the dividend is actually negative
  • check if the dividend is smaller than the divisor and just return it in that case


 Comments   
Comment by David Nolen [ 13/Aug/17 5:00 PM ]

Patch welcome.

Comment by André Wagner [ 14/Aug/17 8:00 AM ]

The patch renames cljs.core/mod to double-mod and redefines mod to invoke js-mod directly when both args have the same sign.
It includes test cases for the previously failing cases, but I've also tested more exhaustively against the Clojure impl: https://gist.github.com/aw7/a32bd69923c65bddc23fd63ee062833c

Comment by David Nolen [ 14/Aug/17 8:46 AM ]

Great thanks, have you submitted your Clojure CA?

Comment by André Wagner [ 14/Aug/17 9:06 AM ]

Yeah, 2h ago.

Comment by António Nuno Monteiro [ 14/Aug/17 5:42 PM ]

Nit: shouldn't the `double-mod` function be marked as private then?

Comment by André Wagner [ 15/Aug/17 4:06 AM ]

I guess there's no need for `double-mod` to exist as a separate function at all, I've added a patch where it's inlined into `mod`.





[CLJS-2318] module-deps.js doesn't respect the package.json `module` field Created: 12/Aug/17  Updated: 13/Aug/17  Resolved: 13/Aug/17

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

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

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

fixed https://github.com/clojure/clojurescript/commit/1163381724c0e8ab11e998e9ab02d7a752848acc





[CLJS-2317] Upgrade Google Closure Library Created: 10/Aug/17  Updated: 11/Aug/17  Resolved: 11/Aug/17

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

Type: Task Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: closure-library

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

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

Can we please remove the whitespace changes in this patch?

Comment by António Nuno Monteiro [ 11/Aug/17 10:31 AM ]

Reverted the whitespace changes in the replaced patch.

Comment by David Nolen [ 11/Aug/17 2:57 PM ]

fixed https://github.com/clojure/clojurescript/commit/92433701acc6f86665b81349dc8c9eb4048ec464





[CLJS-2316] Upgrade Closure Compiler to August release Created: 09/Aug/17  Updated: 11/Aug/17  Resolved: 11/Aug/17

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

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

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

 Comments   
Comment by David Nolen [ 10/Aug/17 6:08 AM ]

I tried to apply but it looks like the zip for this hasn't been uploaded yet.

Comment by António Nuno Monteiro [ 11/Aug/17 10:48 AM ]

Replaced the patch with one that doesn't depend on the dl.google.com link, but instead downloads the Closure Compiler JAR from Maven just like the other dependencies.

Comment by David Nolen [ 11/Aug/17 2:19 PM ]

Patch appears to need a rebase to master.

Comment by David Nolen [ 11/Aug/17 3:22 PM ]

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





[CLJS-2315] module_deps.js can't resolve JSON modules Created: 09/Aug/17  Updated: 11/Aug/17  Resolved: 11/Aug/17

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

Type: Defect Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

 Comments   
Comment by David Nolen [ 10/Aug/17 6:47 AM ]

After applying the patch, running the test for this patch fails.

Comment by António Nuno Monteiro [ 10/Aug/17 11:53 AM ]

Replaced the patch with one that includes files that I had forgotten to stage.

Comment by David Nolen [ 11/Aug/17 8:12 AM ]

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





[CLJS-2314] Eliminate str call on literal strings in str macro Created: 09/Aug/17  Updated: 10/Aug/17  Resolved: 10/Aug/17

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

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

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

 Description   

The str macro unconditionally calls str on each of its arguments. This can be skipped for literal strings, saving on the unnecessary extra runtime call.

For example: (str "x" x "y") would be compiled down to JavaScript with fewer calls ["x",u.c(a),"y"].join("")
and silly things like (str "a" "b") compile all the way down to "ab".



 Comments   
Comment by Mike Fikes [ 09/Aug/17 4:17 PM ]

Hey David, on the surface, this seems like a reasonable change, which could even benefit users targeting :simple or :none environments.

If you'd like to see some benchmarks added to the suite that help justify whether it is worth it, let me know.

Comment by David Nolen [ 10/Aug/17 6:27 AM ]

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





[CLJS-2313] :language-out is a build affecting option Created: 09/Aug/17  Updated: 10/Aug/17  Resolved: 10/Aug/17

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

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: closure, compiler

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

 Description   

We need to add this to the list.



 Comments   
Comment by David Nolen [ 10/Aug/17 6:26 AM ]

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





[CLJS-2312] Miss-compile: Uncaught SyntaxError: Unexpected token default Created: 09/Aug/17  Updated: 11/Aug/17  Resolved: 11/Aug/17

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

Type: Defect Priority: Major
Reporter: Dieter Komendera Assignee: António Nuno Monteiro
Resolution: Completed Votes: 0
Labels: compiler

Attachments: Text File CLJS-2312.patch     Zip Archive default-demo2.zip     Zip Archive defaults-demo.zip    
Patch: Code
Approval: Vetted

 Description   

This is a regression since 1.9.854.

This:

(ns foo.main
  (:require [cljs.js :as cljs]
            [cljs.tools.reader :as r]
            [cljs.analyzer :refer [*cljs-warning-handlers*]]
            [cljs.tools.reader.reader-types :as rt]
            [clojure.string :as string]))

(defn foo [default]
  (js/console.log default))

(foo "bar")

compiles to

goog.provide('foo.main');
goog.require('cljs.core');
goog.require('cljs.js');
goog.require('cljs.tools.reader');
goog.require('cljs.analyzer');
goog.require('cljs.tools.reader.reader_types');
goog.require('clojure.string');
foo.main.foo = (function foo$main$foo(default$){
return console.log(default);
});
foo.main.foo.call(null,"bar");

Which results in Uncaught SyntaxError: Unexpected token default at runtime.

The compiler itself is also affected:
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L500

When removing all the requires in the test case, it compiles fine.

Attaching a test case.



 Comments   
Comment by Dieter Komendera [ 09/Aug/17 9:18 AM ]

I was wrong in my initial assumption that this has anything to do with requiring stuff from the cljs* namespace, instead this happens with :language-out set to es5.

Comment by David Nolen [ 10/Aug/17 6:07 AM ]

I don't really understand the provided patch. It could use a description (on the commit itself). It appears to revert the default changes and then only avoids munging if the var has a namespace?

Comment by António Nuno Monteiro [ 11/Aug/17 10:50 AM ]

The patch has been updated with an inline comment detailing the proposed solution.

Comment by David Nolen [ 11/Aug/17 2:16 PM ]

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





[CLJS-2311] Cut a Closure Library release Created: 08/Aug/17  Updated: 10/Aug/17  Resolved: 10/Aug/17

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

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

Approval: Accepted

 Comments   
Comment by David Nolen [ 10/Aug/17 6:57 AM ]

A new Closure Library release has been cut.





[CLJS-2310] should not emit goog.require for foreign-libs under :modules Created: 08/Aug/17  Updated: 13/Aug/17  Resolved: 13/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: modules

Approval: Screened

 Description   

ModuleManager will attempt to eval the module but will encounter goog.require("foreign-lib") which will fail. This patch requires a test.



 Comments   
Comment by David Nolen [ 09/Aug/17 7:54 AM ]

This issue is not quite a simple as it sounds. ModuleManager doesn't appear to know what namespaces it is loading, it only knows what files compose a module. The sources of a module will be loaded via XHR, concatenated and then eval'ed. This is why a statement like goog.require("cljsjs.react") will fail, since that namespace is never actually provided.

However, for modules loaded via script tags on the page we cannot elide goog.require for foreign libs.

This seems to suggest we may want to load modules uniformly? That is, under :none, :output-to files (with entry points using cljs.loader) should perhaps use cljs.loader to load the module. There are some alternatives that are also probably worth consideration.

Comment by David Nolen [ 11/Aug/17 4:42 PM ]

Using cljs.loader to load the module in the :output-to file introduces a bunch of other problems as far as I can tell, likely related to loading files a second time. Instead :output-to files should load foreign libs separately via goog.require before everything else.

Comment by David Nolen [ 13/Aug/17 4:48 PM ]

fixed https://github.com/clojure/clojurescript/commit/8bdcc9a4dc0b011d89886cea6ea7bed92d435ea8, but missing the test, due to some relative path issues, I don't have time to look at the moment.





[CLJS-2309] :module foreign-libs order not preserved Created: 08/Aug/17  Updated: 16/Aug/17  Resolved: 16/Aug/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: modules

Attachments: Text File CLJS-2309.patch    
Patch: Code and Test
Approval: Vetted

 Description   

It appears foreign-libs order is not preserved. Since these do not provide real Closure require/provide information it is important they get added to their respective module in the correct order. Patch should provide a test case.



 Comments   
Comment by António Nuno Monteiro [ 13/Aug/17 5:49 PM ]

Attached a patch with a test that demonstrates the issue is no longer reproducible.

Comment by David Nolen [ 14/Aug/17 8:03 AM ]

Test patch applied https://github.com/clojure/clojurescript/commit/0b8cff8027fa7fc5b2d325df0206ba90d780f7a6. Keeping the issue open for now.

Comment by David Nolen [ 15/Aug/17 5:42 PM ]

Got a repro from Tony Kay today. This is definitely an issue. I suspect due to the fact that we don't dep sort after adding preloads.

Comment by Tony Kay [ 15/Aug/17 6:09 PM ]

I tested both :simple and :advanced optimizations against my larger project (both were failing before). The tip of master (905) is working for me now.

Comment by David Nolen [ 16/Aug/17 12:21 AM ]

fixed in master

Comment by David Nolen [ 16/Aug/17 12:22 AM ]

was fixed by https://github.com/clojure/clojurescript/commit/6130143bbc07e49c3d6e9313377226b9551be434





[CLJS-2308] Node: process.env get overwritten in 19.854 Created: 07/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

Type: Defect Priority: Critical
Reporter: Jan Hein Hoogstad Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

node (tested both 6.10 and 8.2.1)



 Description   

In the latest cljs-version process.env seems to get completely overwritten and only {"NODE_ENV":"production"} is set. All other env variables are missing. In 1.9671 this was still behaving as expected.



 Comments   
Comment by António Nuno Monteiro [ 07/Aug/17 5:52 PM ]

Duplicate of CLJS-2302





[CLJS-2307] Closure warns on unreachable checked array code Created: 07/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

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

 Description   

The checked-aget and checked-aset code involves a try / catch where the entire body involves {{assert}}s. Thus when asserts are elided and Closure also sees the resulting code it looks roughly like code you'd get for

(try
  nil
  nil
  (catch :default e ...))

and Closure warns that the code in the catch block is unreachable:

Aug 07, 2017 2:17:03 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: .../cljs/core.js:541: WARNING - unreachable code
}catch (e11022){var e_11027 = e11022;
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Aug 07, 2017 2:17:03 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: .../cljs/core.js:595: WARNING - unreachable code
}catch (e11034){var e_11039 = e11034;
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


 Comments   
Comment by Mike Fikes [ 07/Aug/17 2:32 PM ]

While it would be possible to fix this by slightly abusing assert itself by wrapping the entire body of code in question in an assert form, and plugging in true in all the form exit points, the attached patch takes a more direct approach by simply introducing a private macro when-assert that takes a single form and emits it or elides it depending on *assert*.

Comment by David Nolen [ 07/Aug/17 8:58 PM ]

fixed https://github.com/clojure/clojurescript/commit/02c6d2a706d6668b007c2a186f602b498cfaaa65





[CLJS-2306] Provide better warning message when namespace can't be found Created: 07/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

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

 Description   

Currently the message includes `or JavaScript source providing ""`, which can be confusing



 Comments   
Comment by David Nolen [ 07/Aug/17 9:01 PM ]

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





[CLJS-2305] Tests: Unable to resolve symbol: opts in this context Created: 07/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   
$ script/test
Exception in thread "main" java.lang.RuntimeException: Unable to resolve symbol: opts in this context, compiling:(cljs/closure.clj:2025:12)
	at clojure.lang.Compiler.analyze(Compiler.java:6748)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3848)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6948)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3855)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6948)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2805)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6946)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:6056)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:6376)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6946)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6934)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:6056)
	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5428)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3993)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6944)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6934)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.access$300(Compiler.java:38)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:595)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6946)
	at clojure.lang.Compiler.analyze(Compiler.java:6729)
	at clojure.lang.Compiler.analyze(Compiler.java:6685)
	at clojure.lang.Compiler.eval(Compiler.java:7009)
	at clojure.lang.Compiler.load(Compiler.java:7457)
	at clojure.lang.RT.loadResourceScript(RT.java:374)
	at clojure.lang.RT.loadResourceScript(RT.java:365)
	at clojure.lang.RT.load(RT.java:455)
	at clojure.lang.RT.load(RT.java:421)
	at clojure.core$load$fn__6368.invoke(core.clj:6008)
	at clojure.core$load.invokeStatic(core.clj:6007)
	at clojure.core$load.doInvoke(core.clj:5991)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invokeStatic(core.clj:5812)
	at clojure.core$load_one.invoke(core.clj:5807)
	at clojure.core$load_lib$fn__6313.invoke(core.clj:5852)
	at clojure.core$load_lib.invokeStatic(core.clj:5851)
	at clojure.core$load_lib.doInvoke(core.clj:5832)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$load_libs.invokeStatic(core.clj:5889)
	at clojure.core$load_libs.doInvoke(core.clj:5873)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$require.invokeStatic(core.clj:5911)
	at clojure.core$require.doInvoke(core.clj:5911)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval133.invokeStatic(cljsc.clj:9)
	at user$eval133.invoke(cljsc.clj:9)
	at clojure.lang.Compiler.eval(Compiler.java:7005)
	at clojure.lang.Compiler.load(Compiler.java:7457)
	at clojure.lang.Compiler.loadFile(Compiler.java:7395)
	at clojure.main$load_script.invokeStatic(main.clj:277)
	at clojure.main$script_opt.invokeStatic(main.clj:337)
	at clojure.main$script_opt.invoke(main.clj:332)
	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: java.lang.RuntimeException: Unable to resolve symbol: opts in this context
	at clojure.lang.Util.runtimeException(Util.java:221)
	at clojure.lang.Compiler.resolveIn(Compiler.java:7242)
	at clojure.lang.Compiler.resolve(Compiler.java:7186)
	at clojure.lang.Compiler.analyzeSymbol(Compiler.java:7147)
	at clojure.lang.Compiler.analyze(Compiler.java:6708)
	... 69 more

Bisect:

4f8dde57e01b5f3b23bf454e6c414896cf806e78 is the first bad commit
commit 4f8dde57e01b5f3b23bf454e6c414896cf806e78
Author: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Date:   Sat Aug 5 15:16:30 2017 -0700

    CLJS-2302: Disable process-shim by default in Node.js targets

:040000 040000 eded6b164347d85da8a2703f74c198ab79c7f892 0d5fd0f7618919d60f7abb6c6e6f602ed0a7bc50 M	src


 Comments   
Comment by David Nolen [ 07/Aug/17 9:10 AM ]

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





[CLJS-2304] Fix compiler infrastructure tests on Windows Created: 06/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

Attachments: Text File CLJS-2304.patch    

 Comments   
Comment by David Nolen [ 07/Aug/17 7:13 AM ]

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





[CLJS-2303] Disable duplicate alias checking for self-host Created: 05/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

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

 Description   

Because self-host has its own logic for `clojure` -> `cljs` namespace aliasing, the following snippet fails:

(require 'clojure.test)
(require 'clojure.test)
Alias clojure.test already exists in namespace cljs.user, aliasing cljs.test


 Comments   
Comment by António Nuno Monteiro [ 05/Aug/17 9:30 PM ]

The patch for this ticket should be applied after CLJS-2266, or it will demonstrate that issue.

Comment by David Nolen [ 07/Aug/17 7:19 AM ]

Patch needs to be rebased on master.

Comment by David Nolen [ 07/Aug/17 8:59 PM ]

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





[CLJS-2302] Disable process-shim by default in Node.js targets Created: 05/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

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

 Description   

We no longer process JS modules by default under Node.js, so we shouldn't be setting the process shim by default.

Users can still specify `:process-shim true` in this case if they really want to.

Otherwise we're running the risk of overwriting the whole `process` variable in user's code.



 Comments   
Comment by David Nolen [ 07/Aug/17 7:15 AM ]

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

Comment by Mike Fikes [ 07/Aug/17 8:28 AM ]

See regression CLJS-2305





[CLJS-2301] Avoid use of deprecated goog.string/isEmptySafe in clojure.string/blank? Created: 05/Aug/17  Updated: 05/Aug/17

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

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

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

 Description   

clojure.string/blank? calls goog.string/isEmptySafe, which is marked as deprecated. Instead this can be inlined with the code that this internally calls, which is non-deprecated code. Also, it can be seen that such a change has no effect on perf, with these benchmarking tests tried out:

Before:

Benchmarking with V8
[s nil f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 28 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 26 msecs
Benchmarking with SpiderMonkey
[s nil f clojure.string/blank?], (f s), 1000000 runs, 265 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 268 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 268 msecs
Benchmarking with JavaScriptCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s nil f clojure.string/blank?], (f s), 1000000 runs, 331 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 336 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 59 msecs
Benchmarking with ChakraCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 56 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 56 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 57 msecs


After:

Benchmarking with V8
[s nil f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 26 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
Benchmarking with SpiderMonkey
[s nil f clojure.string/blank?], (f s), 1000000 runs, 262 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 262 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 261 msecs
Benchmarking with JavaScriptCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s nil f clojure.string/blank?], (f s), 1000000 runs, 328 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 324 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 60 msecs
Benchmarking with ChakraCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 62 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 63 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 62 msecs
Lindas-iMac-2:clojurescript mfikes$ # Using or nil?


 Comments   
Comment by Mike Fikes [ 05/Aug/17 11:37 AM ]

The change in the attached patch was used to produce the benchmarks in the ticket description.

I also tried an alternative which uses a Clojure or and a nil? check in lieu of the goog.string/makeSafe call, and this resulted in the same benchmark numbers.

So, the patch goes with the recommended deprecation change in the documentation for goog.string/isEmptyOrWhitespaceSafe, which indicates "Use goog.string.isEmptyOrWhitespace(goog.string.makeSafe(str)) instead.", which exactly matches the code we are indirectly calling today.





[CLJS-2300] Delegate clojure.string/capitalize to goog.string/capitalize Created: 04/Aug/17  Updated: 05/Aug/17

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

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

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

 Description   

Since the implementation of clojure.string/capitalize (6 years ago), Closure library added an identical function (3 years ago), which is actually much faster:

cljs.user=> (simple-benchmark [s "abc"] (goog.string/capitalize s) 10000000)
[s "abc"], (goog.string/capitalize s), 10000000 runs, 1181 msecs
nil
cljs.user=>  (simple-benchmark [s "abc"] (clojure.string/capitalize s) 10000000)
[s "abc"], (clojure.string/capitalize s), 10000000 runs, 5295 msecs

We could just have ClojureScript's delegate to Closure's.



 Comments   
Comment by Mike Fikes [ 04/Aug/17 9:17 PM ]

Before/After tests look promising:

Before:

Benchmarking with V8
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 166 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 380 msecs
Benchmarking with SpiderMonkey
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 833 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 1541 msecs
Benchmarking with JavaScriptCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 53 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 438 msecs
Benchmarking with Nashorn
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 903 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 1181 msecs
Benchmarking with ChakraCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 345 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 727 msecs

After:

Benchmarking with V8
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 28 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 28 msecs
Benchmarking with SpiderMonkey
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 263 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 263 msecs
Benchmarking with JavaScriptCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 5 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 327 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 327 msecs
Benchmarking with ChakraCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 57 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 63 msecs
Comment by Mike Fikes [ 04/Aug/17 10:53 PM ]

The benchmarks produced in the previous comment are with included in the attached patch.

The patch also includes a generative regression test, using the previous implementation as a reference.

All engines pass the test fine if you crank up the number of values tested, except for ChakraCore. ChakraCore appears to be buggy, and I'll follow up upstream with a ticket for them. At its core, if you capitalize "ß" you should get back "SS". With the original ClojureScript implementation, you get back "ﮰ" on ChakraCore, and with the revision in the patch (essentially Closure Library) you get back "鯪", neither of which are correct. So, this patch doesn't introduce a regression on ChakraCore.

Comment by Mike Fikes [ 05/Aug/17 11:59 AM ]

ChakraCore bug: https://github.com/Microsoft/ChakraCore/issues/421





[CLJS-2299] Failure with alias and bad require of clojure.spec Created: 04/Aug/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

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

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

 Description   

Try these two forms in succession with a QuickStart JAR node REPL:

(require '[clojure.spec :as s])
(require '[clojure.spec.alpha :as s])

The second will cause an error clojure.lang.ExceptionInfo: Alias s already exists in namespace cljs.user, aliasing clojure.spec

This does not occur with 1.9.671



 Comments   
Comment by António Nuno Monteiro [ 05/Aug/17 5:51 PM ]

Patch attached with fix.

Summary of the fix:

When we moved from the REPL special `require` last year to the new `:ns*` require, we stopped backing up the compiler state for requires and imports. The patch adds backing up of the compiler state and restores it if an analysis error occurs.

Comment by David Nolen [ 07/Aug/17 7:18 AM ]

fixed https://github.com/clojure/clojurescript/commit/870d8731014c3cfe56539580eefd5671437fde36





[CLJS-2298] REPLs should automatically load user.(cljs|cljc) files at root of Java classpath Created: 04/Aug/17  Updated: 22/Sep/17

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Sameer Rahmani
Resolution: Unresolved Votes: 0
Labels: repl

Approval: Vetted




[CLJS-2297] cljs.nodejs env incorrectly returns "nil" (the string) for missing values Created: 03/Aug/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

Type: Defect Priority: Major
Reporter: lvh Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

{ > (aget (.-env nodejs/process) "HOME") "\"/Users/lvh\"" > (aget (.-env nodejs/process) "DOES_NOT_EXIST") "nil" }

with goog.object/get:

{ > (goog.object/get (.-env nodejs/process) "HOME") "\"/Users/lvh\"" > (goog.object/get (.-env nodejs/process) "DOES_NOT_EXIST") "nil" }

for reference, plain node returns undefined:

{ > typeof process.env["doesnotexist"] 'undefined' }



 Comments   
Comment by lvh [ 03/Aug/17 5:15 PM ]

Invalid. Was a repl tooling bug.





[CLJS-2296] Foreign libs that expose modules are not being processed under target node Created: 03/Aug/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, nodejs

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

 Comments   
Comment by David Nolen [ 03/Aug/17 4:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/3057bbeeaea3adf04c25d4cbfa417bf0e4a3848a





[CLJS-2295] `index-node-modules-dir` can't determine :main for package.json files that have `.` in the string Created: 02/Aug/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

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

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

 Description   

Example: jss-extend



 Comments   
Comment by David Nolen [ 03/Aug/17 6:42 AM ]

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





[CLJS-2294] Options checkers presume :optimizations is always set Created: 02/Aug/17  Updated: 09/Sep/17

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

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

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

 Description   

There are some opts checkers that

check-preloads is called with opts with implicit opts added, so in that case :optimizations is always set: https://dev.clojure.org/jira/browse/CLJS-1996

Should other checkers be called with all-opts also?
Or should the checkers be fixed to check if the optimizations key is not available?



 Comments   
Comment by Juho Teperi [ 02/Aug/17 9:36 AM ]

I'm thinking build should call add-implicit-opts the first thing, and just bind that to opts so everything uses the same opts? No reason to have original opts and opts with defaults added separately?

Comment by Juho Teperi [ 02/Aug/17 10:06 AM ]

This patch removes all-opts and instead calls the add-implicit-opts the first thing in cljs.closure/build.

Comment by David Nolen [ 03/Aug/17 4:23 PM ]

I mean I'm pretty sure I'm OK with but can we please explain briefly what problem this is solving that you encountered?

Comment by Juho Teperi [ 03/Aug/17 4:28 PM ]

Right, the problem I encountered was that setting :source-map true without :optimizations option causes error. If :optimizations is nil, check-source-map goes to the branch that validates source-map options with optimizations enabled.

(cljs.closure/build
  "src"
  {:main "hello-world.core"
   :output-to "out/main.js"
   :output-dir "out"
   ; works if uncommented
   ; :optimizations :none
   :source-map true})


Exception in thread "main" java.lang.AssertionError: Assert failed: :source-map true must specify a file in the same directory as :output-to "out/main.js" if optimization setting applied
(or (nil? (:output-to opts)) (:modules opts) (string? source-map)), compiling:(/home/juho/tmp/problems/build.clj:3:1)
Comment by Juho Teperi [ 09/Sep/17 11:47 AM ]

Fixed patch to ensure it can be applied.





[CLJS-2293] Self-host: Can't load cljs.js owing to set alias Created: 02/Aug/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

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

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

 Description   

On master with script/noderepljs:

cljs.user=> (require 'cljs.js)
WARNING: No such namespace: set, could not locate set.cljs, set.cljc, or JavaScript source providing "" at line 361 /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/compiler.cljc
WARNING: Use of undeclared Var set/difference at line 361 /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/compiler.cljc
nil


 Comments   
Comment by David Nolen [ 03/Aug/17 6:44 AM ]

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





[CLJS-2292] Var being replaced warnings with :refer-clojure :rename Created: 01/Aug/17  Updated: 01/Aug/17

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

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


 Description   

If you rename a core Var, you will get a warning if you redefine it. It is as if the :rename doesn't imply :exclude.

Repro with QuickStart JAR:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 52319
To quit, type: :cljs/quit
cljs.user=> (ns foo.core (:refer-clojure :rename {map clj-map}))
nil
foo.core=> (map inc [1 2])
(2 3)
foo.core=> (def map {:a :b})
WARNING: map already refers to: cljs.core/map being replaced by: foo.core/map at line 1 <cljs repl>
#'foo.core/map
foo.core=> map
{:a :b}
foo.core=> (clj-map inc [1 2])
(2 3)

Compare to Clojure:

user=> (ns foo.core (:refer-clojure :rename {map clj-map}))
nil
foo.core=> (map inc [1 2])

CompilerException java.lang.RuntimeException: Unable to resolve symbol: map in this context, compiling:(/private/var/folders/gx/nymj3l7x4zq3gxb97v2zwzb40000gn/T/form-init8370940485091648461.clj:1:1)
foo.core=> (def map {:a :b})
#'foo.core/map
foo.core=> map
{:a :b}
foo.core=> (clj-map inc [1 2])
(2 3)

Note that you cannot workaround this by simply adding an explicit :exclude for map above. While this works with the current ClojureScript compiler, it breaks in Clojure, making the alias symbol clj-map unresolvable.






[CLJS-2291] Set up Windows CI Created: 01/Aug/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

Type: Task Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: test
Environment:

Windows


Attachments: Text File CLJS-2291.patch     Text File CLJS-2291-rebased.patch    

 Comments   
Comment by David Nolen [ 03/Aug/17 6:43 AM ]

This patch needs to be rebased to master.

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

Attached rebased patch.

Comment by David Nolen [ 03/Aug/17 4:13 PM ]

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





[CLJS-2290] Node packages using require('assert') fail compilation Created: 31/Jul/17  Updated: 01/Aug/17  Resolved: 01/Aug/17

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

Type: Defect Priority: Critical
Reporter: Will Cohen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

Attachments: Text File CLJS-2290.patch    
Patch: Code and Test
Approval: Vetted

 Description   

When including packages that contain "require('assert')" as a node dependency – for example, https://github.com/uber/react-map-gl – Clojurescript fails to compile the module with the following error (note that importing react-map-gl currently requires closure compiler master because Mapbox GL has dependencies which require the fix from https://github.com/google/closure-compiler/pull/2579):

WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/react-map-gl/dist/utils/map-state.js line 14 : 4
WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/react-map-gl/dist/utils/map-state.js line 14 : 14
WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/viewport-mercator-project/dist/perspective-mercator-viewport.js line 24 : 14
WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/viewport-mercator-project/dist/perspective-mercator-viewport.js line 24 : 4
WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/viewport-mercator-project/dist/viewport.js line 52 : 14
WARNING: JSC_JS_MODULE_LOAD_WARNING. Failed to load module "assert" at [...]/node_modules/viewport-mercator-project/dist/viewport.js line 52 : 4

For reference, the relevant js files:
https://unpkg.com/react-map-gl@3.0.1/dist/utils/map-state.js
https://unpkg.com/viewport-mercator-project@4.1.1/dist/perspective-mercator-viewport.js
https://unpkg.com/viewport-mercator-project@4.1.1/dist/viewport.js

Naively adding https://github.com/defunctzombie/commonjs-assert to package.json or :npm-deps alone does not resolve the issue.



 Comments   
Comment by António Nuno Monteiro [ 31/Jul/17 10:38 PM ]

Attached patch with fix.

Comment by David Nolen [ 01/Aug/17 7:19 AM ]

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





[CLJS-2289] Node packages with browser entry don't work with Closure Created: 31/Jul/17  Updated: 15/Sep/17  Resolved: 15/Sep/17

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

Type: Enhancement Priority: Major
Reporter: Juho Teperi Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

Approval: Vetted

 Description   

Closure doesn't yet support package.json browser field: https://github.com/google/closure-compiler/issues/2433

However, our, module_deps.js / brower-resolve, and use this field.

One package that has problems is https://github.com/PaulLeCam/react-leaflet which depends on https://github.com/BerkeleyTrue/warning which uses browser entry.
index-node-modules-dir reuturns a list of all JS files in node_modules, this includes both warning/warning.js and warning/browser.js. index-node-modules only includes warning/browser.js. Because Closure doesn't understand package.json browser field, it can't resolve the warning require when processing React-leaflet: https://github.com/PaulLeCam/react-leaflet/blob/master/lib/Pane.js#L47

(cljs.closure/index-node-modules-dir)
;; =>
{:file "/home/juho/Source/x/y/node_modules/warning/warning.js", :module-type :es6, :provides ["warning/warning.js" "warning/warning" "warning"]}
{:file "/home/juho/Source/x/y/node_modules/warning/browser.js", :module-type :es6, :provides ["warning/browser.js" "warning/browser"]}
(cljs.closure/index-node-modules ["react-leaflet"])
;; =>
{:file "/home/juho/Source/x/y/node_modules/warning/browser.js", :module-type :es6, :provides ["warning" "warning/browser.js" "warning/browser"]}


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

This is now fixed in Closure Compiler master (https://github.com/google/closure-compiler/pull/2598).





[CLJS-2288] Fix tests failing on Windows due to platform specific file separators Created: 31/Jul/17  Updated: 11/Aug/17  Resolved: 11/Aug/17

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

Type: Defect Priority: Major
Reporter: Oliver George Assignee: Oliver George
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

Windows


Attachments: Text File CLJS-2288-2.patch     Text File CLJS-2288-cr1.patch    

 Description   

There are tests which fail on Windows due to assumptions about the file separator used. We can get these test working on windows by avoiding the assumption.



 Comments   
Comment by Oliver George [ 31/Jul/17 6:04 PM ]

Sample of fixes and FIXMEs needed to get cljs.closure-tests working on Windows.

Provided for feedback. Not ready for merging.

Comment by David Nolen [ 01/Aug/17 6:36 AM ]

I would expect (io/file "foo" "bar.js") not (io/file "foo/bar.js").

Comment by Juho Teperi [ 01/Aug/17 11:06 AM ]

:provides values should always use /.

Proper solution is probably to fix index-node-modules-dir to replace }} with {{/. Similar to how module_deps.js works.

I attached a patch which should fix this, but I don't have acccess to Windows just now. The changes to test-cljs-1882 looked correct, so I included that in this patch.

Comment by Oliver George [ 02/Aug/17 5:51 PM ]

I think we're parking this ticket for now. Could be closed or revisited once the work on CLJS-2291 is committed.

Comment by David Nolen [ 11/Aug/17 3:08 PM ]

Resolved in master.





[CLJS-2287] Self-host: `require` prints result of loading node deps / global exports Created: 30/Jul/17  Updated: 01/Aug/17  Resolved: 01/Aug/17

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 30/Jul/17 11:43 PM ]

The attached patch emits `null` when `:def-emits-var` is true, since there's no `:repl-env` in bootstrap.

We could also make it so that the patch always emits `null`, but that would result in potentially extraneous code being emitted if not at the REPL.

Comment by David Nolen [ 01/Aug/17 7:22 AM ]

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





[CLJS-2286] Simplify JS module processing Created: 30/Jul/17  Updated: 01/Aug/17  Resolved: 01/Aug/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

Attachments: Text File CLJS-2286.patch    
Patch: Code and Test
Approval: Vetted

 Description   

After CLJS-2279, we're setting `setProcessCommonJSModules` to `true` in ES6 module processing. If we now set `setTransformAMDToCJSModules` to be `true` if any of the provided module is an AMD module, we can simplify the way we process JS modules currently by getting rid of the multimethod entirely and just reuse the current ES6 implementation for every kind of module.

This has the added benefit of allowing dependencies between different module types, which was previously not possible.



 Comments   
Comment by David Nolen [ 01/Aug/17 7:43 AM ]

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





[CLJS-2285] Relative path exception thrown on Windows using :preload Created: 30/Jul/17  Updated: 15/Aug/17

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

Type: Defect Priority: Major
Reporter: Oliver George Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Windows


Attachments: Text File CLJS-2285b.patch     Text File CLJS-2285.patch    
Patch: Code

 Description   

Seems like a small bug in appending "/" in the strip-user-dir helper. This code was recently updated.

Steps to repeat from CLJS-2036 can be used. I think it boils down to:

1. Add a :preload namespace to the build
2. Require a foreign lib in the preload ns

Expect: Builds fine

Actual: `IllegalArgumentException: C:\Repos\canidep\src\uilib.js is not a relative path` full exception below.

Works on 1.9.671.

Fails on 1.9.854.

Exception in thread "main" java.lang.IllegalArgumentException: C:\Repos\canidep\src\uilib.js is not a relative path, compiling:(C:\Repos\canidep\scripts\build.clj:36:1)
        at clojure.lang.Compiler.load(Compiler.java:7391)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$script_opt.invokeStatic(main.clj:335)
        at clojure.main$script_opt.invoke(main.clj:330)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:93)
        at clojure.lang.Reflector.invokeStaticMethod(Reflector.java:207)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:93)
        at clojure.lang.Reflector.invokeStaticMethod(Reflector.java:207)
        at user$eval5.invokeStatic(form-init8570637412295703479.clj:1)
        at user$eval5.invoke(form-init8570637412295703479.clj:1)
        at clojure.lang.Compiler.eval(Compiler.java:6927)
        at clojure.lang.Compiler.eval(Compiler.java:6917)
        at clojure.lang.Compiler.load(Compiler.java:7379)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$init_opt.invokeStatic(main.clj:277)
        at clojure.main$init_opt.invoke(main.clj:277)
        at clojure.main$initialize.invokeStatic(main.clj:308)
        at clojure.main$null_opt.invokeStatic(main.clj:342)
        at clojure.main$null_opt.invoke(main.clj:339)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:421)
        at clojure.lang.Var.invoke(Var.java:383)
        at clojure.lang.AFn.applyToHelper(AFn.java:156)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: C:\Repos\canidep\src\uilib.js is not a relative path
        at clojure.java.io$as_relative_path.invokeStatic(io.clj:414)
        at clojure.java.io$file.invokeStatic(io.clj:426)
        at clojure.java.io$file.invoke(io.clj:418)
        at cljs.closure$write_javascript.invokeStatic(closure.clj:1698)
        at cljs.closure$write_javascript.invoke(closure.clj:1691)
        at cljs.closure$source_on_disk.invokeStatic(closure.clj:1740)
        at cljs.closure$source_on_disk.invoke(closure.clj:1735)
        at cljs.closure$build$fn__6925.invoke(closure.clj:2536)
        at clojure.core$map$fn__4785.invoke(core.clj:2646)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:49)
        at clojure.lang.Cons.next(Cons.java:39)
        at clojure.lang.RT.next(RT.java:688)
        at clojure.core$next__4341.invokeStatic(core.clj:64)
        at clojure.core$dorun.invokeStatic(core.clj:3033)
        at clojure.core$doall.invokeStatic(core.clj:3039)
        at clojure.core$doall.invoke(core.clj:3039)
        at cljs.closure$build.invokeStatic(closure.clj:2536)
        at cljs.closure$build.invoke(closure.clj:2444)
        at cljs.build.api$build.invokeStatic(api.clj:205)
        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$eval8894.invokeStatic(build.clj:36)
        at user$eval8894.invoke(build.clj:36)
        at clojure.lang.Compiler.eval(Compiler.java:6927)
        at clojure.lang.Compiler.load(Compiler.java:7379)
        ... 42 more


 Comments   
Comment by Oliver George [ 30/Jul/17 5:24 AM ]

The problem is highlighted by this:

In cljs.util/relative-name the user-path uses "/" as the separator

(.getFile (.toURL (io/file (System/getProperty "user.dir"))))

=> "/C:/Repos/canidep/"

But the code which appends "/" if it's missing uses File/separator

File/separator

=> "\\"
Comment by Oliver George [ 30/Jul/17 8:27 PM ]

I prefer the patch on CLJS-2221 to solve this problem.

Comment by Oliver George [ 30/Jul/17 8:35 PM ]

Make test for util/relative-path platform specific. Test specific Windows
behaviour.

Does not fix bug.

Comment by Inge Solvoll [ 15/Aug/17 7:10 AM ]

Something similar happens when following this tutorial https://ajpierce.github.io/posts/react-figwheel-npm-2/

When starting figwheel, I get this stacktrace:

java.lang.IllegalArgumentException: c:\dev\splort\node_modules\react-dom\index.js is not a relative path
at clojure.java.io$as_relative_path.invokeStatic (io.clj:414)
clojure.java.io$file.invokeStatic (io.clj:426)
clojure.java.io$file.invoke (io.clj:418)
cljs.closure$write_javascript.invokeStatic (closure.clj:1698)
cljs.closure$write_javascript.invoke (closure.clj:1691)
cljs.closure$process_js_modules$fn__15415.invoke (closure.clj:2348)

Is this the same entry point to the error, or does it need a separate issue?





[CLJS-2284] Fix build API tests not to pollute `out` in the current directory Created: 29/Jul/17  Updated: 30/Jul/17  Resolved: 30/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 30/Jul/17 8:04 AM ]

fixed https://github.com/clojure/clojurescript/commit/97459100a9c71c74a585a4de523ec4a6e3f4dc7c





[CLJS-2283] Regression with js-obj and gobject alias Created: 29/Jul/17  Updated: 29/Jul/17  Resolved: 29/Jul/17

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

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: regression

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

 Description   

If you use js-obj without a string key it can throw. Repro with shipping cljs.jar:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 50933
To quit, type: :cljs/quit
cljs.user=> (js-obj :a 4)
WARNING: No such namespace: gobject, could not locate gobject.cljs, gobject.cljc, or JavaScript source providing "" at line 1 <cljs repl>
WARNING: Use of undeclared Var gobject/set at line 1 <cljs repl>
repl:18
throw e__6186__auto__;
^

ReferenceError: gobject is not defined
    at repl:3:1
    at repl:6:3
    at repl:14:3
    at repl:19: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)
    at emitOne (events.js:115:13)


 Comments   
Comment by Mike Fikes [ 29/Jul/17 8:24 AM ]

Workaround: (require '[goog.object :as gobject])

Comment by David Nolen [ 29/Jul/17 8:28 AM ]

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





[CLJS-2282] Some valid keywords are strings in JS object literals Created: 29/Jul/17  Updated: 29/Jul/17  Resolved: 29/Jul/17

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

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

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

 Description   

Some property names are represented as strings when they would be perfectly valid keywords.

Examples

  • #js {:_abc 1} is printed as #js {"_abc" 1}
  • #js {:*compiler* 1} is printed as #js {"*compiler*" 1}


 Comments   
Comment by Mike Fikes [ 29/Jul/17 7:34 AM ]

The attached patch revises the regex to add to the set of allowed initial characters.

Comment by David Nolen [ 29/Jul/17 8:29 AM ]

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





[CLJS-2281] module_deps.js cannot compute inputs for ES6 sources Created: 28/Jul/17  Updated: 30/Jul/17  Resolved: 30/Jul/17

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

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

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

 Description   

Currently module_deps.js cannot follow requires for ES6 sources because it expects that it will parse CommonJS. module-deps supports transforms prior to CommonJS require resolution for this specific reason. We should leverage this functionality. A good benchmark is if we can compile Google's fully Closure compatible Material Components project without issue.



 Comments   
Comment by Jeremy Schoffen [ 29/Jul/17 10:27 AM ]

Hello, here is a minimal repo concerning this issue, hope this helps testing.

https://github.com/JeremS/class-test-npm

Cheers,

Comment by António Nuno Monteiro [ 29/Jul/17 7:03 PM ]

Attached patch with fix.

Unfortunately `module-deps`'s transforms are not applied to files in `node_modules`. As per the README: "When you call mdeps() with an opts.transform, the transformations you specify will not be run for any files in node_modules/. This is because modules you include should be self-contained and not need to worry about guarding themselves against transformations that may happen upstream."

The attached patch employs a different strategy. I forked `module-deps` to https://github.com/cljs-oss/module-deps which includes the changes in this pull request: https://github.com/substack/module-deps/pull/63, allowing us to use konan (https://www.npmjs.com/package/konan) instead of `detective` (https://www.npmjs.com/package/detective) to parse both ES6 and CommonJS imports.

This way, we have a minimal patch on top of `module-deps` and don't need to worry about writing transformations that could prove buggy, relying instead on a tested library that parses both imports and ES6. We can always change to the original module-deps if the aforementioned PR is merged.

The repo linked in the comment above works with this patch after fixing bugs in the repro itself: account for the ES6 default export, a typo where MDCCheckbox is used but the imported module is a text field and the usage of `defaultAdapter` as a function with it's a property.

Comment by Thomas Heller [ 30/Jul/17 3:51 AM ]

From a tool perspective (ie. shadow-cljs) I really did not like launching a node process for module_deps.js. Instead I rewrote all of it in Clojure plus a little Java Class that parses .js files and looks for require and import/export using the Closure Compiler. It is fairly simple code and completely eliminates the need for detective and others. Replicating everything module_deps.js does is a bit more work but the central piece is the extractor for the requires.

Not saying that you should do this as well but it is an option.

The .java code looks like this:
https://gist.github.com/thheller/d5761ee8adbe6f857bf6e05a71efda06

I have not merged the code into shadow-cljs yet but it does work quite well.

Comment by David Nolen [ 30/Jul/17 8:15 AM ]

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





[CLJS-2280] Provide process.env :preload and auto-configure Created: 28/Jul/17  Updated: 28/Jul/17  Resolved: 28/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

Approval: Vetted

 Description   

We should provide a process.env preload and automatically configure it. There should be a compiler option to disable the default - :shim-process.



 Comments   
Comment by David Nolen [ 28/Jul/17 3:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/537f60c975a29983c62647b4ea67b0bc08979366





[CLJS-2279] Infer `:module-type ` for provided `node_modules` Created: 27/Jul/17  Updated: 28/Jul/17  Resolved: 28/Jul/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

Attachments: Text File CLJS-2279.patch    
Patch: Code and Test
Approval: Vetted

 Description   

When crawling a `node_modules` installation, we currently hardcode every provided `:module-type` to `:commonjs`. However, certain packages are starting to provide ES6 modules and we could do a better job at figuring out which files provide CommonJS vs. ES6 modules.

This might be slightly hard to do and we should throw around some ideas on how to accomplish it.



 Comments   
Comment by Thomas Heller [ 28/Jul/17 3:01 AM ]

Many packages also ship CommonJS AND ES6 together and use the "package.json" "module" key for the ES6 code and "main" for normal CommonJS.

See:
https://github.com/rollup/rollup/wiki/pkg.module
https://github.com/nodejs/node-eps/pull/60

However many packages also do weird mixed stuff where the entry is ES6 but then uses require in the imported files (eg. material-ui/index.es.js). So it is not safe to assume that once you enter ES6 it will stay ES6 although the spec suggests otherwise.

FWIW in my own experiments I did not run into any issues ALWAYS setting :language-in to :ecmascript6, (.setProcessCommonJSModules true) and (.setTransformAMDToCJSModules true). Closure seems to be smart enough to figure things out on its own? Their JsFileParser is also able to detect ES6 files (by checking if a file contains import/export) without actually parsing.

Comment by David Nolen [ 28/Jul/17 1:13 PM ]

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





[CLJS-2278] JavaScript object literals are printed wth keys that cannot be read Created: 27/Jul/17  Updated: 28/Jul/17  Resolved: 28/Jul/17

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

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

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

 Description   

If you evaluate #js {"foo bar" 2}, it prints as #js {:foo bar 2}.

I'd suggest that, for JavaScript keys that cannot be represented as unqualified keywords, string notation would be used.

Example:

#js {"alpha" 1 "beta gamma" 2 "delta/epsilon" 3}

could print as

#js {:alpha 1 "beta gamma" 2 "delta/epsilon" 3}

Note that the ability to avoid namespace maps was added with CLJS-2190, which is in master.



 Comments   
Comment by Mike Fikes [ 27/Jul/17 9:13 PM ]

The attached patch conditionally converts the JavaScript object key from a string to a keyword iff it is a string that can be converted to a legal unqualified keyword.

By "legal": Abides with the first bullet under Symbols https://clojure.org/reference/reader#_symbols, doesn't allow / (because the resulting keyword would be qualified) and doesn't allow . (because the description of keywords below disallows .).

The only bit that is not supported is the last bullet under Symbols: "A symbol can contain one or more non-repeating :'s." (but only inside the string, not at the beginning nor end). I'm presuming that this code is not in an overly performance critical code path, and that a regex approach is sufficiently performant, but trying to match this last aspect might make for a slow regex, compared to the simple one that simply allows zero or more characters from a given character class.

Comment by David Nolen [ 28/Jul/17 6:48 AM ]

fixed https://github.com/clojure/clojurescript/commit/2046dc8dd7c4df9237a40cfbe48e8b97312d4d59





[CLJS-2277] Document return value of js-delete Created: 27/Jul/17  Updated: 27/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

Document that js-delete returns true upon success, false otherwise.






[CLJS-2276] Self-host: Need test.check dep for CLJS-2275 Created: 27/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

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

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

 Description   

The change in CLJS-2275 requires that we be able to load test.check in self host.



 Comments   
Comment by Mike Fikes [ 27/Jul/17 2:54 PM ]

The attached patch re-adds the support in script/test-self-parity.

Comment by Mike Fikes [ 27/Jul/17 3:11 PM ]

For historical context, this patch re-adds the stuff that was in CLJS-2082, which we had later reverted owing to the fact that it added a dep on org.clojure/test.check 0.10.0-alpha1, which introduced a bug TCHECK-131 which would end up in the shipping cljs.jar. That bug has now been fixed, with the fixe released in org.clojure/test.check 0.10.0-alpha2. We are actually now on that version of test.check owing to CLJS-2273, so all this patch really does is add the infrastructural stuff to script/test-self-parity so that it can load test.check, along with the sample generative tests. This is sufficient to allow test.check to be loaded for CLJS-2275.

Comment by David Nolen [ 27/Jul/17 8:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/190fa6489c9578ec7dba8235e5c905ae32ff7fda





[CLJS-2275] cljs.spec.alpha/fdef resolves eagerly Created: 27/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

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

Approval: Accepted

 Description   
(s/fdef foo.bar/kw->str
  :args (s/cat :kw keyword?)
  :ret  string?)

Triggers an error, in clojure.spec.alpha this works



 Comments   
Comment by David Nolen [ 27/Jul/17 1:58 PM ]

fixed https://github.com/clojure/clojurescript/commit/18761390b13d0dc4d80c0925c75d3eab49279ba3





[CLJS-2274] Update CI script to install deps Created: 26/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

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

Attachments: Text File CLJS-2274.patch    

 Description   

Missed this in CLJS-2272



 Comments   
Comment by David Nolen [ 27/Jul/17 6:36 AM ]

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





[CLJS-2273] Bump tools.reader to 1.0.3 and development dependencies Created: 25/Jul/17  Updated: 26/Jul/17  Resolved: 26/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 26/Jul/17 4:20 AM ]

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





[CLJS-2272] Tests that depended on default install deps behavior failing Created: 25/Jul/17  Updated: 25/Jul/17  Resolved: 25/Jul/17

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

Type: Defect Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, test

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

 Comments   
Comment by David Nolen [ 25/Jul/17 3:18 PM ]

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





[CLJS-2271] native-satisfies? using aget Created: 24/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

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


 Description   

native-satisfies? is using aget and should instead be using either gobj/get or unchecked-get.



 Comments   
Comment by Mike Fikes [ 24/Jul/17 9:33 AM ]

Already fixed with https://github.com/clojure/clojurescript/commit/84a2128dab9f52e67ee227a66be4f849d83de0a3





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

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




[CLJS-2269] Warn on top level code split loads Created: 24/Jul/17  Updated: 26/Jul/17  Resolved: 26/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: modules

Approval: Vetted

 Description   

See CLJS-2264 & and CLJS-2265. We should warn on top-level module loading and provide some basic guidance. Docs should make expectations clear.



 Comments   
Comment by David Nolen [ 26/Jul/17 4:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/364c2ff66bf7e7ff11a28d9366f3f98a82ffdbcb





[CLJS-2268] clojure.string/escape in ClojureScript (unlike in Clojure) assumes cmap is a map Created: 23/Jul/17  Updated: 23/Jul/17

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

Type: Defect Priority: Minor
Reporter: Max Kreminski Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs


 Description   

The ClojureScript implementation of the clojure.string/escape function assumes that the cmap parameter will always be a map. This makes it different from (and specifically less general than) the Clojure implementation of the same function, which permits cmap to be anything callable.

Here's the relevant lines of the clojure.string/escape implementations in Clojure and ClojureScript. The ClojureScript implementation calls get on cmap, while the Clojure implementation invokes cmap directly.

Here's an example that works on Clojure, but doesn't work on ClojureScript, because it passes a function to clojure.string/escape instead of a map:

(defn regex-escape
  "Escapes regex special chars in the string `s`."
  [s]
  (let [special? #{\- \[ \] \{ \} \( \) \* \+ \? \. \\ \^ \$ \|}]
    (clojure.string/escape s #(when (special? %) (str \\ %)))))

Ideally, this discrepancy would be fixed by changing the ClojureScript implementation of clojure.string/escape to follow the Clojure one. This would also match the behavior described in the function's docstring, which is the same on both platforms.






[CLJS-2267] Allow ^:const inlined vars to affect if emission Created: 21/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

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

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

 Description   

If an if test is a truthy or falsey constant, this fact is used to emit either the then or else branch, while also avoiding Closure warnings about unreachable code.

With the new ^:const Var inlining feature, we can allow a modest broadening of scope to also allow for truthy or falsey const expressions, perhaps slightly improving performance of affected code that is not run through Closure, and also avoiding new unreachable code warnings that can otherwise now crop up with the new JavaScript that results from ^:const Var inlining.



 Comments   
Comment by David Nolen [ 24/Jul/17 12:59 AM ]

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





[CLJS-2266] Self-host: Cannot require clojure.x where clojure.x has no macros namespace Created: 21/Jul/17  Updated: 07/Aug/17  Resolved: 07/Aug/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

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

 Description   

Repro:

(require 'cljs.js)

(cljs.js/eval-str (cljs.js/empty-state)
                  "(require 'clojure.x)"
                  nil
                  {:eval cljs.js/js-eval
                   :load (fn [{:keys [name macros]} cb]
                           (cb (when (and (= name 'cljs.x)
                                          (not macros))
                                 {:lang   :clj
                                  :source "(ns cljs.x)"})))}
                  println)

Expected result:

{:ns cljs.user, :value nil}

Actual result:

{:error #error {:message No such macros namespace: cljs.x, could not locate cljs/x.clj or cljs/x.cljc, :data {:tag :cljs/analysis-error}}}

Git bisect indicates CLJS-2069:

d4db18970c8eec587b2c9e022034983e29eb8e81 is the first bad commit
commit d4db18970c8eec587b2c9e022034983e29eb8e81
Author: António Nuno Monteiro <anmonteiro@gmail.com>
Date:   Sat Jun 3 15:10:40 2017 -0700

    CLJS-2069: Self-host: automatic `clojure` -> `cljs` aliasing doesn't work when loading macro namespaces

:040000 040000 31b0ec3b5346c8959e48487449576114b9d9a2ea 59d730263393923274257ea1eadaf4c2828c4199 M      src


 Comments   
Comment by Mike Fikes [ 21/Jul/17 1:20 PM ]

A concrete example of this is the inability to require cljs.tools.reader by indicating clojure.tools.reader.

Comment by António Nuno Monteiro [ 05/Aug/17 9:15 PM ]

Attached patch with fix.

Comment by David Nolen [ 07/Aug/17 7:14 AM ]

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





[CLJS-2265] Library namespaces loading before their dependencies with :none optiizations Created: 20/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

Type: Defect Priority: Minor
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: bug, repl


 Description   

Loading ClojureScript module that has library dependencies will fail due to the library namespaces loading before their dependencies.

For example say the have a project with a cljs-time ([com.andrewmcveigh/cljs-time "0.5.0"]) dependency, there modules

:modules {:core {:entries '#{org.modules.transitive}
                 :output-to "out/modules.js"}
          :time {:entries '#{org.modules.time}
                 :output-to "out/time.js"}}

and this code

(ns org.modules.time
  (:require [cljs-time.core :as ct]
            [cljs.loader :as loader]))

(defn display-time [time]
  time)
(ns org.modules.transitive
  (:require [cljs.loader :as loader]))

(loader/load
  :time
  (fn []
    (js/console.log ((resolve 'org.modules.time/display-time) 100))))

When the :time module is loaded the cljs-time.core is loaded, but any dependencies it may have (e.g. cljs-time.core.internal) are loaded afterwards causing the loading of the :time module to fail. After a backoff delay (around 5 seconds) the Google Closure Module Manager retries loading the :time module, this time successfully. Until the :time module is loaded successfully all module loading is subject to this backoff delay.

Demonstration Repo



 Comments   
Comment by David Nolen [ 24/Jul/17 12:33 AM ]

CLJS-2264





[CLJS-2264] Double loading of :cljs-base with :none optimizations Created: 20/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

Type: Defect Priority: Minor
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, repl


 Description   

If a project has these modules

:modules {:core {:entries '#{org.modules.core}
                 :output-to "out/modules.js"}
          :menu {:entries '#{org.modules.menu}
                 :output-to "out/menu.js"}}

and this root (org.modules.core) namespace

(ns org.modules.core
  (:require [cljs.loader :as loader]))

(loader/load
  :menu
  (fn [] (js/console.log "Module loaded)))

Then the :cljs-base module will be loaded twice.

The issue is that the loading of :menu causes `:cljs-base` to be loaded as well. Loading the root namespace marks :cljs-base as loaded but only at the end of the file. Under the hood ClojureScript adds

(cljs.loader/set-loaded! :core)

at the end of the :core module. By then it's too late, the :menu module has already started loading.

This is only a problem in the REPL due to the :cljs-base module being loaded through document.write.

Demonstration Repo



 Comments   
Comment by David Nolen [ 24/Jul/17 12:30 AM ]

This is not a bug. Loading modules like this at the top-level means you need to call set-loaded! yourself. In general what is being shown here is an antipattern. If you're just going to immediately load some other module why are you using code splitting?

One simple workaround is to simply put a setTimeout around this top level load.





[CLJS-2263] Docstring for neg-int? backwards Created: 19/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

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

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

 Description   

Typo introduced here for neg-int?'s docstring:

https://github.com/clojure/clojurescript/commit/224e140117d330933fb9b7e993ff26f997f36cbd



 Comments   
Comment by David Nolen [ 24/Jul/17 12:27 AM ]

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





[CLJS-2262] Correct comment that *warn-on-infer* is file-scope Created: 19/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

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

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

 Description   

I had accidentally written a comment in the code next to *warn-on-infer* indicating that it has global scope.



 Comments   
Comment by David Nolen [ 24/Jul/17 12:26 AM ]

fixed https://github.com/clojure/clojurescript/commit/3037f04cdc7d8cc977842e9f129ef9f3aee70796





[CLJS-2261] Issue using interop record constructors in macros namespaces Created: 18/Jul/17  Updated: 03/Aug/17  Resolved: 03/Aug/17

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

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

Attachments: Text File CLJS-2261-2.patch     Text File CLJS-2261.patch     Text File CLJS-2261-test.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Repro:

(require 'cljs.js)

(let [st (cljs.js/empty-state)]
  (cljs.js/eval-str st "(ns cljs.user (:require [foo.bar :refer-macros [cake]]))" nil
    {:eval cljs.js/js-eval
     :load (fn [{:keys [macros]} cb]
             (if macros
               (cb {:lang   :clj
                    :source "(ns foo.bar) (defmacro cake [] `(X.))"})
               (cb {:lang   :clj
                    :source "(ns foo.bar) (defrecord X [])"})))}
    (fn [_]
      (cljs.js/eval-str st "(cake)" nil
        {:eval cljs.js/js-eval
         :context :expr} prn))))

Expected:

{:ns cljs.user, :value #foo.bar.X{}}

Master produces:

WARNING: Use of undeclared Var cljs.user/X at line 1
WARNING: Use of undeclared Var cljs.user/X at line 1
{:error #error {:message "ERROR", :data {:tag :cljs/analysis-error}, :cause #object[TypeError TypeError: cljs.user.X is not a constructor]}}

Not a regression as far as I can tell, and also affects regular JVM ClojureScript.



 Comments   
Comment by David Nolen [ 24/Jul/17 12:24 AM ]

I believe I know the problem. `dotted-symbol?` needs to ignore `foo.` cases (we just do a naive goog.string/contains check). Can you provide a test patch and I can apply that and then try the fix? Thanks.

Comment by Mike Fikes [ 24/Jul/17 12:41 PM ]

Hey David, the test in this ticket is somewhat incorrect in that it has a macro expanding to a defrecord defined in the macro namespace instead of in the ClojureScript namespace. Fixing that aspect in the description.

Fixing this aspect of the ticket shows that this is not a regression, as far as I can tell.

It also affects JVM ClojureScript.

Things work if the macro uses (->X) instead of (X.), so the defect is constrained to using constructor interop.

Your hunch may still be correct because it still fails in the same way.

Attaching a patch that adds tests specifically for self-host (script/test-self-host})), as well as general tests that will run on JVM and self-host ({{script/teest and script/test-self-parity).

Comment by António Nuno Monteiro [ 01/Aug/17 5:43 PM ]

Attached patch with Mike's tests + tools.reader bump (which is where the fix is).

Comment by António Nuno Monteiro [ 01/Aug/17 6:04 PM ]

Blocked until some issues are sorted upstream in TRDR

Comment by Nicola Mometto [ 03/Aug/17 4:18 AM ]

António Nuno Monteiro I just release 1.0.5 which now requires cljs to resolve `Foo.` symbols to `their.ns/Foo.` in `resolve-symbol`. I realize this might be a bit annoying but it's the only way to support both clojure's behaviour of not including namespace segments and cljs' of requiring it

Comment by António Nuno Monteiro [ 03/Aug/17 11:06 AM ]

Attached CLJS-2261-2.patch which also bumps tools.reader to 1.0.5

Comment by David Nolen [ 03/Aug/17 4:22 PM ]

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





[CLJS-2260] Convert :constant AST node to tools.analyzer format Created: 18/Jul/17  Updated: 23/Sep/17

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

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

Attachments: Text File const-ast-2.patch     Text File const-ast.patch    
Patch: Code

 Description   

Part of https://dev.clojure.org/jira/browse/CLJS-1461

Work on :const node:

  • rename :constant op to :const
  • add :val entry


 Comments   
Comment by David Nolen [ 15/Sep/17 3:24 PM ]

This patch needs a rebase.

Comment by Amanda Walker [ 23/Sep/17 8:13 PM ]

Here is the rebased patch.





[CLJS-2259] Extra .cljs_node_repl directory containing cljs.core output Created: 18/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: regression

Approval: Vetted

 Description   

Presuming this is a regression: Starting with 1.9.456, with the shipping cljs.jar, you end up with an extra .cljs_node_repl subdirectory inside the top-level .cljs_node_repl directory (and this appears to be where the analyzer things core symbols reside).

Minimal repro (note the paths in the stacktrace):

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 56211
To quit, type: :cljs/quit
cljs.user=> (ffirst 1)
repl:13
throw e__5614__auto__;
^

Error: 1 is not ISeqable
    at Object.cljs$core$seq [as seq] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:4097:8)
    at Object.cljs$core$first [as first] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:4116:19)
    at cljs$core$ffirst (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:5512:34)
    at repl:1:96
    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)
cljs.user=>

git bisect shows it is caused by CLJS-1893:

462e2295d6e6c1a01b279c8aa091f832c9d09824 is the first bad commit
commit 462e2295d6e6c1a01b279c8aa091f832c9d09824
Author: dnolen <dnolen@cognitect.com>
Date:   Fri Jan 20 15:21:12 2017 -0500

    CLJS-1893: Unnecessary analysis of core.cljs

    In all places where opts is optionally provided check to see if cljs.env/*compiler* is bound, if it is deref and read :options

:040000 040000 eb9ed33f31e1c8b322c18212752573ce449c17aa 955f16b82539e257d22ab135b4ae048cceb47b73 M	src


 Comments   
Comment by David Nolen [ 27/Jul/17 7:03 AM ]

fixed https://github.com/clojure/clojurescript/commit/45f63555aaad62384573001afb667e6bff58097e





[CLJS-2258] Stack overflow regression for sequence xform applied to eduction Created: 18/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: regression, transducers

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

 Description   

There is a regression in the ability to use sequence to apply transducers to eductions that was introduced with 1.9.562.

In 1.9.542 and earlier,

(sequence (map str) (eduction [1]))
produced ("1").

Minimal repro:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 50609
To quit, type: :cljs/quit
cljs.user=> (sequence (map str) (eduction [1]))
repl:13
throw e__5525__auto__;
^

RangeError: Maximum call stack size exceeded
    at Function.cljs.core.TransformerIterator.create (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:13959:50)
    at cljs.core.Eduction.cljs$core$IIterable$_iterator$arity$1 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:33244:38)
    at Object.cljs$core$_iterator [as _iterator] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3158:13)
    at Object.cljs$core$iter [as iter] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:13554:18)
    at Function.cljs.core.TransformerIterator.create (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:13960:55)
    at cljs.core.Eduction.cljs$core$IIterable$_iterator$arity$1 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:33244:38)
    at Object.cljs$core$_iterator [as _iterator] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3158:13)
    at Object.cljs$core$iter [as iter] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:13554:18)
    at Function.cljs.core.TransformerIterator.create (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:13960:55)
    at cljs.core.Eduction.cljs$core$IIterable$_iterator$arity$1 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:33244:38)


 Comments   
Comment by Mike Fikes [ 18/Jul/17 7:36 AM ]

CLJS-2034:

d93c4356e7ab78743ae66d8cffe8df54869f0be3 is the first bad commit
commit d93c4356e7ab78743ae66d8cffe8df54869f0be3
Author: António Nuno Monteiro <anmonteiro@gmail.com>
Date:   Fri May 12 13:39:37 2017 -0700

    CLJS-2034: Sequence and Eduction produce infinite loop in transducer that appends to the reduction

    The implementation of transducers in ClojureScript tracked an old counterpart in
    the Clojure codebase. This patch addresses the change introduced in the
    following commit to Clojure, which replaced `LazyTransformer` with
    `TransformerIterator`, effectively making the transducer behavior akin to the
    one in Clojure.

    https://github.com/clojure/clojure/commit/c47e1bbcfa227723df28d1c9e0a6df2bcb0fecc1

:040000 040000 001a34e8f941a2e1697299be2632143e3e191587 5a508a3162eb9eb1023c0c0cd9ff7ee9cf2ad733 M	src
Comment by António Nuno Monteiro [ 18/Jul/17 12:08 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 18/Jul/17 12:37 PM ]

I don't know if this is a bug or not, but with 1.9.542

cljs.user=> (iter (eduction [1 2 3]))
#object[cljs.core.SeqIter]

and after the patch:

cljs.user=> (iter (eduction [1 2 3]))
repl:13
throw e__8096__auto__;
^

Error: [object Object] is not ISeqable
    at Object.cljs$core$seq [as seq] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:4413:8)
    at Object.cljs$core$pr_sequential_writer [as pr_sequential_writer] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31431:14)
    at cljs.core.TransformerIterator.cljs$core$IPrintWithWriter$_pr_writer$arity$3 (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:32456:18)
    at Object.cljs$core$pr_writer_impl [as pr_writer_impl] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31612:12)
    at Object.cljs$core$pr_writer [as pr_writer] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31722:18)
    at Object.cljs$core$pr_seq_writer [as pr_seq_writer] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31726:11)
    at Object.cljs$core$pr_sb_with_opts [as pr_sb_with_opts] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31789:11)
    at Object.cljs$core$pr_str_with_opts [as pr_str_with_opts] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31803:63)
    at Function.cljs.core.pr_str.cljs$core$IFn$_invoke$arity$variadic (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31886:18)
    at Object.cljs$core$pr_str [as pr_str] (/Users/mfikes/Projects/clojurescript/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31882:25){

I wonder if there is some public code path where this would show up (as opposed to directly calling iter).

Comment by António Nuno Monteiro [ 18/Jul/17 12:57 PM ]

`(iter (eduction [1 2 3]))` is now a TransformerIterator, which doesn't have a print method.

For comparison with Clojure:

user=> (clojure.lang.RT/iter (eduction [1 2 3]))
#object[clojure.lang.TransformerIterator 0x28adf451 "clojure.lang.TransformerIterator@28adf451"]
Comment by David Nolen [ 24/Jul/17 12:12 AM ]

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





[CLJS-2257] Expand dotted symbols into field accesses in the analyzer Created: 17/Jul/17  Updated: 17/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Ambrose Bonnaire-Sergeant Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently, there is a lot of implicit information in a :var node in the analyzer around dotted symbols.

The :name of a :var node can contain implicit field accesses, and this information must be manually disambiguated with every new tool that consumes the CLJS AST (like core.typed).

A solution might be to expand specific dotted :var nodes into :dot nodes containing a :var node. Locals in particular might benefit from this transformation, would others? (Global variables, js/* variables)






[CLJS-2256] Generated code doesn't add newline after sourceMappingURL comment Created: 17/Jul/17  Updated: 18/Jul/17  Resolved: 18/Jul/17

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

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

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

 Description   

Currently generated code doesn't output a newline after the `sourceMappingURL` comment, which makes code transforms (e.g. Webpack bundling) output code on that line that also ends up being commented out



 Comments   
Comment by David Nolen [ 18/Jul/17 12:39 AM ]

fixed https://github.com/clojure/clojurescript/commit/62926332a78e89935f5aa263195fc29cc297c3b7





[CLJS-2255] Clean up :npm-deps Created: 17/Jul/17  Updated: 25/Jul/17  Resolved: 25/Jul/17

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

Type: Task Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: npm-deps

Attachments: Text File CLJS-2255.patch    

 Description   

We should separate module installation from building and REPLs. I propose the following new public build fns:

cljs.build.api/install-deps!
cljs.build.api/get-deps
cljs.build.api/print-deps


 Comments   
Comment by David Nolen [ 17/Jul/17 2:00 AM ]

`get-deps` should return a vector of {:name ... :version ... :type ...} entries

Comment by António Nuno Monteiro [ 17/Jul/17 8:37 PM ]

Attached a tentative patch adding `cljs.build.api/install-node-deps!` and `cljs.build.api/get-node-deps` functions.

Looking for feedback if this is the satisfies the requirements for what you had in mind. IMO printing dependencies is just a call away from `get-node-deps`.

Comment by David Nolen [ 18/Jul/17 12:36 AM ]

I agree about printing deps. These functions should probably take compiler options since they are user facing and that's the easiest interface from that standpoint.

We should also remove automatic calls to dep installation in build as well as REPLs. Installing dependencies on each build, REPL start just doesn't seem like a good idea.

Comment by David Nolen [ 25/Jul/17 1:19 AM ]

After thinking about it some more I think the supplied patch is OK.

Comment by David Nolen [ 25/Jul/17 1:26 AM ]

fixed https://github.com/clojure/clojurescript/commit/8972224b4b4617a98a9fdd497af1aeb91a29ed2a





[CLJS-2254] Module Indexing: Provide relative paths for a package's main module Created: 16/Jul/17  Updated: 17/Jul/17  Resolved: 17/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

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

 Comments   
Comment by António Nuno Monteiro [ 16/Jul/17 8:14 PM ]

The attached patch also provides tests for previously untested `module_deps.js` indexing to make sure it's at parity with the indexing on the Clojure side.

Comment by David Nolen [ 17/Jul/17 12:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/7139c4c17932b1e11e7a2b665914b32936f02644





[CLJS-2253] Add a `:target` `:react-native` Created: 16/Jul/17  Updated: 16/Jul/17  Resolved: 16/Jul/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: Text File CLJS-2253.patch    

 Comments   
Comment by António Nuno Monteiro [ 16/Jul/17 4:08 PM ]

We can achieve the same thing this is doing with `:target :nodejs`





[CLJS-2252] Self-host: :redef-in-file doesn't trigger for bootstrapped Created: 16/Jul/17  Updated: 16/Jul/17

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

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


 Description   

If you redefine a symbol in a file and require that file using self-hosted ClojureScript, the :redef-in-file diagnostic doesn't trigger.

It is difficult to create a minimal repro for this, as it requires a setup that loads files. (Perhaps one can be made with the script/test-self-parity infrastructure).

It appears that this can be resolved by setting ana/file-defs at the right places and unsetting it at the right async completion points.






[CLJS-2251] Follow-up fix to CLJS-2249 and related commit Created: 16/Jul/17  Updated: 16/Jul/17  Resolved: 16/Jul/17

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

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

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

 Description   

the fix in [1] is not enough since any other arity that calls `cljs.env/default-compiler-env` without computing the implicit options will suffer from the same problem. Instead we should update the js-dependency-index in the compiler env once we have computed all the implicit options, and every caller of `build` will get consistency for free without needing to call `add-implicit-options` themselves.

https://github.com/clojure/clojurescript/commit/d4b871cce73e43e489496b6c2bf460492bb7742a



 Comments   
Comment by David Nolen [ 16/Jul/17 1:38 AM ]

fixed https://github.com/clojure/clojurescript/commit/309e209f4739f411e7fad959095ecbdc17def948





[CLJS-2250] Support :foreign-libs overrides via :provides Created: 15/Jul/17  Updated: 28/Jul/17  Resolved: 28/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: foreign-libs

Approval: Screened

 Description   

Currently you must supply matching `:provides` and `:file` to override an existing entry. It would be nicer to be able to simply specify an incomplete `:provides`. This would support the transition to `:global-exports`, users can fix up deps locally.



 Comments   
Comment by David Nolen [ 28/Jul/17 8:39 AM ]

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





[CLJS-2249] Regression with foreign-libs under Node.js Created: 15/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

Type: Defect Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: foreign-libs

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

 Comments   
Comment by António Nuno Monteiro [ 15/Jul/17 7:39 PM ]

closing as this is the wrong fix for the issue in question.

Comment by David Nolen [ 15/Jul/17 7:41 PM ]

This is actual a bug, but the patch is just fixing the wrong place. I believe I fixed the issue in master. Would take a patch which is just the test.

Comment by António Nuno Monteiro [ 15/Jul/17 7:50 PM ]

The revised patch now contains just the test which passes after the fix in master.

Comment by David Nolen [ 15/Jul/17 7:52 PM ]

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





[CLJS-2248] Build API tests rely on Yarn Created: 15/Jul/17  Updated: 16/Jul/17  Resolved: 16/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 16/Jul/17 1:40 AM ]

The patch needs to be rebased to master.

Comment by António Nuno Monteiro [ 16/Jul/17 11:47 AM ]

Replaced the patch with a new one that applies on current master.

Comment by David Nolen [ 16/Jul/17 5:53 PM ]

fixed https://github.com/clojure/clojurescript/commit/576f2865289d68743597022033c6752c8d0ebc18





[CLJS-2247] Warn when overwriting protocol method Created: 15/Jul/17  Updated: 16/Jul/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.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.




[CLJS-2246] Revert CLJS-2245 and CLJS-2240 and fix `lein test` Created: 15/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

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

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

 Description   

Rationale: the behavior introduced in CLJS-2245 was already supported without any additional compiler flags before CLJS-2240.



 Comments   
Comment by David Nolen [ 15/Jul/17 3:57 PM ]

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





[CLJS-2245] Add support for using a local `node_modules` installation through a new `:node-modules` compiler flag Created: 15/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

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

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

 Description   

Assume the user installed all the necessary packages and do not shell out to `npm` to install anything.

Makes the node_module support build tool agnostic, users can now use Yarn if they please.



 Comments   
Comment by David Nolen [ 15/Jul/17 2:21 PM ]

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

Comment by Mike Fikes [ 15/Jul/17 2:24 PM ]

I gave this patch a try and it appeared to be a big improvement over the situation without it. My yarn.lock file was honored, and things didn't "fall apart" with undesired deps being pulled in.

I was able to issue (require ...) statements against Node modules in the REPL, with them ultimately failing for expected reasons. (One module being pulled in is actually using ES6 code constructs, so failed, and trying to (require 'react-native) starts to work until it appears to need to require code marked with @providesModule, not being able to find it.)

TL;DR, AFAICT, the patch is good, considering that I can't fully test it with my setup.





[CLJS-2244] Orphaned processed JS modules breaks :modules Created: 15/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

Type: Defect Priority: Blocker
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: js-modules, modules

Approval: Vetted

 Description   

An orphaned processed JS module when using :modules code splitting will cause a no `:out-file` exception to be thrown.



 Comments   
Comment by David Nolen [ 15/Jul/17 9:15 AM ]

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





[CLJS-2243] Self-host: Add support for :global-exports Created: 15/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

Type: Enhancement Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap, foreign-libs

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

 Comments   
Comment by David Nolen [ 15/Jul/17 8:14 AM ]

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





[CLJS-2242] Lots of undeclared Var warns in cljs.spec.gen.alpha Created: 14/Jul/17  Updated: 15/Jul/17  Resolved: 15/Jul/17

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

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


 Description   

I saw this with the browser REPL in Quick Start, but it is also easy to repro with script/noderepljs:

orion:(master)clojurescript$ script/noderepljs
ClojureScript Node.js REPL server listening on 49628
WARNING: Use of undeclared Var cljs.spec.alpha/get-spec at line 52 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/repl.cljs
WARNING: Use of undeclared Var cljs.spec.alpha/describe at line 56 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/repl.cljs
WARNING: Use of undeclared Var cljs.spec.alpha/describe at line 56 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/repl.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/such-that at line 277 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 442 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/bind at line 448 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/choose at line 448 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/hash-map at line 453 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/gen-for-pred at line 491 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 532 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 533 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/one-of at line 541 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/tuple at line 603 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 672 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/one-of at line 676 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 788 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/tuple at line 790 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/bind at line 894 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 896 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 897 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 899 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 901 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector-distinct at line 906 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector-distinct at line 907 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector at line 912 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector at line 915 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector at line 918 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 1176 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 1181 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 1181 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 1188 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 1189 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 1191 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/cat at line 1195 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/one-of at line 1198 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 1200 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/fmap at line 1202 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/vector at line 1203 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/for-all* at line 1286 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/quick-check at line 1287 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 1329 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/generate at line 1332 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/frequency at line 1380 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 1381 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/return at line 1381 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/delay-impl at line 1382 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
WARNING: Use of undeclared Var cljs.spec.gen.alpha/sample at line 1393 /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs
To quit, type: :cljs/quit


 Comments   
Comment by Mike Fikes [ 14/Jul/17 8:02 PM ]
3cf960bb161d8c5fd75b046a4a119d9d0f645409 is the first bad commit
commit 3cf960bb161d8c5fd75b046a4a119d9d0f645409
Author: dnolen <dnolen@cognitect.com>
Date:   Fri Jul 14 18:31:45 2017 -0400

    CLJS-2241: Multiple requires of Node.js modules in non :nodejs target are not idempotent at the REPL

    leave a comment about module subtlely, drop unneeded analyze

:040000 040000 c3fa69586da025b5ee7599d9a8ad3d50361b8699 a569843f2f91d8d7da5d180c15126b3c2ed4f5eb M	src
Comment by David Nolen [ 15/Jul/17 8:12 AM ]

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





[CLJS-2241] Multiple requires of Node.js modules in non :nodejs target are not idempotent at the REPL Created: 14/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Defect Priority: Blocker
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: js-modules, repl

Approval: Vetted

 Description   

Given the following setup:

(require '[cljs.repl :as r])
(require '[cljs.repl.browser :as rb])

(def opts
  {:browser-repl true
   :verbose true
   :npm-deps {"lodash" "4.17.4"}})

(r/repl* (rb/repl-env) opts)

Under a REPL when the target is not the Node.js the following behavior is observed:

(require '["lodash/array" :as lodash-array])

Will succeed the first time. The same require a second time produce the following stack trace:

clojure.lang.ExceptionInfo: No such namespace: module$Users$davidnolen$development$clojure$hello-world$node-modules$lodash$array, could not locate module$Users$davidnolen$development$clojure$hello_world$node_modules$lodash$array.cljs, module$Users$davidnolen$development$clojure$hello_world$node_modules$lodash$array.cljc, or JavaScript providing "module$Users$davidnolen$development$clojure$hello-world$node-modules$lodash$array" at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (require (quote ["lodash/array" :as lodash-array]))}, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4617)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:690)
	at cljs.analyzer$error.invoke(analyzer.cljc:690)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:692)
	at cljs.analyzer$analyze_deps.invokeStatic(analyzer.cljc:2111)
	at cljs.analyzer$ns_side_effects.invokeStatic(analyzer.cljc:3430)
	at cljs.analyzer$ns_side_effects.invoke(analyzer.cljc:3424)
	at cljs.analyzer$analyze_STAR_$fn__2393.invoke(analyzer.cljc:3546)
	at clojure.lang.PersistentVector.reduce(PersistentVector.java:341)
	at clojure.core$reduce.invokeStatic(core.clj:6544)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3543)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3569)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3553)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3350)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3498)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3543)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3569)
	at cljs.repl$evaluate_form$fn__6113.invoke(repl.cljc:496)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:496)
	at cljs.repl$eval_cljs.invokeStatic(repl.cljc:616)
	at cljs.repl$eval_cljs.invoke(repl.cljc:603)
	at cljs.repl$repl_STAR_$read_eval_print__6234.invoke(repl.cljc:865)
	at cljs.repl$repl_STAR_$fn__6240$fn__6249.invoke(repl.cljc:905)
	at cljs.repl$repl_STAR_$fn__6240.invoke(repl.cljc:904)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1224)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:841)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:745)
	at user$eval1109.invokeStatic(repl.clj:19)
	at user$eval1109.invoke(repl.clj:19)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.load(Compiler.java:7379)
	at clojure.lang.Compiler.loadFile(Compiler.java:7317)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$script_opt.invokeStatic(main.clj:335)
	at clojure.main$script_opt.invoke(main.clj:330)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)


 Comments   
Comment by David Nolen [ 14/Jul/17 5:32 PM ]

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

Comment by Mike Fikes [ 14/Jul/17 8:04 PM ]

Regression for this commit tracked in CLJS-2242





[CLJS-2240] don't shell out to module_deps.js if `:npm-deps` not specified Created: 14/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Defect Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules

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

 Comments   
Comment by David Nolen [ 14/Jul/17 1:56 PM ]

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





[CLJS-2239] Self-host: Add `:target :nodejs` to the docstrings in cljs.js Created: 14/Jul/17  Updated: 16/Jul/17  Resolved: 16/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 16/Jul/17 1:39 AM ]

fixed https://github.com/clojure/clojurescript/commit/8429372b13a06fd90c69eb296c6fd63980557326





[CLJS-2238] Perf regression with node module indexing Created: 14/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps, performance

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

 Description   

On master, I'm observing that, using Figwheel, changes to source take about 12 seconds to appear in a React Native app running in an iOS simulator, whereas with the 1.9.671 such changes take about 2 seconds.

I apologize for not having a minimal repro available. I suspect it is easily reproducible locally using a setup generated using re-natal init FutureApp (see https://github.com/drapanjanas/re-natal)

Of note: I don't have :npm-deps set. My compiler options are:

{:output-to     "target/ios/not-used.js"
 :main          "env.ios.main"
 :output-dir    "target/ios"
 :optimizations :none
 :checked-arrays false 
 :warnings      true}

A git bisect shows that this was introduced with the change in CLJS-2212.



 Comments   
Comment by Mike Fikes [ 14/Jul/17 1:26 PM ]

For reference, in handle-js-modules the top-level set ends up having 14748 elements, and it does appear to take about 12 seconds to produce that number.

My requires set has 77 elements and the node-required (the intersection) has 0 elements.

Comment by David Nolen [ 14/Jul/17 2:03 PM ]

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

Comment by Mike Fikes [ 14/Jul/17 2:04 PM ]

With the patch, changes appear in about 2 seconds.





[CLJS-2237] Defrecord does not escape field names matching JS keywords (with/in/...) Created: 14/Jul/17  Updated: 14/Jul/17

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

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


 Description   

Defrecord produces code that's incompatible with ECMASCRIPT3 language:

(defrecord Query [with in])
Compiling "target/main.js" from ["src"]...
Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                  ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                               ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 4 warning(s)
Successfully compiled "target/main.js" in 6.224 seconds.

Probably there should be some name escaping happening? I see that (defn with []) is compiled as cljs-test.core.with$. Should we do the same for records?



 Comments   
Comment by David Nolen [ 14/Jul/17 12:19 PM ]

The problem has less to do with records than how to handle reserved