<< Back to previous view

[CLJS-2265] Library namespaces loading before their dependencies with :none optiizations Created: 20/Jul/17  Updated: 20/Jul/17

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

Type: Defect Priority: Minor
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Unresolved 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






[CLJS-2264] Double loading of :cljs-base with :none optimizations Created: 20/Jul/17  Updated: 20/Jul/17

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

Type: Defect Priority: Minor
Reporter: Peter Schuck Assignee: Unassigned
Resolution: Unresolved 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






[CLJS-2263] Docstring for neg-int? backwards Created: 19/Jul/17  Updated: 19/Jul/17

Status: Open
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: Unresolved 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






[CLJS-1461] Convert analyzer to conform to tools.analyzer's spec Created: 28/Sep/15  Updated: 19/Jul/17

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

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


 Description   

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

Work in progress: https://github.com/frenchy64/clojurescript/pull/7

Order of work:

  1. Patch 1 ready for review
    • :const
      • rename :constant op to :const
      • add :val entry
  2. :def
    • rename :var-ast entry to :var I misunderstood :var-ast, :var entry is already there and perfectly fine.
    • add :ns entry
  3. :the-var
    • rename :var-special op to :the-var
  4. :throw
    • rename :throw entry to :exception
  5. :try
    • rename :try entry to :body
  6. :letfn
    • rename :expr entry to :body
  7. :let/:loop
    • rename :expr entry to :body
  8. :quote
    • add :quote op
  9. :deftype
    • rename :deftype* op to :deftype
  10. :defrecord
    • rename :defrecord* op to :defrecord
  11. :host-field/:host-method
    • split :dot op into :host-field/:host-method
  12. :invoke
    • rename :f to :fn
  13. :js-object/:js-array
    • split :js-value op into :js-object/:js-array
  14. :with-meta
    • rename :meta op to :with-meta
  15. :var/:binding/:local/:js-var
    • split :var op into :var/:binding/:local/:js-var
    • emit :local op if :js-shadowed-by-local
    • change :local to be #{:fn :letfn :let :arg ...}
  16. :fn-method
    • add :fn-method op to :methods of a :fn
    • :variadic -> :variadic?
    • :expr -> :body
    • add :fixed-arity
  17. :children
    • move to tools.analyzer :children format + replace :children calls with a compatible function from AST -> children
  18. Unit tests
    • add them all at the end
  19. AST format documentation
    • modify from tools.analyzer's

Extra stuff:

  • argument validation in 'var parsing
  • :max-fixed-arity -> :fixed-arity
  • :case node overhaul
    • which format do we follow?
    • TAJ vs TAJS





[CLJS-2262] Correct comment that *warn-on-infer* is file-scope Created: 19/Jul/17  Updated: 19/Jul/17

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved 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.






[CLJS-2109] incorrect syntax-quote symbol resolution (resolve-symbol 'clojure.core) -> 'clojure/core Created: 20/Jun/17  Updated: 18/Jul/17  Resolved: 23/Jun/17

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

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

Approval: Accepted

 Comments   
Comment by David Nolen [ 23/Jun/17 11:09 AM ]

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

Comment by Mike Fikes [ 18/Jul/17 3:30 PM ]

Self-host regression: CLJS-2261





[CLJS-2261] Self-host: Regression using interop record constructors in macros namespaces Created: 18/Jul/17  Updated: 18/Jul/17

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

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


 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 [_ cb]
             (cb {:lang :clj
                  :source "(ns foo.bar) (defrecord X []) (defmacro cake [] `(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]}}

Git bisect indicates CLJS-2109

f3886c9f38248d9525ce6cf85d6aadeaf4a09bbf is the first bad commit
commit f3886c9f38248d9525ce6cf85d6aadeaf4a09bbf
Author: David Nolen <david.nolen@gmail.com>
Date:   Fri Jun 23 12:09:30 2017 -0400

    CLJS-2109: incorrect syntax-quote symbol resolution (resolve-symbol 'clojure.core) -> 'clojure/core

:040000 040000 9c464c6db9b4d4f9ca76ebffe3097cf6ba3b166b 7913b16c5bba884e0c602cbdbadfb1673ab56980 M	src





[CLJS-2260] Convert :constant AST node to tools.analyzer format Created: 18/Jul/17  Updated: 18/Jul/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.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





[CLJS-2258] Stack overflow regression for sequence xform applied to eduction Created: 18/Jul/17  Updated: 18/Jul/17

Status: Open
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: Unresolved Votes: 0
Labels: regression, transducers

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

 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"]




[CLJS-2259] Extra .cljs_node_repl directory containing cljs.core output Created: 18/Jul/17  Updated: 18/Jul/17

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

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


 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





[CLJS-1893] Unnecessary analysis of core.cljs Created: 20/Jan/17  Updated: 18/Jul/17  Resolved: 20/Jan/17

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

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


 Comments   
Comment by David Nolen [ 20/Jan/17 2:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/462e2295d6e6c1a01b279c8aa091f832c9d09824

Comment by Mike Fikes [ 18/Jul/17 8:07 AM ]

See regression CLJS-2259





[CLJS-2034] Sequence and Eduction produce infinite loop in transducer that appends to the reduction Created: 12/May/17  Updated: 18/Jul/17  Resolved: 12/May/17

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

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

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

 Description   

Example: a transducer that appends :foo to the result of the reduction:

(defn xf []
  (fn [rf]
    (fn
      ([] (rf))
      ([result] (rf result :foo))
      ([result input]
       (rf result input)))))

(sequence (xf) [1 2 3])

In Clojure, the above code yields

'(1 2 3 :foo)
. In ClojureScript, it results in an infinite loop. The same happens with `eduction`.
However,
(into [] (xf) [1 2 3])
doesn't produce an error.



 Comments   
Comment by António Nuno Monteiro [ 12/May/17 2:13 PM ]

I'm looking into fixing this one.

Comment by António Nuno Monteiro [ 12/May/17 3:46 PM ]

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

Attached patch with fix and test.

Comment by David Nolen [ 12/May/17 3:53 PM ]

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

Comment by Mike Fikes [ 18/Jul/17 7:37 AM ]

See regression in CLJS-2258.





[CLJS-2255] Clean up :npm-deps Created: 17/Jul/17  Updated: 18/Jul/17

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved 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.





[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: Next

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-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-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: Next
Fix Version/s: Next

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-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: Next
Fix Version/s: Next

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-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-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-1904] Reload support for JS Modules Created: 25/Jan/17  Updated: 16/Jul/17  Resolved: 16/Jul/17

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

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


 Description   

Process modules currently works by changing transforming compiler opts. This means the resulting compiler opts cannot be used to run JS module processing again. This issue is now clear in cljs/repl.cljc. The other issue is that JS module processing is the only time that ClojureScript is involved in the compilation of JS sources - prior to this we never to need to trigger JS compilation ourselves. Thus require + :reload doesn't work on JS namespaces.



 Comments   
Comment by David Nolen [ 16/Jul/17 1:44 AM ]

As far as I can tell this issue is resolved now in master in the general case. There maybe edge cases but we should open issues as we encounter them.





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

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Not Reproducible Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 13/Jul/17 7:14 PM ]

This may no longer effect master? Previously I believe there was case encountered where a module export was treated as ClojureScript fn and there was a direct invoke?

Comment by David Nolen [ 16/Jul/17 1:43 AM ]

Closing this one since there's just not enough information to act on.





[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: Next

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-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: Next
Fix Version/s: Next

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: 15/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved 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.






[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: Next
Fix Version/s: Next

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

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

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: Next
Fix Version/s: Next

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

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: Next
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: Next
Fix Version/s: Next

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-2229] Ensure that new modules work works correctly with REPLs Created: 13/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Task Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, repl

Approval: Accepted

 Description   

We need to audit the new features and ensure they work out of the box for various REPLs. Even a cursory glance shows that we don't index node modules.



 Comments   
Comment by David Nolen [ 14/Jul/17 5:04 PM ]

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





[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: Next
Fix Version/s: Next

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-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: Next
Fix Version/s: Next

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-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 names. As far as I'm concerned the Closure warnings are sufficient, but if somebody wants to devise a warning patch that warns on using reserved fields names on deftpye/record when the output language is ES3, then be my guest.

Comment by David Nolen [ 14/Jul/17 12:20 PM ]

Related CLJS-871

Comment by Nikita Prokopov [ 14/Jul/17 12:37 PM ]

Is there any reason why CLJS defaults to language_in=ES3? Shouldn’t CLJS lock in the version of JS it outputs? As I understand, programmers have no control over how JS looks, but CLJS compiler has knowledge and control over what version of JS it outputs (and feeds into Closure)? In other words, why not set language_in to ES5 by default?





[CLJS-2236] Defrecord does not escape field names matching JS keywords (with/in/...) Created: 14/Jul/17  Updated: 14/Jul/17

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

Type: Enhancement Priority: Major
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?






[CLJS-2235] Allow passing extra maven opts to build scripts Created: 14/Jul/17  Updated: 14/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: Antonin Hildebrand Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: test-matrix

Attachments: Text File CLJS-2235.patch    
Patch: Code
Approval: Vetted

 Description   

I canary project[1] I need to run all maven commands in --batch-mode to prevent download progress messages output and also optionally I want to run it with --quiet flag to optionally suppress its verbose output.

This patch introduces CLJS_SCRIPT_MVN_OPTS env var which can be optionally specified to provide extra command line opts to all mvn commands executed within build scripts.

[1] https://github.com/cljs-oss/canary
[2] https://github.com/cljs-oss/canary/blob/9772649eae7097156251f4abb7ee70ea349ea99b/runner/scripts/build_compiler.sh#L74






[CLJS-2234] Make build scripts optionally less verbose Created: 14/Jul/17  Updated: 14/Jul/17

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

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

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

 Description   

Canary project[1] needs to build ClojureScript compiler via script/build[2]. Canary's runner runs in Travis CI and this was causing too verbose output.

Let's allow altering build scripts verbosity via CLJS_SCRIPT_QUIET env var.

[1] https://github.com/cljs-oss/canary
[2] https://github.com/cljs-oss/canary/blob/9772649eae7097156251f4abb7ee70ea349ea99b/runner/scripts/build_compiler.sh#L84






[CLJS-2232] Self-host: Add support for string-based requires Created: 14/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

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

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

 Comments   
Comment by António Nuno Monteiro [ 14/Jul/17 12:43 AM ]

Patch attached.

Comment by David Nolen [ 14/Jul/17 5:57 AM ]

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





[CLJS-2233] reconsider using string requires for anything non-cljs/closure Created: 14/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Declined Votes: 1
Labels: None


 Description   

I said it before in CLJS-2061 and I still think a "string" in :require should always be used for anything that is not CLJS/closure.

The string variant already works for :nodejs now, so there is no longer a difference in code.

(:require [react :as r])
(:require ["react" :as r)

The only difference is in the backend in how symbols are resolved. That now has a ton of special cases just because a symbol may be an alias for a npm-package. If an npm-package doesn't exist the magic symbol won't exist either. With the "react" require it would be clear that the user wanted to import a NPM package and could be warned accordingly if it wasn't installed.

This is not a great error message (manually added the extra lookup possibility)

No such namespace: react, could not locate react.cljs, react.cljc, Closure namespace "react" or npm package "react" ...

I think this feature would be much easier for the user if we were always using a string for JS requires. It would also make the parts of the config for :foreign-libs obsolete as the :nodejs support has shown (which doesn't need it at all).

IMHO a string is a perfectly fine indicator for the analyzer to treat it differently to a symbol and makes the code a whole lot easier to reason about.

I consider string requires to be the equivalent of :import for Java classes in CLJ and we should have a definite way to tell what something refers to. In case of react we can't really tell without a whole lot of lookup logic, with "react" that is easy.

Just my 2 cents, feel free to close if you disagree.



 Comments   
Comment by David Nolen [ 14/Jul/17 5:22 AM ]

String based require has nothing to do with Node.js. If Clojure had #|| literal to support arbitrary symbols we would use that instead.

Comment by Thomas Heller [ 14/Jul/17 5:56 AM ]

I don't get what #|| is or what it would achieve over strings. This is strictly about separating JS "packages" from everything else, just like Clojure uses :import to separate Java Code from Clojure Code. I also never said that it has anything to do with node, but node can and will now use it. [1]

Well, I get the message. I'll stop talking about it.

[1] https://twitter.com/anmonteiro90/status/885741059065528321





[CLJS-2230] Double checked arrays Created: 13/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: warnings

Approval: Vetted

 Description   

If you have an aget passed as a form to a macro like or it will get analyzed twice and trigger the diagnostic.

Example:

cljs.user=> (and true (or (aget #js {:foo 1} "foo") 2))
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1 <cljs repl>
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1 <cljs repl>
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1 <cljs repl>
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1 <cljs repl>
Error: Assert failed: (or (array? array) (js/goog.isArrayLike array))
...


 Comments   
Comment by David Nolen [ 14/Jul/17 5:54 AM ]

fixed https://github.com/clojure/clojurescript/commit/424e26415476ed302098d37fd2d3cc0e6d5a3051





[CLJS-2231] :checked-arrays: Don't attempt to warn if no *print-err-fn* set Created: 13/Jul/17  Updated: 14/Jul/17  Resolved: 14/Jul/17

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

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


 Description   

If you set :checked-arrays to :warn, if any of your namespaces, during initialization, triggers a checked-array warning, then it will attempt to warn, but since *print-err-fn* is not yet set, you may not be able to bring up your application. (This can happen, even if in your main entry-point namespace your first bit of code is (enable-console-print!)).

If this occurs, my fear would be that developers would have no recourse but to turn off :checked-arrays. (I can't think of a clever way to get around it.)

Will attach a patch for consideration that simply punts for these "initialization-time" warnings and doesn't warn if the *print-err-fn* has not yet been set to the default initial throwing variant.



 Comments   
Comment by Mike Fikes [ 13/Jul/17 8:10 PM ]

Perhaps another solution is CLJS-2002.

Comment by David Nolen [ 14/Jul/17 5:24 AM ]

Fixed by the partial resolution of CLJS-2002 https://github.com/clojure/clojurescript/commit/797e247fbef676544060a57da995f058db061f37





[CLJS-2002] Don't throw when no *print-fn* is set Created: 07/Apr/17  Updated: 13/Jul/17

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: errors, warnings

Attachments: Text File CLJS-2002.patch    
Approval: Vetted

 Description   

Currently calling (enable-console-print!) causes a bunch of code to be retained in :advanced mode even if you never print.

While that is not ideal it doesn't cause runtime errors. Not calling it and trying to print however will throw an exception which will potentially break your app.

No *print-fn* fn set for evaluation environment

So we end up in a no-win situation for :advanced builds where a "forgotten" prn may break your app in production or "maybe" bloating your file size by retaining all the print-related things.

I think the no-print-fn condition should never throw, maybe just try to write a warning using console.log. Or just dropping the prn altogether.



 Comments   
Comment by David Nolen [ 13/Jul/17 8:29 PM ]

Let's move the old behavior to `string-print` only.

Comment by Mike Fikes [ 13/Jul/17 9:01 PM ]

The attached CLJS-2002.patch moves the throw to string-print and using nil as the init for the two Vars solves CLJS-2231. But it doesn't address the ticket as written: Leaving an inadvertent prn in your code leads to a call to string-print which throws.

Comment by David Nolen [ 13/Jul/17 10:06 PM ]

https://github.com/clojure/clojurescript/commit/797e247fbef676544060a57da995f058db061f37 partially addresses this issue. Keeping this open and moving to lower priority, we should revisit.





[CLJS-2221] cljs.util/relative-name still has issues on case-insensitive platforms Created: 13/Jul/17  Updated: 13/Jul/17

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

Type: Defect Priority: Minor
Reporter: Mark Hepburn Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows (file-system is case-insensitive)
Emacs + Cider, and cmd.exe


Attachments: Text File CLJS-2221.patch     File path-for-path-handling.diff    
Patch: Code
Approval: Vetted

 Description   

cljs.util/relative-name currently works by converting paths to strings and manipulating those. This can cause issues where the strings are identical apart from case: in my case relating to a file from deps.cljs, under Emacs (System/getProperty "user.dir") produces a path with a lower-case drive-letter ("c:\\Users
..."), while the URL argument to relative-name has an upper-case drive-letter, eg "C:\\Users
...".

I believe a more robust approach is to use java.nio.file.Path, which handles paths in a platform-specific way. Patch attached, but happy to take alternative suggestions of course.



 Comments   
Comment by Mark Hepburn [ 13/Jul/17 12:51 AM ]

(I haven't done a sweep of other path manipulation functions, but that might not be a bad idea either if this approach is accepted)

Comment by David Nolen [ 13/Jul/17 7:50 AM ]

Thanks! Please follow the patch guidelines here https://clojurescript.org/community/patches

Have you submitted your Clojure CA? https://clojure.org/community/contributing

Comment by Francis Avila [ 13/Jul/17 9:17 AM ]

java.nio.file.Path is new in Java 7, but Clojure officially supports Java 6. Using Path would therefore mean that ClojureScript requires Java 7+.

(Not saying this isn't the right approach, but just FYI that our Java version dependency might increase.)

Comment by David Nolen [ 13/Jul/17 10:22 AM ]

ClojureScript already has a hard dependency on Java 7 due to Google Closure and its dependencies.

Comment by Mark Hepburn [ 13/Jul/17 6:32 PM ]

Thanks Francis, I'll admit that did cross my mind and while I thought Java7 was the minimum, I forgot to check.

Thanks David; I've signed the CA and I'll attach the re-formatted patch.

Comment by Mark Hepburn [ 13/Jul/17 6:33 PM ]

Renamed according to guidelines, and comment expanded.





[CLJS-2227] Squelch some of the array access tests Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Description   

The array access tests trigger noisy logs that make it look like the tests are failing, especially under V8 and the self-hosted Node tests, where js/console.log works. This ticket asks them to be squelched.



 Comments   
Comment by David Nolen [ 13/Jul/17 6:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/73173111b04c4b23ed979d3269fc2b12b196fcd3





[CLJS-2228] Port CLJS-2226 to module_deps.js Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 13/Jul/17 5:25 PM ]

fixed https://github.com/clojure/clojurescript/commit/26fb6b1f19d0b698c7720c3140f107fa360fd7fe





[CLJS-1955] data_readers.cljc can't reference handlers in user code Created: 27/Feb/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

Type: Defect Priority: Critical
Reporter: Dustin Getz Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: reading
Environment:

clojurescript 1.9.456-493


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

 Description   

data_readers.cljc:

{a/UserIdentity foo.core/user-identity}
(ns foo.core)
(defn user-identity [i] i)
(assert (= 1 #a/UserIdentity 1))

`CompilerException java.lang.IllegalStateException: Attempting to call unbound fn: #'foo.core/user-identity

Using a #' in data_readers.cljc results in a different error:
`java.lang.ClassCastException: clojure.lang.Cons cannot be cast to clojure.lang.Named`



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

The patch must come with a test case.

Comment by António Nuno Monteiro [ 08/Jul/17 5:12 PM ]

Patch attached.

Comment by David Nolen [ 13/Jul/17 5:04 PM ]

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





[CLJS-2206] Refactor :npm-deps functionality to use indexed node_modules instead of handling missing modules in special pass Created: 10/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

Approval: Vetted

 Description   

Doing this will have the added benefit in supporting enhancements to the Node.js target. By indexing `node_modules` up front we can later determine how a particular library should be required - either `goog.require` or CommonJS `require`.



 Comments   
Comment by David Nolen [ 11/Jul/17 4:55 PM ]

Depends on CLJS-2211

Comment by David Nolen [ 11/Jul/17 5:35 PM ]

Partially addressed by CLJS-2212. We should audit / remove any vestigial missing-js-modules logic if there is any.





[CLJS-2213] Node.js target should use node_modules index to emit platform specific require Created: 11/Jul/17  Updated: 13/Jul/17  Resolved: 12/Jul/17

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

Type: Task Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: nodejs

Attachments: Text File CLJS-2213.patch    
Approval: Vetted

 Description   

Previously users had to require files differently when targeting Node.js. With CLJS-2211 we can now have an index of top level node_modules finally allowing us to require Node.js files in an idiomatic fashion.



 Comments   
Comment by António Nuno Monteiro [ 12/Jul/17 1:43 AM ]

Attached patch + tests.

Comment by David Nolen [ 12/Jul/17 6:59 AM ]

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

Comment by David Nolen [ 12/Jul/17 7:00 AM ]

One potential issue with this patch is that it reverts CLJS-2151, https://github.com/clojure/clojurescript/commit/addaf39e861191dab6aeb0189c66f5f6ef132918.

Comment by David Nolen [ 12/Jul/17 7:05 AM ]

Oh the other thing I noticed is that `:rename` feature doesn't appear to be supported for Node libs?





[CLJS-2225] Need to add :checked-arrays to known compiler opts Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: closure

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

 Description   

So, for example the compiler will warn if you misspell:

WARNING: Unknown compiler option ':checked-array'. Did you mean ':checked-arrays'?


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

fixed https://github.com/clojure/clojurescript/commit/2247b0c2be4f2900bebfb544fea18ea3ed58f540





[CLJS-2226] :npm-deps can't index scoped packages Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 13/Jul/17 4:47 PM ]

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





[CLJS-2224] resolve-var is wrong wrt. module resolution Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Description   

core names & user-defined vars should take precedence over modules called as functions



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

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





[CLJS-2223] Self-host: Undeclared Var deps/native-node-modules Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 13/Jul/17 3:10 PM ]

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





[CLJS-2198] Safe array operations Created: 09/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: core

Attachments: Text File CLJS-2198-v10.patch     Text File CLJS-2198-v11.patch     Text File CLJS-2198-v1.patch     Text File CLJS-2198-v2.patch     Text File CLJS-2198-v3.patch     Text File CLJS-2198-v4.patch     Text File CLJS-2198-v5.patch     Text File CLJS-2198-v6.patch     Text File CLJS-2198-v7.patch     Text File CLJS-2198-v8.patch     Text File CLJS-2198-v9.patch    
Patch: Code and Test
Approval: Vetted

 Description   

We need an new compiler flag `*unchecked-array-access*`.

We should add a `safe-aget`, `safe-aget` runtime functions. These functions should make three assertions before bottoming out to `unsafe-get`:

  • first argument `array?`
  • second argument must be `number?`
  • second argument must be in array bounds

If `:invalid-array-access` is true, compilation mode is not `:advanced` and `*unchecked-array-access*` is not true, then `aget/aset` macro should not generate inlined JS array access but should instead call `safe-aget` and `safe-aset`.

We also need to update `aset/aget` runtime fns to also do these assertions under the same compiler option configuration specified above.

Finally `*unchecked-array-access*` needs to be be set to true for the standard library. It's important that this flag is file local and we should provide a simple test that setting it in one file will not affect a dependent namespace.



 Comments   
Comment by Mike Fikes [ 09/Jul/17 9:16 PM ]

A first draft patch is attached, which works, but highlights some of the deeper questions that arise (at least with the approach in the patch). I think it is worth playing with the implementation at this point, and seeing if ideas come up on how to better do some things. Here are some fairly extensive notes attempting to describe the patch:

*unchecked-array-access* is very similar to *unchecked-if*

ana/unchecked-array-access? returns the analyzer Var if in clj and the core Var if in self-host (this could probably be dispensed with, but for now it has the same "shape" as *unchecked-if*, which might make it simpler to reason about)

*unchecked-array-access* is slightly different from *unchecked-if* in that it doesn't need to be baked into the AST for conveyance to the compiler like we need for the if special form. This is because macroexpansion is done in the analyzer and we can get away with an analyzer dynamic Var for what we are doing.

(:parallel-build -> really works because only one namespace uses it? I wonder the same about *unchecked-if).

File-scope aspect? We don't need to un-set it at the bottom of file if we have some other mechanism to turn it off after file processed. It seems that this occurs in JVM ClojureScript, but not in self-hosted (perhaps owing to reliance on the Var in cljs.core, or perhaps owing to a lack of setting a dynamic Clojure Var that unsets itself.) I haven't tracked this down. There is a test for this if we come up with a good way. Otherwise it is single gigantic scopes being manually done (like the smaller *unchecked-if* scopes, but covering whole files).

The runtime aget/aset functions delegate to macros for their behavior, but we wrap the scope of where they are defined to turn off *unchecked-array-access*, so they can vary based on :invalid-array-access and :advanced

:invalid-array-access is build-affecting

Can't figure out how to make runtime aget/aset be AOT able (because it changes based :invalid-array-access setting)

unsafe-set added and used as bottom-out for safe-set. If instead we have it call aset, aset will then call safe-set, which will then call aset... infinite recursion

Tried not to introduce any new public Vars anywhere in cljs.core (apart from unsafe-set). In particular the safe-aget and safe-aset are private

Ended up using unsafe-get in the few places that aget is used in the cljs.core macros namespace. Without doing this, what appears to be happening is if you, for example, define a variadic fn, the defn gets expanded, and you end up using safe-gets where you wanted unsafe-gets.

We lose our static analysis warnings because they are based on looking at the resulting js* and seeing if the :op is cljs.core/aget. When we turn on :invalid-array-access, aget gets expanded to safe-aget, which has inside of it some checks, and a bottom out at unsafe-get, thus causing the wrong :op. It would be nice to think through this so we can re-gain some static analysis checks to find incorrect code before runtime.

Comment by David Nolen [ 10/Jul/17 5:22 AM ]

Thanks this is helpful. With respect to the patch, first I'd like to go with Christophe Grand's feedback about naming:

  • unsafe-set -> unchecked-set
  • unsafe-get -> unchecked-get
  • safe-set -> checked-aset
  • safe-get -> checked-aget

The file scoping falls out of the fact that we bind thread local dynamic vars when compiling or analyzing files. So no need to unset. In bootstrapped I suspect this has to be done differently because of the asynchronous design.

We can reclaim static analysis with an analyzer pass (cljs.analyzer/*passes*) that looks at checked-aset and checked-aget invoke ASTs and checks that tags of the argument ASTs.

Comment by David Nolen [ 10/Jul/17 5:40 AM ]

I think I understand the point about aset/aget and AOT. Why not just alias higher order aget/aset to checked-aset/checked-aget using the same checks for the other cases?

Comment by Mike Fikes [ 10/Jul/17 9:16 AM ]

The attached CLJS-2198-v2.patch addresses all of David's feedback apart from the runtime / AOT issue.

  • Names refactored to Grand's suggestion
  • File scoping addressed in self-host and tests revised
  • Static analysis reclaimed by simply looking for :js-op of cljs.core/checked-aget and cljs.core/checked-aset

The difficulty with the runtime / AOT issue: It would appear that the conditions that drive things are only available at compile time. With the way the patch is structured, the runtime functions bottom out in macros that switch based on the compile time setting, and we get what we want apart from the negative aspect that cljs.core effectively gets recompiled (we don't really want it to be affected by build-affecting options).

Perhaps one way out of this (perhaps this is what David alludes to with aliasing?) is to, when starting up a REPL, monkey patch aget / aset to instead be their checked variants. I haven't put too much thought into this approach.

Comment by David Nolen [ 10/Jul/17 9:40 AM ]

re: AOT. We handle cljs.core as a special case already. Basically we can just always compile `cljs.core` with `:invalid-array-access` set to true. This will avoid triggering rebuilds of core when users change the compiler options.

re: aset/aget - all I'm suggesting is that `aset/aget` become aliases for the checked variants in higher order cases as long as the compiler options call for it.

(map aget ...)

Should become:

(map checked-aget ...)

We can handle this during var resolution I think.

Comment by Mike Fikes [ 10/Jul/17 9:49 AM ]

Ahh. Makes sense.

I was wrong, by the way with respect to reclaiming static analysis. Will also revisit the idea of an analyzer pass.

Comment by Mike Fikes [ 10/Jul/17 10:17 PM ]

Added CLJS-2198-v3.patch

Notes on revisions in this patch:

The AOT stuff appears to be working: There is now code in the resolve path that conditionally substitutes in the checked variants for runtime aget / aset while using precisely the same function used by the macros when they are expanding. One way to see that this aliasing is working is to

Given this, it is possible to remove the temporary unset and reset of *unchecked-array-access* that was wrapping aget / aset. This means those functions (and all of runtime cljs.core, really) is unconditionally unchecked), except of course, checked-aget / checked-aset, which employ checks as assertions via :pre.

This means that the runtime cljs.core is not affected by :invalid-array-access, and can be safely AOTd, and the user can change To ensure that this happens, there is a bit where I have the code dissoc any :warnings that may have been set (near the same place where we set :static-fns true when compiling core.

The static analysis checks have also been reclaimed by adding an additional pass. (As an aside, this opens the door to easily adding other static analysis checks for other runtime functions outside of checked-aset and checked-aget. For example, I found it was trivial to add a check for bit-count once this machinery was in place.

The only problem with the static warnings is that they appear twice now. Here is an example:

cljs.user=> (aget #js {:foo 1} "foo")
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1 <cljs repl>
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) in file <cljs repl>

I'm speculating that this is because of the analysis that occurs right at the end of macroexpansion, with perhaps another analysis ocurring on the code returned from macroexpansion. (You only get one such warning if you try it directly with checked-aget). I tried putting a analyzed wrapper call around what I thought was the right place for this but haven't solved that yet.

So that's where things stand right now (just this one double warning bug is known). But, to be honest, this is a fairly large patch with a lot of subtle moving parts that will probably only become robust with more testing and time, I'm thinking.

Comment by Mike Fikes [ 10/Jul/17 10:40 PM ]

Attaching CLJS-2198-v4.patch which has minor revisions to remove leftover unnecessary code which was in CLJS-2198-v3.patch.

Comment by David Nolen [ 11/Jul/17 6:10 AM ]

Thanks will take a look today and see if I can't find the double warning issue. We should make a test case for that once we discover the cause.

Comment by Mike Fikes [ 11/Jul/17 6:34 AM ]

If you add a bit of code to (prn (ex-info "" {})) fight at the point the warning is thrown, you can see that the stacks involved are the same apart from a little extra part of the stack that exists when the first warning is emitted:

$ diff -C 20 a.txt b.txt
*** a.txt	2017-07-11 07:28:09.000000000 -0400
--- b.txt	2017-07-11 07:28:20.000000000 -0400
***************
*** 1,40 ****
  [[clojure.core$ex_info invokeStatic "core.clj" 4725]
    [clojure.core$ex_info invoke "core.clj" 4725]
    [cljs.analyzer$check_invoke_arg_types invokeStatic "analyzer.cljc" 3380]
    [cljs.analyzer$check_invoke_arg_types invoke "analyzer.cljc" 3373]
    [cljs.analyzer$analyze_STAR_$fn__3102 invoke "analyzer.cljc" 3439]
    [clojure.lang.PersistentVector reduce "PersistentVector.java" 341]
    [clojure.core$reduce invokeStatic "core.clj" 6703]
    [clojure.core$reduce invoke "core.clj" 6686]
    [cljs.analyzer$analyze_STAR_ invokeStatic "analyzer.cljc" 3439]
    [cljs.analyzer$analyze_STAR_ invoke "analyzer.cljc" 3429]
    [cljs.analyzer$analyze invokeStatic "analyzer.cljc" 3463]
    [cljs.analyzer$analyze invoke "analyzer.cljc" 3446]
-   [cljs.analyzer$analyze_seq invokeStatic "analyzer.cljc" 3245]
-   [cljs.analyzer$analyze_seq invoke "analyzer.cljc" 3222]
-   [cljs.analyzer$analyze_form invokeStatic "analyzer.cljc" 3391]
-   [cljs.analyzer$analyze_form invoke "analyzer.cljc" 3387]
-   [cljs.analyzer$analyze_STAR_ invokeStatic "analyzer.cljc" 3438]
-   [cljs.analyzer$analyze_STAR_ invoke "analyzer.cljc" 3429]
-   [cljs.analyzer$analyze invokeStatic "analyzer.cljc" 3463]
-   [cljs.analyzer$analyze invoke "analyzer.cljc" 3446]
    [cljs.analyzer$analyze invokeStatic "analyzer.cljc" 3455]
    [cljs.analyzer$analyze invoke "analyzer.cljc" 3446]
    [cljs.analyzer$analyze invokeStatic "analyzer.cljc" 3453]
    [cljs.analyzer$analyze invoke "analyzer.cljc" 3446]
    [cljs.analyzer$analyze_let_binding_init invokeStatic "analyzer.cljc" 1761]
    [cljs.analyzer$analyze_let_binding_init invoke "analyzer.cljc" 1759]
    [cljs.analyzer$analyze_let_bindings_STAR_ invokeStatic "analyzer.cljc" 1781]
    [cljs.analyzer$analyze_let_bindings_STAR_ invoke "analyzer.cljc" 1770]
    [cljs.analyzer$analyze_let_bindings invokeStatic "analyzer.cljc" 1812]
    [cljs.analyzer$analyze_let_bindings invoke "analyzer.cljc" 1811]
    [cljs.analyzer$analyze_let invokeStatic "analyzer.cljc" 1827]
    [cljs.analyzer$analyze_let invoke "analyzer.cljc" 1822]
    [cljs.analyzer$eval2303$fn__2304 invoke "analyzer.cljc" 1848]
    [clojure.lang.MultiFn invoke "MultiFn.java" 251]
    [cljs.analyzer$analyze_seq_STAR_ invokeStatic "analyzer.cljc" 3215]
    [cljs.analyzer$analyze_seq_STAR_ invoke "analyzer.cljc" 3213]
    [cljs.analyzer$analyze_seq_STAR__wrap invokeStatic "analyzer.cljc" 3220]
    [cljs.analyzer$analyze_seq_STAR__wrap invoke "analyzer.cljc" 3218]
    [cljs.analyzer$analyze_seq invokeStatic "analyzer.cljc" 3244]
    [cljs.analyzer$analyze_seq invoke "analyzer.cljc" 3222]
--- 1,32 ----
Comment by David Nolen [ 11/Jul/17 7:07 AM ]

Thanks! Feedback on the latest patch:

  • Lift out the internal aliasing in resolve-var into a helper.
  • the change to compile-file is too aggressive and problematic for compiler devs. Just update-in to set :invalid-array-access to false or the value of *invalid-array-access* so it can be manually overridden.

Otherwise looks good. Still looking into the double warning issue.

Comment by Mike Fikes [ 11/Jul/17 8:39 AM ]

Added CLJS-2198-v5.patch which addresses the two issues in the last comment, but not the double-warning issue.

The lifting of the aliasing is pretty straightforward. (You might have nits about names, etc.)

Take a look at the updated code surrounding build-affecting options. It was revised a little more heavily than you had suggested in the comment, largely around the idea of properly handing {:warnings true} as being another way that :invalid-array-access can get set. The stuff put in the build-affecting comment when enabled is simply :invalid-array-access-warning-enabled true.

Comment by David Nolen [ 11/Jul/17 10:36 AM ]

Based on the feedback so far it looks like we need to emphasize runtime warnings and not hard errors if users are going to be productive with this enabled.

  • add `:checked-arrays` compiler flag which is either false-y, :warn, :error
  • use this option to set the relevant analyzer warnings
  • add `checked-aget'`, `checked-aset'` which allocate an Error to get the raw stacktrace
  • use the warning variants based on the `:checked-arrays` setting
Comment by David Nolen [ 11/Jul/17 4:54 PM ]

The double warning issue is because the check AST pass doesn't mark AST nodes as being analyzed. Just call `analyzed` on the returned `ast` and don't check the `ast` argument if `analyzed?` returns true on it.

Comment by Mike Fikes [ 11/Jul/17 5:14 PM ]

CLJS-2198-v6.patch addresses the double warning.

Remaining known issues include the "knob" revisions that allow users to choose to have either only static analysis warnings or that plus runtime checks.

There is also a problem with initial compile behaving strangely in a real-world project:

What I see occur: I set :warnings true. Then lein clean, lein figwheel. My code gets compiled by Figwheel when it starts up, but no warnings appear. If I look at the JavaScript in my out directory, I see the {:invalid-array-access-warning-enabled true} build-affecting option recorded, but I don't see checked-aget in the JavaScript. If I then go in the REPL and use :reload on a namespace, I then see warnings appear in the REPL console, and if I go back and look at the JavaScript in out, the unchecked

return (({"foo": (1)})["foo"])
code snippets are replaced with code using checked_aget:
return cljs.core.checked_aget.call(null,({"foo": (1)}),"foo");

Comment by Mike Fikes [ 11/Jul/17 7:55 PM ]

The CLJS-2198-v7.patch patch addresses the "knob" idea.

Before providing notes on the changes to implement it, here is a high-level description of the feature that would be targeted to users (this helps ensure we are on the same page with respect to the behavior of the feature):

A new compiler option exists, :checked-arrays. If not set (or set to a false-y value), the compiler behaves as before. If set to :warn, then, where possible via static type inference, warnings will be emitted for passing invalid arguments to aget/aset. The warnings will be emitted via the :invalid-array-access warning key, which is enabled by default, but can be suppressed by setting it to false. If set to :error, in addition to the warning behavior just described, runtime checks will be made for proper values being passed (array objects, and indices within bounds), with these being in the form of assertions (thus, subject to :elide-asserts). When compiling in :advanced mode, all this is disabled (as if :checked-arrays were set to false).

Implementation notes:

Renamed *unchecked-array-access* to *unchecked-arrays* (for consistency with :checked-arrays, and it seems closer in naming to *unchecked-if*). A minor note is that this is not included in the user-targeted description above (it seems just as "internal" as *unchecked-if*). Let me know if this rename is undesired and I'll revert it.

Whereas previous patches had an ana/emit-safe-array-access?, it has been revised with a new ana/checked-arrays, which returns false-y, :warn, or :error, and this is used to drive macroexpansion and aliasing (to pick between unchecked, type-inference-checked (checked-aget), or runtime-value-checked (checked-aget')). This is not simply the compiler option :checked-arrays value, but is guarded by looking to see that we are not in :advanced, and that *unchecked-arrays* hasn't been set (if either are true, it will return false-y).

The new checked-aget'/checked-aset' are just like the older checked-aget/checked-aset, while checked-aget/checked-aset are just there (without :pre) in order for the type inference analsysis pass to hook on.

The build-affecting story becomes simpler now: it is just the :checked-arrays compiler option that is build-affecting. We dissoc that option if compiling cljs.core.

The :invalid-array-access warning is on by default in the latest patch, and really exists if a user wants to write custom code to react to it in a certain way (using cljs.analyzer/with-warning-handlers), or if a user wants to suppress it by setting :warnings {:invalid-array-access false}.

The patch doesn't try to use :checked-arrays to set :invalid-array-access, nor :invalid-array-access to set :checked-arrays.

I think other aspects of the patch are straightforward; the above probably covers the aspects worth explaining.

The patch doesn't attempt to address the "initial compile" issue.

Comment by Mike Fikes [ 12/Jul/17 7:12 AM ]

CLJS-2198-v8.patch exposes feature to cljs.js and employs a dynamic Var (mimicking the way :static-fns is handled), as opposed to directly looking in the compiler environment for the option.

Tested :checked-arrays successfully downstream with Planck:

cljs.user=> (aget #js {:foo 1} "foo")
1
cljs.user=> (swap! planck.repl/app-env assoc :checked-arrays :warn)
{:repl true, :verbose false, :cache-path "/Users/mfikes/.planck_cache", :opts {}, :checked-arrays :warn}
cljs.user=> (aget #js {:foo 1} "foo")
            ^
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1
1
cljs.user=> (swap! planck.repl/app-env assoc :checked-arrays :error)
{:repl true, :verbose false, :cache-path "/Users/mfikes/.planck_cache", :opts {}, :checked-arrays :error}
cljs.user=> (aget #js {:foo 1} "foo")
            ^
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1
Assert failed: (or (array? array) (js/goog.isArrayLike array))
	cljs.core/checked-aget' (cljs/core.cljs:445:1)

Assigning ticket to me to look into initial compile issue.

Comment by Mike Fikes [ 12/Jul/17 8:41 AM ]

It turns out that CLJS-2198-v8.patch fixed the "initial compile" problem.

Attaching CLJS-2198-v9.patch which revises the behavior so that if :checked-arrays is set to :warn and a runtime invoke is made with invalid arguments, it does the same checks as :pre and logs the resulting exception.

Comment by Mike Fikes [ 12/Jul/17 6:08 PM ]

CLJS-2198-v10.patch rebaselines, adds unit test coverage for unchecked-get, aget, checked-get, checked-get', and their -set variants, and fixes an issue with maybe-warn where exists? needs to be used instead of some? when checking for the existence of js/console.warn.

Comment by Mike Fikes [ 13/Jul/17 12:42 PM ]

CLJS-2198-v11.patch rebaselined (note: unit tests haven't passed for it as master is currently failing)

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

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





[CLJS-2222] CI failing after CLJS-2217 Created: 13/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/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: None

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

 Comments   
Comment by David Nolen [ 13/Jul/17 12:48 PM ]

fixed https://github.com/clojure/clojurescript/commit/8005f5710eda74afff56b5eaa3513e23e7337318





[CLJS-2219] Enable JSC under test-simple Created: 12/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Description   

There doesn't seem to be a problem with nested function calls anymore



 Comments   
Comment by David Nolen [ 13/Jul/17 12:31 PM ]

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





[CLJS-2217] Support `:rename` for JS modules Created: 12/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Description   

This may just work when target is not `:nodejs`, need to add a test for that case.



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

Attached patch with fix. This patch should be applied after CLJS-2220 which adds a namespace for runtime testing of npm-deps, and relies on it.

Comment by António Nuno Monteiro [ 12/Jul/17 11:10 PM ]

Patch also contains a small refactoring that addresses the fact that we were accessing `(-> env :ns :name)` several times when we could just bind it in a `let` block.

Comment by David Nolen [ 13/Jul/17 8:03 AM ]

Noting that there is no test for `:rename` `:global-exports` case. Can either take an augmented patch, or I will look into adding it after this one is applied.

Comment by António Nuno Monteiro [ 13/Jul/17 11:36 AM ]

Replaced the patch with one that also tests global exports.

Comment by David Nolen [ 13/Jul/17 12:22 PM ]

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





[CLJS-2218] Make ClojureScript aware of native node modules Created: 12/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 13/Jul/17 12:12 PM ]

fixed https://github.com/clojure/clojurescript/commit/6530e0d0da91d6a5f324ae4b86ca1d2d208c40d9





[CLJS-2220] Add runtime :npm-deps tests Created: 12/Jul/17  Updated: 13/Jul/17  Resolved: 13/Jul/17

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

Type: Task Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, test

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

 Comments   
Comment by David Nolen [ 13/Jul/17 12:08 PM ]

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





[CLJS-2216] Support targeting webworker Created: 12/Jul/17  Updated: 13/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Dieter Komendera Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2216.patch    

 Description   

Add :webworker as a supported target.

This removes the need for a custom bootstrap script for webworkers in development, as described here:
https://github.com/bhauman/lein-figwheel/wiki/Using-Figwheel-with-Web-Workers

Attaching a first patch to review and feedback.



 Comments   
Comment by Thomas Heller [ 12/Jul/17 11:59 AM ]

I have :web-worker support in shadow-cljs but instead of having a dedicated build for it I used :modules. This means that the worker can share code with rest of the website. The web worker module is generated as usual but some extra code is prepended that uses importScripts to load the dependencies. Thanks to proper code splitting the user does not need to download cljs.core twice for :advanced builds, which he would have to in case of 2 separate builds.

Example: https://github.com/thheller/shadow-cljs/wiki/ClojureScript-for-the-browser#web-workers

Might be a better approach to do this via :modules? YMMV.

Comment by Dieter Komendera [ 13/Jul/17 6:24 AM ]

Our use case for having it as a different compile target is that we'd able to specify different :optimizations for the webworker. We're able to ship the webworker with :simple, so we can use ClojureScript eval in the webworker (with all the content policy restrictions one can put on a webworker), while compiling the main app with :advanced.

I realize this might be an edge case though.





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

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

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

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

 Description   

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



 Comments   
Comment by David Nolen [ 16/Jun/17 3:05 PM ]

Patch no longer applies

Comment by Thomas Mulvaney [ 13/Jul/17 5:46 AM ]

Rebased patch attached





[CLJS-2001] Add map-entry? predicate Created: 06/Apr/17  Updated: 13/Jul/17

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

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

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

 Description   

map-entry? has existed in Clojure since 1.8 would be nice to have it in ClojureScript.



 Comments   
Comment by Thomas Mulvaney [ 06/Apr/17 6:00 AM ]

The attached patch looks more like the first implementation of `map-entry?` as per CLJ-1831.

This is because ClojureScript returns PersistentVectors when iterating over PAM and PHM maps.

Comment by David Nolen [ 07/Apr/17 11:06 AM ]

This patch is no good. 2 element vectors are not MapEntry.

Comment by Francis Avila [ 07/Apr/17 7:22 PM ]

Given that Clojure still returns MapEntry (CLJ-1831 was backed out later) and CLJS returns vectors, it is probably impossible for this predicate to be portable. If we can't consider count-2 vectors map-entry?=true, then the only possible cljs impl is (defn map-entry? [x] false). Given this, perhaps the best solution is not to have map-entry? in cljs, to discourage people from using it in portable code.

Comment by David Nolen [ 12/Apr/17 1:03 PM ]

I'm fine with adding a MapEntry type which implements all the vector protocols and returning that instead. That work should be a separate issue though and then we can come back to this one.

Comment by Thomas Mulvaney [ 19/Apr/17 8:00 AM ]

This came about as I was porting some Clojure code. I was probably misusing/abusing map-entry? anyway. The code could be rewritten to check if something is a map first and then do the appropriate thing on the sequence of entries rather than doing the check from "with in" the collection.

As mentioned, a 2 element vector != MapEntry. So, I've opened an issue to track adding a MapEntry type: CLJS-2013

Comment by David Nolen [ 16/Jun/17 9:48 AM ]

Now that we have MapEntry we can do this one correctly.

Comment by Thomas Mulvaney [ 13/Jul/17 5:40 AM ]

Updated patch attached which checks if x is an instance of the recently added MapEntry type.





[CLJS-1598] Honor printing of function values via IPrintWithWriter Created: 03/Mar/16  Updated: 12/Jul/17  Resolved: 12/Jul/17

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

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

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

 Description   

If a user wishes to define how function values are printed, allow that to be controlled via IPrintWithWriter with code like

(extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer opts]
    ,,,))


 Comments   
Comment by Mike Fikes [ 03/Mar/16 10:28 AM ]

Can be tested manually:

$ script/nashornrepljs 
To quit, type: :cljs/quit
cljs.user=> inc
#object[cljs$core$inc "function cljs$core$inc(x){
return (x + (1));
}"]
cljs.user=> (extend-type function
  IPrintWithWriter
  (-pr-writer [obj writer _]
    (let [name (.-name obj)
          name (if (empty? name)
                 "Function"
                 name)]
      (write-all writer "#object[" name "]"))))
#object[Function]
cljs.user=> inc
#object[cljs$core$inc]
Comment by David Nolen [ 11/Mar/16 1:04 PM ]

The problem is this makes printing slower. For people using EDN as interchange format this may be a problem. Would need to see some numbers.

Comment by Antonin Hildebrand [ 08/Apr/16 2:11 PM ]

I'm not sure what is the difference between implements? and satisfies?. But by reading the code I would assume that it should be printed by this line:
https://github.com/clojure/clojurescript/blob/9a2be8bc665385be1ef866e2fd76b476c417d2bf/src/main/cljs/cljs/core.cljs#L9056-L9057

Don't we want to change implements? to satisfies? there? Not sure about (perf) implications.

Comment by Mike Fikes [ 12/Jul/17 4:32 PM ]

To be honest, this commit addressee the core reason I wrote this ticket (I wouldn't personally mind if this ticket is closed). https://github.com/clojure/clojurescript/commit/6be311f5be76d86ec4b02e8d47e7c58fbd016860





[CLJS-2214] :global-exports for foreign libraries Created: 11/Jul/17  Updated: 12/Jul/17  Resolved: 12/Jul/17

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

Type: Task Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: foreign-libs, migration

Attachments: Text File CLJS-2214-2.patch     Text File CLJS-2214.patch    
Approval: Accepted

 Description   

To make foreign libraries more idiomatic to use and in order to ease future migration to direct node_modules usage users need a way to define what global exports a foreign lib provides. Regular foreign libraries export global names and the requires are synthetic. We can fix this issue by allowing foreign libraries to describe what they globally export.

{:file ...
 :file-min ...
 :requires [...]
 :provides [...]
 :global-exports '{cljsjs.react React 
                   cljsjs.react/dom-server ReactDOMServer}} ;; map :provides to a :require'able name


 Comments   
Comment by David Nolen [ 12/Jul/17 11:26 AM ]

`global$module$` for the module name please. And lets lift out some helpers to make this cleaner.

Comment by David Nolen [ 12/Jul/17 4:48 PM ]

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





[CLJS-2191] Clean up doc references to clojure.spec.* in favor of cljs.spec.* Created: 08/Jul/17  Updated: 12/Jul/17  Resolved: 09/Jul/17

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

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

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

 Description   

There are a handful of docstrings that refer to the namespaces as clojure.spec.* instead of cljs.spec.* that could be cleaned up:

  1. In the regex? predicate
  2. The keyword in the conform docstring
  3. The fdef docstring

These can also be cleaned up to append alpha (as is done in other docstrings).



 Comments   
Comment by David Nolen [ 09/Jul/17 8:13 AM ]

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





[CLJS-1885] assoc should return an array-map when passed a nil collection Created: 09/Jan/17  Updated: 12/Jul/17  Resolved: 08/Jul/17

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

Type: Enhancement Priority: Major
Reporter: ewen grosjean Assignee: ewen grosjean
Resolution: Duplicate Votes: 0
Labels: None

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

 Comments   
Comment by David Nolen [ 08/Jul/17 10:02 AM ]

Looks like this was resolved with CLJS-1994





[CLJS-2158] cljs_base module generates empty goog.require Created: 03/Jul/17  Updated: 12/Jul/17  Resolved: 03/Jul/17

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: browser-repl, modules

Approval: Accepted

 Description   

When using :modules under :none and testing via browser REPL you will see a goog.require error due an empty goog.require generated for the cljs_base module file.



 Comments   
Comment by David Nolen [ 03/Jul/17 8:28 AM ]

I believe the problem is that we create a cljs_base.js file in optimizations :none with an empty goog.require. The JS console error seems to corroborate this:

goog.require could not find:

Note no namespace is identified.

Comment by David Nolen [ 03/Jul/17 11:14 AM ]

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





[CLJS-2183] Assert arguments are quoted symbols in some core macros Created: 06/Jul/17  Updated: 12/Jul/17  Resolved: 07/Jul/17

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

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

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

 Description   

Some core macros suffer the problem of destructuring too early, not allowing for current assertions to be called



 Comments   
Comment by David Nolen [ 07/Jul/17 1:37 PM ]

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





[CLJS-1646] Internal compiler error when importing Showdown via CommonJS Created: 24/May/16  Updated: 12/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: Juan Pedro Bolivar Puente Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

cljs 1.8.51, clojure 1.8, lein-cljsbuild 1.1.3



 Description   

Hi!

I am trying to use Showdown 1.4.1 from NPM [1] using a `:commonjs` foreign-lib. Sadly, compilation crashes with the following error:

java.lang.RuntimeException: INTERNAL COMPILER ERROR.
Please report this problem.

null
Node(SCRIPT): node_modules/showdown/dist/showdown.js:1:0
[source unknown]
Parent: NULL
at com.google.javascript.jscomp.ES6ModuleLoader.normalizeInputAddress(ES6ModuleLoader.java:118)
at com.google.javascript.jscomp.ProcessCommonJSModules.inputToModuleName(ProcessCommonJSModules.java:89)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visitScript(ProcessCommonJSModules.java:336)
at com.google.javascript.jscomp.ProcessCommonJSModules$ProcessCommonJsModulesCallback.visit(ProcessCommonJSModules.java:245)
at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:623)
at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:297)
at com.google.javascript.jscomp.NodeTraversal.traverseEs6(NodeTraversal.java:564)
at com.google.javascript.jscomp.ProcessCommonJSModules.process(ProcessCommonJSModules.java:85)
at cljs.closure$eval5486$fn__5487.invoke(closure.clj:1541)
at clojure.lang.MultiFn.invoke(MultiFn.java:233)
at cljs.closure$write_javascript.invokeStatic(closure.clj:1601)
at cljs.closure$write_javascript.invoke(closure.clj:1578)
at cljs.closure$source_on_disk.invokeStatic(closure.clj:1624)
at cljs.closure$source_on_disk.invoke(closure.clj:1619)
at cljs.closure$output_unoptimized$fn__5536.invoke(closure.clj:1662)
at clojure.core$map$fn__4785.invoke(core.clj:2646)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$filter$fn__4812.invoke(core.clj:2700)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$map$fn__4785.invoke(core.clj:2637)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:688)
at clojure.core$next__4341.invokeStatic(core.clj:64)
at clojure.core$str$fn__4419.invoke(core.clj:546)
at clojure.core$str.invokeStatic(core.clj:544)
at clojure.core$str.doInvoke(core.clj:533)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:646)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$deps_file.invokeStatic(closure.clj:1340)
at cljs.closure$deps_file.invoke(closure.clj:1337)
at cljs.closure$output_deps_file.invokeStatic(closure.clj:1360)
at cljs.closure$output_deps_file.invoke(closure.clj:1359)
at cljs.closure$output_unoptimized.invokeStatic(closure.clj:1670)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1653)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invokeStatic(core.clj:648)
at clojure.core$apply.invoke(core.clj:641)
at cljs.closure$build.invokeStatic(closure.clj:1981)
at cljs.closure$build.invoke(closure.clj:1882)
at cljs.build.api$build.invokeStatic(api.clj:210)
at cljs.build.api$build.invoke(api.clj:198)
at cljs.build.api$build.invokeStatic(api.clj:201)
at cljs.build.api$build.invoke(api.clj:198)
at cljsbuild.compiler$compile_cljs$fn__5771.invoke(compiler.clj:60)
at cljsbuild.compiler$compile_cljs.invokeStatic(compiler.clj:59)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:48)
at cljsbuild.compiler$run_compiler.invokeStatic(compiler.clj:168)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:122)
at user$eval5908$iter_59445948$fn5949$fn_5967.invoke(form-init8819033363931377476.clj:1)
at user$eval5908$iter_59445948$fn_5949.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:521)
at clojure.core$seq__4357.invokeStatic(core.clj:137)
at clojure.core$dorun.invokeStatic(core.clj:3024)
at clojure.core$doall.invokeStatic(core.clj:3039)
at clojure.core$doall.invoke(core.clj:3039)
at user$eval5908.invokeStatic(form-init8819033363931377476.clj:1)
at user$eval5908.invoke(form-init8819033363931377476.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6927)
at clojure.lang.Compiler.eval(Compiler.java:6917)
at clojure.lang.Compiler.load(Compiler.java:7379)
at clojure.lang.Compiler.loadFile(Compiler.java:7317)
at clojure.main$load_script.invokeStatic(main.clj:275)
at clojure.main$init_opt.invokeStatic(main.clj:277)
at clojure.main$init_opt.invoke(main.clj:277)
at clojure.main$initialize.invokeStatic(main.clj:308)
at clojure.main$null_opt.invokeStatic(main.clj:342)
at clojure.main$null_opt.invoke(main.clj:339)
at clojure.main$main.invokeStatic(main.clj:421)
at clojure.main$main.doInvoke(main.clj:384)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:383)
at clojure.lang.AFn.applyToHelper(AFn.java:156)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: java.lang.NullPointerException
... 85 more

[1] https://www.npmjs.com/package/showdown



 Comments   
Comment by Juan Pedro Bolivar Puente [ 24/May/16 12:39 PM ]

Forgot to mention, this is the line in my :foreign-libs

{:file "node_modules/showdown/dist/showdown.js"
:provides ["showdown"]
:module-type :commonjs}

Same problem occurs if I use showdown.min.js

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

Can't repro in ClojureScript 1.9.293. Showdown seems to be correctly consumed by the Closure Compiler.

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

FWIW, it also works for me in 1.8.51, could this related to tooling / dependency conflicts?





[CLJS-2139] Undeclared var regression in fn bodies Created: 29/Jun/17  Updated: 12/Jul/17  Resolved: 30/Jun/17

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

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

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

 Description   
(defn foo [] x)

No longer produces a warning. Probably related to the changes to suppress double warnings as a result of fn invoke optimization. The fix needs to also supply an analyzer test case.



 Comments   
Comment by David Nolen [ 29/Jun/17 5:31 PM ]

Actually I looked into this - not related to changes around double warnings an fn optimization. Dropping `warning-for` from analyze does not change the behavior.

Comment by Mike Fikes [ 29/Jun/17 6:58 PM ]

git bisect shows that this regression was introduced with the commit for CLJS-2066.

Comment by Mike Fikes [ 29/Jun/17 9:28 PM ]

The root cause is fairly cut-n-dry. The solution is fairly straightforward, but perhaps a little more complex than desired. Explanation is in commit comment in the attached patch. Adds a regression test specifically for this case.

Comment by Mike Fikes [ 29/Jun/17 9:53 PM ]

Since method param analysis never results in warnings being emitted, a much simpler (1-line) patch is sufficient for the problem. Attaching a v2 patch as an alternative to consider.

Comment by David Nolen [ 30/Jun/17 8:19 AM ]

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





[CLJS-2215] Allow clj->js to preserve namespaces Created: 11/Jul/17  Updated: 11/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Enzzo Cavallo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs, enhancement, interop

Attachments: Text File clj-to-js.patch    
Patch: Code and Test

 Description   

Original issue
https://dev.clojure.org/jira/browse/CLJS-536

Keypoints from CLJS-536

  • IEncodeJS is powerfull, but for keywords, can break other libreries that expect trim-ns behavior (Le Wang)
  • With the introduction of specs, the namespaced keywords are being used more and more. This issue prevents streamlined edn->json->edn transformation. I think it should be reopened. IMO the 'lossy' method should never be a default one. (Jozef Wagner)
  • Should be possible do this without break existing code and using kwargs (Paulus Esterhazy)

An use example can be `(clj->js {:foo/bar 33} :keyword-fn #(.-fqn %)) ;=> #js {:foo/bar 33}`

PS: key->js should use key>js method, but I keep it with clj>js to avoid break things (it should be another bug?!).






[CLJS-2212] Replace missing-js-modules with new index-node-module-dir Created: 11/Jul/17  Updated: 11/Jul/17  Resolved: 11/Jul/17

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

Type: Enhancement Priority: Blocker
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

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

 Comments   
Comment by David Nolen [ 11/Jul/17 5:32 PM ]

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





[CLJS-2211] Add function to index a top-level node_modules installation Created: 11/Jul/17  Updated: 11/Jul/17  Resolved: 11/Jul/17

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

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

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

 Description   

This is a pre-requisite for CLJS-2206



 Comments   
Comment by David Nolen [ 11/Jul/17 4:50 PM ]

fixed https://github.com/clojure/clojurescript/commit/921b2630804b387f342e54a711e220a1cf8ff0c6





[CLJS-2200] bump to tools.reader 1.0.2 Created: 09/Jul/17  Updated: 11/Jul/17  Resolved: 11/Jul/17

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

Type: Task Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: reader

Approval: Accepted

 Description   

We should bring back any relevant tests when this is done.



 Comments   
Comment by David Nolen [ 11/Jul/17 6:42 AM ]

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





[CLJS-2210] INTERNAL COMPILER ERROR when trying to compile web3 package in :npm-deps Created: 11/Jul/17  Updated: 11/Jul/17  Resolved: 11/Jul/17

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

Type: Defect Priority: Minor
Reporter: Matúš Lešťan Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: compiler
Environment:

Mac OS



 Description   

When I try to compile empty Clojurescript project with :npm-deps {:web3 "0.19.0"} I get INTERNAL COMPILER ERROR which says "Please report this problem." so I'm reporting.

Using [org.clojure/clojurescript "1.9.671"] and [lein-figwheel "0.5.11"]

Here's full error:

---- Exception ----

java.lang.RuntimeException : INTERNAL COMPILER ERROR.
Please report this problem.

null
Node(NAME BigNumber): /Users/matus/www/clojure-hacking/npm-deps-test/node_modules/web3/node_modules/bignumber.js/bignumber.js:15:8
var BigNumber, crypto, parseNumeric,
Parent(VAR): /Users/matus/www/clojure-hacking/npm-deps-test/node_modules/web3/node_modules/bignumber.js/bignumber.js:15:4
var BigNumber, crypto, parseNumeric,

java.lang.NullPointerException :

---- Exception Stack Trace ----

clojure.core/eval core.clj: 3105
...
user/eval32738 REPL Input
...
figwheel-sidecar.repl-api/start-figwheel! repl_api.clj: 26
figwheel-sidecar.repl-api/start-figwheel! repl_api.clj: 29
...
clojure.core/alter-var-root core.clj: 5294
clojure.core/alter-var-root core.clj: 5299
...
figwheel-sidecar.repl-api/start-figwheel!/fn repl_api.clj: 29
clojure.core/apply core.clj: 646
...
figwheel-sidecar.system/start-figwheel! system.clj: 695
figwheel-sidecar.system/start-figwheel! system.clj: 737
clojure.core/apply core.clj: 646
...
figwheel-sidecar.system/start-figwheel!* system.clj: 693
figwheel-sidecar.system/start-figwheel-system system.clj: 658
figwheel-sidecar.system/dispatch-system-component-errors system.clj: 644
figwheel-sidecar.system/start-figwheel-system/fn system.clj: 658
com.stuartsierra.component.SystemMap/start component.cljc: 178
com.stuartsierra.component/start-system component.cljc: 161
com.stuartsierra.component/start-system component.cljc: 163
...
com.stuartsierra.component/update-system component.cljc: 129
com.stuartsierra.component/update-system component.cljc: 135
clojure.core/reduce core.clj: 6544
...
com.stuartsierra.component/update-system/fn component.cljc: 139
com.stuartsierra.component/try-action component.cljc: 117
clojure.core/apply core.clj: 648
...
com.stuartsierra.component/eval30841/fn/G component.cljc: 5 (repeats 2 times)
figwheel-sidecar.system.FigwheelSystem/start system.clj: 118
clojure.core/swap! core.clj: 2260
...
com.stuartsierra.component/eval30841/fn/G component.cljc: 5 (repeats 2 times)
com.stuartsierra.component.SystemMap/start component.cljc: 178
com.stuartsierra.component/start-system component.cljc: 161
com.stuartsierra.component/start-system component.cljc: 163
...
com.stuartsierra.component/update-system component.cljc: 129
com.stuartsierra.component/update-system component.cljc: 135
clojure.core/reduce core.clj: 6544
...
com.stuartsierra.component/update-system/fn component.cljc: 139
com.stuartsierra.component/try-action component.cljc: 117
clojure.core/apply core.clj: 648
...
com.stuartsierra.component/eval30841/fn/G component.cljc: 5 (repeats 2 times)
figwheel-sidecar.components.cljs-autobuild.CLJSAutobuild/start cljs_autobuild.clj: 255
figwheel-sidecar.components.cljs-autobuild/color-output/fn cljs_autobuild.clj: 79
figwheel-sidecar.build-middleware.stamp-and-clean/hook/fn stamp_and_clean.clj: 66
figwheel-sidecar.components.cljs-autobuild/open-urls-hook/fn cljs_autobuild.clj: 141
figwheel-sidecar.components.cljs-autobuild/catch-print-hook/fn cljs_autobuild.clj: 185
figwheel-sidecar.components.cljs-autobuild/figwheel-start-and-end-messages/fn cljs_autobuild.clj: 48
figwheel-sidecar.components.cljs-autobuild/notify-command-hook/fn cljs_autobuild.clj: 68
figwheel-sidecar.build-middleware.injection/hook/fn injection.clj: 197
figwheel-sidecar.components.cljs-autobuild/cljs-build cljs_autobuild.clj: 28
cljs.build.api/build api.clj: 202
cljs.closure/build closure.clj: 2235
cljs.closure/process-js-modules closure.clj: 2161
...
clojure.core/mapcat core.clj: 2674 (repeats 2 times)
clojure.core/apply core.clj: 641
clojure.core/seq core.clj: 137
...
clojure.core/map/fn core.clj: 2646
cljs.closure/process-js-modules/fn closure.clj: 2162
...
cljs.closure/eval7106/fn closure.clj: 1607
com.google.javascript.jscomp.Compiler.parse Compiler.java: 1037
com.google.javascript.jscomp.Compiler.parseInputs Compiler.java: 1811
com.google.javascript.jscomp.Compiler.processAMDAndCommonJSModules Compiler.java: 2120
com.google.javascript.jscomp.ProcessCommonJSModules.process ProcessCommonJSModules.java: 127
com.google.javascript.jscomp.NodeTraversal.traverseEs6 NodeTraversal.java: 553
com.google.javascript.jscomp.NodeTraversal.traverse NodeTraversal.java: 305
com.google.javascript.jscomp.NodeTraversal.traverseBranch NodeTraversal.java: 604
com.google.javascript.jscomp.NodeTraversal.handleScript NodeTraversal.java: 579
com.google.javascript.jscomp.NodeTraversal.traverseChildren NodeTraversal.java: 701
com.google.javascript.jscomp.NodeTraversal.traverseBranch NodeTraversal.java: 625
com.google.javascript.jscomp.NodeTraversal.traverseChildren NodeTraversal.java: 701
com.google.javascript.jscomp.NodeTraversal.traverseBranch NodeTraversal.java: 629
com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.visit ProcessCommonJSModules.java: 705
com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.maybeUpdateName ProcessCommonJSModules.java: 943
com.google.javascript.jscomp.ProcessCommonJSModules$RewriteModule.updateNameReference ProcessCommonJSModules.java: 1072
java.lang.NullPointerException:



 Comments   
Comment by David Nolen [ 11/Jul/17 6:08 AM ]

Sorry the message is a bit confusing but you are reporting this to the wrong project. That's a compiler error internal to Google Closure Compiler not ClojureScript.





[CLJS-2209] case docstring should explain constants may be evaluated (cljs only) Created: 10/Jul/17  Updated: 10/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Approval: Vetted

 Description   

The ClojureScript docstring for case says, "The test-constants are not evaluated." But that statement is not complete. The ClojureScript "Differences from Clojure" page <https://www.clojurescript.org/about/differences> says ":const metadata: ... causes case test constants which are symbols resolving to ^:const Vars to be inlined with their values". The case docstring should reflect that. Related discussion at <https://groups.google.com/d/msg/clojure/u1RZsmjbQ64/p7B9eRwuAQAJ>.






[CLJS-2208] module_deps.js is not compatible with older JS implementations Created: 10/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 10/Jul/17 12:41 PM ]

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





[CLJS-2207] cljs.test/js-filename is using non-portable .endsWith Created: 10/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

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

 Description   

.startsWith and .endsWith evidently require ECMAScript 6. (These are not on older versions of JavaScriptCore. In particular the CI tests are failing.)

A regression only on master currently with the change for CLJS-1966.



 Comments   
Comment by David Nolen [ 10/Jul/17 8:13 AM ]

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





[CLJS-1764] Double warning for undeclared Var (REPL only) Created: 26/Aug/16  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

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

 Description   

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

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

(def x 2)

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


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

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

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

Only seems to happen at the REPL

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

Patch with fix.

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

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

How will this not effect non REPL cases?

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

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

*analyze-deps*
at the REPL?

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

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

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

abc

(defn x [y] xyz)

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

Comment by David Nolen [ 08/Jul/17 10:34 AM ]

pretty sure this was resolved when Mike Fikes fixed how we process fn bodies.

Comment by David Nolen [ 08/Jul/17 12:23 PM ]

Can confirm this is still a problem thanks to bump from António, however I do not understand the patch. It doesn't seem safe to suppress warnings here like that.

Comment by David Nolen [ 10/Jul/17 5:35 AM ]

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





[CLJS-2204] Tests failing with respect to lodash/array namespace Created: 09/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

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

 Description   

script/test results in

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:src/test/cljs/npm_deps_test/string_requires_in_classpath.cljs {:file #object[java.io.File 0x2ef812b "src/test/cljs/npm_deps_test/string_requires_in_classpath.cljs"]}, compiling:(/Users/mfikes/Projects/clojurescript/bin/../bin/cljsc.clj:22:1)
	at clojure.lang.Compiler.load(Compiler.java:7441)
	at clojure.lang.Compiler.loadFile(Compiler.java:7367)
	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: clojure.lang.ExceptionInfo: failed compiling file:src/test/cljs/npm_deps_test/string_requires_in_classpath.cljs {:file #object[java.io.File 0x2ef812b "src/test/cljs/npm_deps_test/string_requires_in_classpath.cljs"]}
	at clojure.core$ex_info.invokeStatic(core.clj:4725)
	at clojure.core$ex_info.invoke(core.clj:4725)
	at cljs.compiler$compile_file$fn__4355.invoke(compiler.cljc:1496)
	at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1459)
	at cljs.compiler$compile_file.invoke(compiler.cljc:1435)
	at cljs.closure$compile_file.invokeStatic(closure.clj:532)
	at cljs.closure$compile_file.invoke(closure.clj:523)
	at cljs.closure$eval6554$fn__6555.invoke(closure.clj:601)
	at cljs.closure$eval6490$fn__6491$G__6479__6498.invoke(closure.clj:485)
	at cljs.closure$compile_task$fn__6648.invoke(closure.clj:897)
	at cljs.closure$compile_task.invokeStatic(closure.clj:893)
	at cljs.closure$compile_task.invoke(closure.clj:885)
	at cljs.closure$parallel_compile_sources$fn__6658.invoke(closure.clj:924)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1963)
	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1963)
	at clojure.lang.RestFn.invoke(RestFn.java:425)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:661)
	at clojure.core$bound_fn_STAR_$fn__6752.doInvoke(core.clj:1993)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:748)
Caused by: clojure.lang.ExceptionInfo: No such namespace: lodash/array, could not locate lodash_SLASH_array.cljs, lodash_SLASH_array.cljc, or Closure namespace "lodash/array" in file src/test/cljs/npm_deps_test/string_requires_in_classpath.cljs {:tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4725)
	at clojure.core$ex_info.invoke(core.clj:4725)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:677)
	at cljs.analyzer$error.invoke(analyzer.cljc:673)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:675)
	at cljs.analyzer$error.invoke(analyzer.cljc:673)
	at cljs.analyzer$analyze_deps.invokeStatic(analyzer.cljc:1995)
	at cljs.analyzer$analyze_deps.invoke(analyzer.cljc:1971)
	at cljs.analyzer$ns_side_effects.invokeStatic(analyzer.cljc:3307)
	at cljs.analyzer$ns_side_effects.invoke(analyzer.cljc:3302)
	at cljs.analyzer$analyze_STAR_$fn__2894.invoke(analyzer.cljc:3397)
	at clojure.lang.PersistentVector.reduce(PersistentVector.java:341)
	at clojure.core$reduce.invokeStatic(core.clj:6703)
	at clojure.core$reduce.invoke(core.clj:6686)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3397)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3387)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3421)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3404)
	at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1321)
	at cljs.compiler$emit_source.invoke(compiler.cljc:1301)
	at cljs.compiler$compile_file_STAR_$fn__4331.invoke(compiler.cljc:1402)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1208)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1197)
	at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1391)
	at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1384)
	at cljs.compiler$compile_file$fn__4355.invoke(compiler.cljc:1482)
	... 25 more
ClojureScript compilation failed


 Comments   
Comment by António Nuno Monteiro [ 09/Jul/17 5:01 PM ]

Fix attached.

Comment by Mike Fikes [ 09/Jul/17 5:05 PM ]

António's patch fixes things for me locally.

Comment by David Nolen [ 10/Jul/17 5:32 AM ]

fixed https://github.com/clojure/clojurescript/commit/075136890abc7c143b26a19c35716cfb1d49666f





[CLJS-1513] Javascript emitted for cljs.pprint is misinterpreted under Mobile Safari 7.0 Created: 14/Dec/15  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

Type: Defect Priority: Minor
Reporter: Ruslan Prokopchuk Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

Tested with 1.7.28 and 1.7.170 versions of ClojureScript.

browser = Mobile Safari 7.0
os = iOS 7.1.2



 Description   

Create minimal ClojureScript project, add {{(:require [cljs.pprint :refer [pprint]])}} to the source file and compile with :optimizations :advanced. Output js will contain strings like

~\x3c\x3c-(~;~@{~w~^ ~_~}~;)-\x3c~:\x3e
which lead to the following error in the Mobile Safari 7.0:

Error: Directive "{" is undefined
~<<-(~;~@{~w~^ ~_~}~;)-<~:>
         ^


 Comments   
Comment by David Nolen [ 16/Jun/17 1:15 PM ]

Is this still a problem?

Comment by Oliver George [ 23/Jun/17 10:14 PM ]

The string is still present in the compiled js generated by more recent releases of clojurescript.

I'm unable to recreate the problem on more recent version of Mobile Safari. Tested on 8, 9 and 10.

iOS7 / Mobile Safari 7 is old. Seems you can't even use the XCode Simulator if you run OSX > Yosemite.

Comment by Oliver George [ 09/Jul/17 7:35 PM ]

I think this can be closed as an incomplete and unrepeatable report.

The original report was from my team. We've done our best to recreate the issue and repeat it unsuccessfully.





[CLJS-2205] NPM deps: Correctly compute `:provides` if file ends in `index.js` Created: 09/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 10/Jul/17 5:29 AM ]

fixed https://github.com/clojure/clojurescript/commit/05bab7a6a3531b0474e3f7eed0e9f9699345a760





[CLJS-2203] REPL is turning on all warnings by default (including :invalid-array-access) Created: 09/Jul/17  Updated: 10/Jul/17  Resolved: 10/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: repl

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

 Description   

If you fire up a REPL and evaluate (aget #js {:foo 1} "foo") you will see the aget warning. This is because the REPL initialization code assumes that if the compiler options doesn't include warnings, that it should assume the same as :warnings true.



 Comments   
Comment by David Nolen [ 10/Jul/17 4:57 AM ]

fixed https://github.com/clojure/clojurescript/commit/149724bcb28c44bf331ff96c813c0c3aba287b0f





[CLJS-536] clj->js trims the namespace prefix from keywords while writing them to string Created: 12/Jul/13  Updated: 09/Jul/17  Resolved: 02/Dec/13

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

Type: Defect Priority: Major
Reporter: Vasile Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: clj->js

Attachments: Text File 0001-CLJS-536-Add-support-for-namespaced-keywords.patch     Text File 0001-CLJS-536-Add-support-for-namespaced-keywords.patch    

 Description   

The following behavior was observed and confirmed from the code:

(clj->js :ns/n) => "n"

I believe this is a limitation and the namespace of the keyword should be kept while writing it to string.
The code in core.js does this while handling keywords:

(keyword? x) (name x)

while it should do this (or something similar):

(keyword? x) (str (namespace x) "/" (name x))



 Comments   
Comment by Vasile [ 12/Jul/13 12:03 PM ]

a better (working) fix: (keyword? x) (str (if (namespace x) (str (namespace x) "/")) (name x))

Comment by David Nolen [ 16/Jul/13 6:22 AM ]

I'd be willing to take a patch that allows this behavior to be configured.

Comment by Niklas Närhinen [ 01/Nov/13 7:33 AM ]

Proposed fix

Comment by Niklas Närhinen [ 01/Nov/13 7:37 AM ]

Fixed version of patch

Comment by David Nolen [ 01/Nov/13 9:23 AM ]

Excellent, Niklas I don't see you on the list of contributors, please send in your Contributor Agreement, http://clojure.org/contributing, so we can apply the patch.

Comment by David Nolen [ 02/Dec/13 8:52 PM ]

I looked more closely at the clj->js source, the system is already customizable. We can't determine ahead of time how you might want to emit keywords and symbols - thus you can extend Keyword and Symbol to IEncodeJS yourself and get the desired behavior.

Comment by Andrea Richiardi [ 17/Jan/17 2:52 PM ]

I have just stumbled across this one, shall we at least say it in the docstring of clj->js that we are losing the namespace part?

Comment by Jozef Wagner [ 19/Feb/17 4:11 AM ]

With the introduction of specs, the namespaced keywords are being used more and more. This issue prevents streamlined edn->json->edn transformation. I think it should be reopened. IMO the 'lossy' method should never be a default one.

Comment by Paulus Esterhazy [ 19/Feb/17 5:53 AM ]

Unless we are willing to break existing code, I don't think it will be possible to change the default behavior.

I'm also not sure that extending IEncodeJS is the best solution, as it affects every call to clj->js, including calls to libraries which may rely on the ns-stripping behavior.

However, the attached patch allows you to make the decision on a per-call basis.

One quibble with the patch: perhaps it would be better to use kwargs style `(clj->js v :preserve-namespaces true)` in line with `js->clj`?

Comment by Le Wang [ 13/Jun/17 9:26 AM ]

How would extending IEncodeJS work if I want to only include namespaces sometimes, and don't want to change the default behaviour for all libraries I've included? If this is not possible, it seems this is not a valid solution so this problem.

Comment by Enzzo Cavallo [ 09/Jul/17 8:00 PM ]

I would like to propose a solution
https://gist.github.com/souenzzo/46da88205f90f5ffba0c0a11b8f32119
(First I made a patch, but I saw that the jira does not accept any more attachments, so I published in the gist)
It allows to process keywords with any function
Dont break any existing code
Use named arguments (kwargs style)





[CLJS-2201] Self-host: test-js-filename failing Created: 09/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

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

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

 Description   

With the change made for CLJS-1966, script/test-self-parity started failing with:

FAIL in (test-js-filename) (at cljs/test.js?rel=1499608143677:427:14)
expected: (= "core-advanced-test.js" (ct/js-filename "nW@builds/out-adv/core-advanced-test.js:1191:77"))
  actual: (not (= "core-advanced-test.js" "nW@builds/out-adv/core-advanced-test.js"))


 Comments   
Comment by David Nolen [ 09/Jul/17 3:17 PM ]

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





[CLJS-2202] String requires not working from files in classpath Created: 09/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

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

 Comments   
Comment by David Nolen [ 09/Jul/17 3:13 PM ]

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





[CLJS-2135] require macro prints last result of loaded-libs Created: 28/Jun/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

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

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

 Description   

When using the require macro in the REPL, you see the last result of the loaded-libs emitted JavaScript. Oftentimes this can be an innocuous true, but if you use :reload-all you'll see the loaded libs:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 53584
To quit, type: :cljs/quit
cljs.user=> (require 'clojure.set)
true
cljs.user=> (require 'clojure.set :reload-all)
#{"goog.dom.NodeType" "clojure.set" "goog.array" "cljs.spec" "clojure.walk" "goog.reflect" "goog.asserts" "goog.object" "clojure.string" "goog.math.Long" "cljs.repl" "goog.string" "goog.debug.Error" "cljs.spec.impl.gen" "goog.math.Integer" "cljs.core" "cljs.pprint" "goog.string.StringBuffer"}

If you use `:repl-verbose`, you can see what is happening. The last bit of the above looks like:

goog.require('clojure.set', 'reload-all');
if(!COMPILED) cljs.core._STAR_loaded_libs_STAR_ = cljs.core.into(cljs.core._STAR_loaded_libs_STAR_17863, cljs.core._STAR_loaded_libs_STAR_);

This ticket requests that a null; be emitted so that require returns nil as was done with the require REPL special.



 Comments   
Comment by Mike Fikes [ 28/Jun/17 8:49 AM ]

The attached patch produces the desired result at the REPL (having `require` always return `nil`). It does it in a way that doesn't affect namespaces compiled (that may use top-level `require`) outside of the REPL.

Comment by David Nolen [ 09/Jul/17 2:21 PM ]

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





[CLJS-1428] Add a cljs.core/*command-line-args* var Created: 16/Aug/15  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: core

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

 Description   

Add a cljs.core/*command-line-args* var that mimics Clojure's:

https://github.com/clojure/clojure/blob/4bb1dbd596f032621c00a670b1609a94acfcfcab/src/clj/clojure/core.clj#L6148

Rationale:
1) Simplifies writing command-line scripts in ClojureScript if this var is universally available.
2) Consistency with Clojure.

Existing tooling can ignore the var (it would be initialized with nil).

Command-line REPLs or other command-line environments can bind a sequence to this var when launched.



 Comments   
Comment by Justin Thomas [ 31/Aug/15 10:14 PM ]

In this file: https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/nodejs.cljs Looks like you could set this: (def *command-line-args* (.-argv process)) and it'd be accessible under nodejs/*command-line-args*. Not sure where the star vars are set in the elsewhere in the code or if that's a good solution putting it in each repo.

Maybe get the target var somehow and set it based on that--similar to how the nodejs file is loaded?

Found this: src/main/clojure/cljs/core.cljc and saw references to a ns called env.

Comment by David Nolen [ 01/Sep/15 6:44 AM ]

This ticket needs more rationale and stronger outline of the design issues before proceeding. As it stands no one should be working on this yet.

Comment by Justin Thomas [ 01/Sep/15 4:27 PM ]

Just trying to learn the code at the moment. As for rationale, using the command line vars in node is a common use case for shell scripts. Accessing the command line vars in a more clojure-y way seems like a good idea.

Comment by David Nolen [ 08/Jul/17 10:47 AM ]

I agree that in a post bootstrap world this sounds pretty useful.

Comment by António Nuno Monteiro [ 08/Jul/17 3:24 PM ]

Attached a simple patch just adding the variable to the `cljs.core` namespace, that bootstrap REPLs can set.

I think that providing a default behavior warrants more discussion that falls out of the scope of this ticket.

Comment by David Nolen [ 09/Jul/17 12:23 PM ]

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





[CLJS-2199] String requires broken after recompile Created: 09/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Juho Teperi Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, npm-deps

Approval: Vetted

 Description   

String requires uses js-module-exists? to check if module needs to loaded: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L722

It checks the compiler state for :js-module-index.

The first time e.g. react-dom/server is not found.

If using the same compiler state for recompile, the second time the module is already present in compiler-state.

This means that the module is not indexed into foreign-libs: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/closure.clj#L2043

Result is that cljs_deps.js won't have the line for react-dom/sever or No such namespace exception about react-dom$server.



 Comments   
Comment by David Nolen [ 09/Jul/17 12:21 PM ]

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





[CLJS-2192] Add ChakraCore testing facilities Created: 08/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

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

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

 Description   

It is trivial to get pre-built ChakraCore binaries. This ticket asks that support for ChakraCore be added to script/test and others so that it is easy for users to also test with this engine if desired. The amount of time needed to run the tests is not significantly more than the other fast engines.



 Comments   
Comment by Mike Fikes [ 08/Jul/17 4:24 PM ]

Attached patch adds ChakraCore to the test scripts, including (and tested on) Windows.
The tests pass on macOS and Linux, but fail on Windows in a 3 spots (will deal with those few failures via separate ticket or tickets).
Also adds ChakraCore to CI (it passes there).

Comment by David Nolen [ 09/Jul/17 8:12 AM ]

The patch needs to be rebased to master.

Comment by Mike Fikes [ 09/Jul/17 9:21 AM ]

Attached re-baselined CLJS-2192-v2.patch.

Comment by David Nolen [ 09/Jul/17 11:59 AM ]

fixed https://github.com/clojure/clojurescript/commit/6583de880ee6bf92e0baa50dee8da621e45494e1





[CLJS-2172] memfn docstring refers to Java and reflection Created: 05/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File CLJS-2172.patch     Text File CLJS-2172-v2.patch    
Patch: Code and Test
Approval: Screened

 Description   

Currently:

Expands into code that creates a fn that expects to be passed an
object and any args and calls the named instance method on the
object passing the args. Use when you want to treat a Java method as
a first-class fn. name may be type-hinted with the method receiver's
type in order to avoid reflective calls.

Proposed:

Expands into code that creates a fn that expects to be passed an
object and any args and calls the named instance method on the
object passing the args. Use when you want to treat a JavaScript 
method as a first-class fn.


 Comments   
Comment by Mike Fikes [ 05/Jul/17 8:45 AM ]

Note: Even though this ticket was marked as "trivial" owing to it being simply a docstring change, the solution in CLJS-2172.patch is not trivial in that it involves exclusively using the ClojureScript-specific copy of memfn that was originally created for self-hosted. Since this is a bit more extensive of a change, added unit tests. Also, a small consequence is that doc on memfn now shows the macro symbol name as cljs.core/memfn (as it already does in self-hosted REPLs that implement doc).

I suppose, if this change is accepted, it opens the door to potentially remove the with-meta that is in the implementation, which came from the Clojure copy (if there is really no need for it in ClojureScript).

Comment by David Nolen [ 09/Jul/17 8:19 AM ]

Patch needs to be rebased to master.

Comment by Mike Fikes [ 09/Jul/17 9:12 AM ]

Attached re-baselined CLJS-2172-v2.patch.

Comment by David Nolen [ 09/Jul/17 9:57 AM ]

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





[CLJS-1959] under :nodejs target we should provide __dirname and __filename constants Created: 27/Feb/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: nodejs

Attachments: Text File CLJS-1959.patch    
Patch: Code
Approval: Vetted

 Comments   
Comment by David Nolen [ 08/Jul/17 9:22 AM ]

The current patch does not work. Also the patch must supply a test to be accepted.

Comment by David Nolen [ 09/Jul/17 8:52 AM ]

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





[CLJS-1702] Warning when using private vars Created: 07/Jul/16  Updated: 09/Jul/17

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

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: warnings

Approval: Vetted

 Description   

Currently no warning or error of any kind is given. Throwing an error and forcing users to go through vars is somewhat less attractive since vars dump information like file, line, etc. A warning would be a simple way to flag users that they are treading into dangerous territory. Downstream tooling error handling can make it a hard error if they like.






[CLJS-1966] cljs.test assumes the output directory is '/out/' when determining the filename for a failed or errored test result. Created: 06/Mar/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Critical
Reporter: Creighton Kirkendall Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: cljs.test

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

 Description   

If your output directory is does not contain '/out/' in the path the code for getting the filename on failure for test results will return nonsense.

You can see here where we we make the assumption that '/out/' is used.
https://github.com/clojure/clojurescript/blob/2f8a1529955acc943ac8082ab5848b2cba54bc4d/src/main/cljs/cljs/test.cljs#L379



 Comments   
Comment by António Nuno Monteiro [ 08/Jul/17 4:04 PM ]

Patch attached.

Comment by David Nolen [ 09/Jul/17 8:25 AM ]

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





[CLJS-2194] cljs.util/relative-name bug Created: 08/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

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

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

 Description   

cljs.util/relative-name is still not quite right for paths that end with trailing slashes



 Comments   
Comment by David Nolen [ 09/Jul/17 8:09 AM ]

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





[CLJS-2197] Calling instrumented var fails to check conformance Created: 09/Jul/17  Updated: 09/Jul/17

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

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


 Description   

If you spec a fn var and then instrument and call it, an args conformance check is made. If instead you call the var using the actual var in operator position, it will fail to make the conformance check. The same will occur with a HOF, as in applying the fn to an args sequence. These last two variants work in Clojure.

Repro:

$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 50246
To quit, type: :cljs/quit
cljs.user=> (require '[clojure.spec.alpha :as s] '[clojure.spec.test.alpha :as st])
true
cljs.user=>   (s/fdef clojure.core/symbol
    :args (s/alt :separate (s/cat :ns string? :n string?)
                 :str string?
                 :sym symbol?)
    :ret symbol?)
cljs.core/symbol
cljs.user=> (st/instrument)
[cljs.core/symbol]
cljs.user=> (symbol 3)
repl:13
throw e__5614__auto__;
^

Error: Call to #'cljs.core/symbol did not conform to spec:
In: [0] val: 3 fails at: [:args :separate :ns] predicate: string?
In: [0] val: 3 fails at: [:args :str] predicate: string?
In: [0] val: 3 fails at: [:args :sym] predicate: symbol?
:cljs.spec.alpha/spec  #object[cljs.spec.alpha.t_cljs$spec$alpha2801]
:cljs.spec.alpha/value  (3)
:cljs.spec.alpha/args  (3)
:cljs.spec.alpha/failure  :instrument

    at new cljs$core$ExceptionInfo (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:34869:10)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$3 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:34930:9)
    at Function.cljs.core.ex_info.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:34926:26)
    at cljs$core$ex_info (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:34912:26)
    at /Users/mfikes/Downloads/.cljs_node_repl/cljs/spec/test/alpha.js:133:25
    at G__3555__delegate (/Users/mfikes/Downloads/.cljs_node_repl/cljs/spec/test/alpha.js:164:15)
    at G__3555 (/Users/mfikes/Downloads/.cljs_node_repl/cljs/spec/test/alpha.js:185:26)
    at repl:1:96
    at repl:9:3
    at repl:14:4
cljs.user=> (#'symbol 3)
repl:13
throw e__5614__auto__;
^

TypeError: name.indexOf is not a function
    at Function.cljs.core.symbol.cljs$core$IFn$_invoke$arity$1 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3531:16)
    at cljs.core.Var.G__8901__2 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3627:65)
    at cljs.core.Var.G__8901 [as call] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3773:19)
    at repl:1:2682
    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=> (apply symbol [3])
repl:13
throw e__5614__auto__;
^

TypeError: name.indexOf is not a function
    at Function.cljs.core.symbol.cljs$core$IFn$_invoke$arity$1 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:3531:16)
    at Object.cljs$core$apply_to [as apply_to] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:12421:45)
    at Function.cljs.core.apply.cljs$core$IFn$_invoke$arity$2 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:12860:18)
    at cljs$core$apply (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:12818:24)
    at repl:1:95
    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)


 Comments   
Comment by Mike Fikes [ 09/Jul/17 8:04 AM ]

It is worth noting that the second variant, (#'symbol 3) works in Planck (but not the third) (apply symbol [3]).





[CLJS-2195] npm-deps tests are not idempotent Created: 08/Jul/17  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 09/Jul/17 8:02 AM ]

fixed https://github.com/clojure/clojurescript/commit/341b00419cab487234909fd15476264353ba6bda





[CLJS-1706] cljs.reader support for namespaced map literal Created: 15/Jul/16  Updated: 09/Jul/17  Resolved: 09/Jul/17

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

Type: Defect Priority: Critical
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 4
Labels: None


 Description   

Clojure 1.9 extends support for namespaced maps and cljs.reader needs to be able to read them.

#:example{:key "value"} currently results in a Could not find tag parser for :example in ... error.



 Comments   
Comment by Thomas Heller [ 17/Jul/16 3:20 AM ]

I wanted to start implementing this and started looking at the "spec" [1]. It mentions support for #:: and #::alias, but since cljs.reader does not have access to aliases (or the current ns) I do not know how to proceed. Should it just throw then encountering #:: since it isn't really all that useful in a EDN context?

http://dev.clojure.org/jira/browse/CLJ-1910

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

What does the Clojure EDN reader do here?

Comment by Linus Ericsson [ 11/Aug/16 2:26 PM ]

The clojure.edn-reader seems to catch that it doesn't have a default namespace, nor any aliases.

user> (clojure.edn/read-string "#:a {:b 1}")
{:a/b 1}

user> (clojure.edn/read-string "#::a {:b 1}")
RuntimeException Namespaced map must specify a valid namespace: :a  clojure.lang.EdnReader$NamespaceMapReader.invoke (EdnReader.java:494)

user> (clojure.edn/read-string "#:: {:b 1}")
RuntimeException Invalid token: :  clojure.lang.Util.runtimeException (Util.java:221)

This seems to be sane defaults also for cljs.reader.

Also see the commit adding namespaces maps in clojure

Comment by David Nolen [ 08/Jul/17 10:06 AM ]

related CLJS-1800, CLJS-2059

Comment by António Nuno Monteiro [ 08/Jul/17 3:10 PM ]

this will be fixed by https://github.com/clojure/clojurescript/pull/69 (CLJS-1800)

Comment by David Nolen [ 09/Jul/17 7:56 AM ]

fixed https://dev.clojure.org/jira/browse/CLJS-1800





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

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

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

Win 7 64bit


Approval: Vetted

 Description   

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

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

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

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

THIS WORKS:

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

BUT THIS DOESN'T:

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

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

Thanks for help!



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

Patch must supply a test case.

Comment by António Nuno Monteiro [ 08/Jul/17 3:13 PM ]

Not sure if this is a bug. Running this at a Clojure REPL produces the same results:

user=> (defrecord MyRecord [value])
user.MyRecord
user=> (require '[clojure.pprint :as pprint])
nil
user=> (pprint/pprint (MyRecord. "a"))
{:value "a"}
nil
Comment by Miroslav Kubicek [ 09/Jul/17 12:23 AM ]

Good catch, that is indeed interesting... but I'd still argue that this is definitely a bug (which just seems to be present in clj as well) - pretty print should work in the same way as print does, only prettier (aka more readable). It should not have its own idiosyncrasies. Every other print function (even including spit) works this way, so I can not imagine what would be the reason (or a use case) for pprint not to honor tagged literals specification of clojure and print things its own way.





[CLJS-2196] SpiderMonkey path needs quoting in test scripts Created: 08/Jul/17  Updated: 08/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: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

The paths to the other engines (V8, Nashorn, etc.), are quoted, allowing paths with spaces. This needs to also be done for SpiderMonkey.






[CLJS-2179] Add test for preprocess JS module as symbol Created: 05/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Task Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, newbie

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

 Comments   
Comment by António Nuno Monteiro [ 08/Jul/17 2:21 PM ]

Patch with tests attached.

Comment by David Nolen [ 08/Jul/17 4:55 PM ]

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





[CLJS-2152] "is not a relative path" exception thrown when `:libs` directory is provided. Created: 02/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Bruce Hauman Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: build
Environment:

OSX


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

 Description   

This problem shows up in this simple case:

In the compiler config set :libs to ["js_libs"]

Create a GCL file in "js_libs"

goog.provide("tabby");

tabby.hello = function() {return "hello there from tabby";};

In the main cljs file require the GCL lib:

(ns example.core
  (:require [tabby]))

This should produce the following stacktrace:

java.lang.IllegalArgumentException: /tabby.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:1658)
    cljs.closure$write_javascript.invoke (closure.clj:1651)
    cljs.closure$source_on_disk.invokeStatic (closure.clj:1697)
    cljs.closure$source_on_disk.invoke (closure.clj:1692)
    cljs.closure$output_unoptimized$fn__13956.invoke (closure.clj:1735)
    clojure.core$map$fn__4785.invoke (core.clj:2646)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.RT.seq (RT.java:521)
    clojure.core$seq__4357.invokeStatic (core.clj:137)
    clojure.core$filter$fn__4812.invoke (core.clj:2700)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.RT.seq (RT.java:521)
    clojure.core$seq__4357.invokeStatic (core.clj:137)
    clojure.core$map$fn__4785.invoke (core.clj:2637)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.next (RT.java:688)
    clojure.core$next__4341.invokeStatic (core.clj:64)
    clojure.core$str$fn__4419.invoke (core.clj:546)
    clojure.core$str.invokeStatic (core.clj:544)
    clojure.core$str.doInvoke (core.clj:533)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invokeStatic (core.clj:646)
    clojure.core$apply.invoke (core.clj:641)
    cljs.closure$deps_file.invokeStatic (closure.clj:1433)
    cljs.closure$deps_file.invoke (closure.clj:1430)
    cljs.closure$output_deps_file.invokeStatic (closure.clj:1453)
    cljs.closure$output_deps_file.invoke (closure.clj:1452)
    cljs.closure$output_unoptimized.invokeStatic (closure.clj:1743)
    cljs.closure$output_unoptimized.doInvoke (closure.clj:1726)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invokeStatic (core.clj:648)
    clojure.core$apply.invoke (core.clj:641)
    cljs.closure$build.invokeStatic (closure.clj:2352)
    cljs.closure$build.invoke (closure.clj:2236)
    cljs.build.api$build.invokeStatic (api.clj:202)
    cljs.build.api$build.invoke (api.clj:189)


 Comments   
Comment by Francis Avila [ 02/Jul/17 8:35 PM ]

I ran in to this issue on a large, complex project which I was upgrading from 1.8.51 to a few commits short of 1.9.655. However, I never created a minimal case. Some more information I can add:

  1. This is a regression since 1.8.51.
  2. I experienced this on a project where :libs and :src were the same. (Cljs and goog-compatible js were intermingled in same source root.) I thought that might have been relevant but it looks like it is not.
  3. The length of the goog-provided namespace does not matter. (One segment and many-segment both fail.)
  4. A workaround is to include the full path to the goog file in :libs. For example, {{:libs ["js_libs/tabby.js"] }} will not fail. This is the workaround I used in my project.
Comment by David Nolen [ 08/Jul/17 9:21 AM ]

The patch must supply a test.

Comment by António Nuno Monteiro [ 08/Jul/17 2:14 PM ]

Patch attached.

Comment by Martin Klepsch [ 08/Jul/17 3:44 PM ]

Tested the patch with cljsjs/incremental-dom. Working as advertised 👍

FWIW, in contrast to Francis' observations I thought this only affected :libs with single segment goog.provide names. After stumbling into this with incremental-dom I tried to reproduce it with openlayers but I couldn't reproduce the issue there.

Comment by David Nolen [ 08/Jul/17 4:48 PM ]

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





[CLJS-2059] Printing a namespaced qualified map and reading it back to ClojureScript fails Created: 29/May/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Blocker
Reporter: Michiel Borkent Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None


 Description   

Example:

`(cljs.reader/read-string (pr-str {:foo/id "foo" :foo/bar "bar"}))`

fails with

#object[Error Error: Could not find tag parser for :foo in ("inst" "uuid" "queue" "js")]
Error: Could not find tag parser for :foo in ("inst" "uuid" "queue" "js")

This fails in 1.9.542 but I could not choose that version from the dropdown.



 Comments   
Comment by David Nolen [ 16/Jun/17 9:11 AM ]

Related to CLJS-1800

Comment by António Nuno Monteiro [ 08/Jul/17 2:24 PM ]

https://github.com/clojure/clojurescript/pull/69 (CLJS-1800) fixes this one





[CLJS-1800] Defer to tools.reader for cljs.reader functionality Created: 30/Sep/16  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

Approval: Vetted

 Description   

We already have a dependency on tools.reader and maintaining our own EDN reader is just an unnecessary burden.



 Comments   
Comment by Aleš Roubíček [ 02/Oct/16 1:02 AM ]

I'm fiddling with this and found two differences in reading (according to cljs.reader-tests):

  1. cljs.tools.reader reads the @ macro as 'clojure.core/deref instead of 'deref
  2. cljs.tools.reader does not read small maps as PersistentArrayMap

Shoud this be solved by patching cljs.tools.reader or by changing cljs.reader-tests?

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

Questions should be asked about 2). 1) seems fine.

Comment by Rohit Aggarwal [ 05/Oct/16 4:11 AM ]

I think fixing cljs.tools.reader/read-map to return a PersistentArrayMap or a PersistentHashMap is a relatively easy fix and should be raised in TRDR bug tracker.

Comment by Aleš Roubíček [ 26/Oct/16 7:04 AM ]

I did the patch for cljs.tools.reader http://dev.clojure.org/jira/browse/TRDR-40

Comment by David Nolen [ 08/Jul/17 4:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/07ee2250af02b25f232111890c0f40f23150768d





[CLJS-2193] :npm-deps dependencies are implicit Created: 08/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Blocker
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: npm-deps

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

 Description   

Currently the Node dependencies for `:npm-deps` to work are implicit. Looking `cljs/module_deps.js` it's clear that at least `module-deps`, `resolve`, and `browserResolve` must exist. This should probably be handled transparently for the user.



 Comments   
Comment by David Nolen [ 08/Jul/17 9:52 AM ]

On my local machine at least I had to install these manually. I think this should probably be handled for the user?

Comment by David Nolen [ 08/Jul/17 12:09 PM ]

Even after manually installing all the necessary deps if I try to run just the `test-npm-deps` test by itself on master I get the following stacktrace from Node.js:

module.js:471
    throw err;
    ^

Error: Cannot find module 'resolve'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at [eval]:3:19
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:570:32)
    at evalScript (bootstrap_node.js:353:27)
Comment by David Nolen [ 08/Jul/17 2:08 PM ]

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





[CLJS-450] (ns) within (do) inconsistent with Clojure behaviour Created: 27/Dec/12  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Trivial
Reporter: Stuart Campbell Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

An (ns) within a (do) doesn't quite work as expected at the REPL:

ClojureScript:cljs.user> (do (ns foo) (def x 42))   
nil
ClojureScript:foo> x
nil
ClojureScript:cljs.user> cljs.user/x
42

The Clojure equivalent:

user=> (do (ns foo) (def x 42))
#'foo/x
foo=> x
42


 Comments   
Comment by David Nolen [ 05/Jan/13 2:05 PM ]

Looks like we need to do something similar to what is done in Clojure with top level do - http://github.com/frenchy64/typed-clojure/pull/4

Comment by David Nolen [ 08/Jul/17 11:00 AM ]

far as I know this particular issue is now fixed at the REPL.





[CLJS-1644] Recursive protocol method for a record never terminates Created: 23/May/16  Updated: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: Brian Stiles Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Any



 Description   

When extending a protocol for a record, a recursive method can never terminate in some cases.

(defrecord R [a])

(defprotocol P
  (f [x]))

(extend-protocol P
  R
  (f [x]
    (if x
      (recur nil)
      x))
  default
  (f [x]
    (if x
      (recur nil)
      x)))

(prn (f 1))        ; #1
(prn (f (R. 1)))   ; #2

prn call #1 prints nil as expected, but prn call #2 never terminates.

It looks like the compiler creates a variable assigned to this within the while loop such that the test of "x" is always really testing this when it should be testing the value of x passed in by the call to recur.

Note, I'm testing ClojureScript 1.8.51. The "Affects Version/s" field above only gives 1.7.228 as the most recent version.



 Comments   
Comment by Brian Stiles [ 23/May/16 4:14 AM ]

Actually, "always really testing 'this' when it should be testing the value of x passed in by the call to recur" is only true if the type of the value represented by x remains the same. If, as in the example, the type changes, the call should be dispatched to the properly matching method implementation.

This seems to behave as expected in Clojure.

Comment by David Nolen [ 08/Jul/17 10:58 AM ]

Confirmed with Mike Fikes this is still an issue on master even with the recur enhancements.





[CLJS-1076] :nashorn target Created: 02/Mar/15  Updated: 08/Jul/17

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

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


 Description   

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






[CLJS-2129] extend-type allow target object to change Created: 26/Jun/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

Approval: Accepted

 Description   

In the following code

(defprotocol ISearch
  (search [this coll]))

(defrecord Search [item])

(def s2 (->Search :hello))


(extend-type Search
  ISearch
  (search [this coll]
    (when (seq coll)
      (if (= (:item this) (first coll))
        (:item this)
        (recur s2 (rest coll))))))

(-> (->Search 1)
  (search [:a :hello "b"]))

Note that the first argument to recur is s2, changing the target object when recurring.

In Clojure, this is allowed and the last form yields :hello.

In ClojureScript, the use of this-as in the implementation defeats this.

Note that, for deftype, defrecord, and reify, the use of this-as is currently needed because we pass in a dummy nil as the target object https://github.com/clojure/clojurescript/blob/69342169ba868574aa7f5a88669e6333ccc3df01/src/main/clojure/cljs/analyzer.cljc#L1818



 Comments   
Comment by Mike Fikes [ 08/Jul/17 10:49 AM ]

Duplicate of CLJS-1644





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

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

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


 Description   

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

To reproduce, evaluate the following forms in a REPL:

(require 'cljs.js)

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

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

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

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

Instead, you get the following in the eval callback:

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

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

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



 Comments   
Comment by David Nolen [ 08/Jul/17 10:50 AM ]

I'm not inclined to say that throwing an error on this is misfeature. We should just warn.





[CLJS-2168] Properly document browser-env options Created: 04/Jul/17  Updated: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: Timothy Pote Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: browser-repl

Attachments: Text File 001-CLJS-2168.patch    
Patch: Code
Approval: Screened

 Description   

There are a number of browser-env options that work only partially or not at all:

  • :optimizations - Only :whitespace and :simple appear to work for me
  • :host - Is never read. Instead, we always bind to 0.0.0.0.
  • :serve-static - Is never read.
  • :preloaded-libs - Is never read.

These should either be properly documented, removed, or made to work.



 Comments   
Comment by Timothy Pote [ 04/Jul/17 4:55 PM ]

I think we should:

  • Properly document :optimizations and throw exception on an unsupported option
  • Make :host function properly
  • Remove :serve-static as an option
  • Remove :preloaded-libs as an option
Comment by Timothy Pote [ 04/Jul/17 4:57 PM ]

Note that, as it stands, this also addresses CLJS-1502.

Comment by Timothy Pote [ 04/Jul/17 5:51 PM ]

This does what I said in my initial comment.

Note that this commit is rather small if you exclude whitespace.

Comment by Timothy Pote [ 05/Jul/17 8:43 AM ]

After thinking about it some more, I'm not sure what the gain is for being able to specify :optimizations in the browser-env, and the cost is confusion on the part of the users. I do not think it's apparent that this is a compiler option for the child iframe JS and evaluated repl forms. This may be a case where we can just "do the right thing" and remove some burden from the user.

I'm thinking we either remove :optimizations entirely or we only use it for evaluated repl forms and use :simple for the initial payload. Considering the user can already override this in the arguments to cljs.repl/repl, I lean toward removing it altogether.

Comment by David Nolen [ 08/Jul/17 10:48 AM ]

Related CLJS-1502





[CLJS-1502] Browser REPL broken when started with :optimizations :none Created: 05/Dec/15  Updated: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: ewen grosjean Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Creating a browser-repl env like this: (cljs.repl.browser/repl-env :optimizations :none) does not work because client.js is not compiled in a single file. The browser throws the following error: goog is not defined.
:optimizations :simple (the default) works fine.
A quick fix would be to force :optimizations :simple in the REPL options. However, being able to set :optimizations :none would probably speed up compilation times.



 Comments   
Comment by David Nolen [ 08/Jul/17 10:48 AM ]

Related CLJS-2168





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

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

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

Windows 10


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

 Description   

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



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

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

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

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

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

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

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

Needs review

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

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

Comment by David Nolen [ 08/Jul/17 10:44 AM ]

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





[CLJS-2163] Clean up uses of aget / aset on objects Created: 04/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: core

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

 Description   

With CLJS-2148 new warnings were added which emit diagnostics when inference indicates invalid aget or aset usage, and the usages were also addressed.

This ticket asks that other uses of aget and aset be fixed (where the use site does not have sufficient inference to trigger the warnings).



 Comments   
Comment by Mike Fikes [ 05/Jul/17 3:51 PM ]

For the two changes in ObjMap, I used unsafe-get, in case efficiency matters. (It looks like this deftype is deprecated, so I didn't bother to add tests for it)

For the change in js-invoke, I used unsafe-get in case efficiency matters. I added a unit test for js-invoke.

For the two changes in source-map, I used unsafe-get thinking that efficiency might really matter here. I didn't add any unit tests for these changes.

I ran a browser REPL (QuickStart), script/noderepljs, the Nashorn, and Rhino REPLs to ensure they didn't break. Interestingly, the Node REPL needed unsafe-get, otherwise you get "Cannot read property 'get' of undefined" for goog.object.get. Presumably this is too early in startup to use goog.object.

The changes in js-obj appear to already have coverage in the test suite. I also manually tested at the REPL.

Comment by Mike Fikes [ 07/Jul/17 10:43 PM ]

Attaching a revised {{CLJS-2163-2.patch}} that patches a few more missed uses in the ObjMap implementation and two important ones:

  1. The printing of JavaScript objects in pr-writer-impl
  2. The js->clj implementation

The two above use unsafe-get in the patch, presuming they need to be extra performant.

Comment by David Nolen [ 08/Jul/17 10:20 AM ]

This looks good but the changes to source map are unnecessary `sources` and `name` are in fact JavaScript arrays. Otherwise looks good.

Comment by David Nolen [ 08/Jul/17 10:43 AM ]

Actually just going to fix the source map changes myself.

Comment by David Nolen [ 08/Jul/17 10:44 AM ]

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





[CLJS-1574] CLJS string equivalence is very slow in Chrome Created: 16/Feb/16  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: Stephen Nelson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: performance
Environment:

Google Chrome 48.0.2564.109 on Mac OS X 10.11.3
Safari 9.0.3 (11601.4.4) on Mac OS X 10.11.3



 Description   

Clojurescript's equivalence for strings in Google Chrome is ~1000 times slower than equivalent javascript functionality, and ~1000 times slower than the same function in Safari.

Google Chrome
js equiv:  0.005 seconds
cljs equiv:  1.898 seconds
Safari
js equiv:  0.005 seconds
cljs equiv:  0.006 seconds
(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(deftest test-js-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (js* "~{} == ~{}" str1 str2)
                  "js equivalence")
        end   (now)
        ]
    (println "js equiv: " (delta end start))))

(deftest test-cljs-eq-perf
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (is (= str1 str2)
                  "cljs equivalence")
        end   (now)
        ]
    (println "cljs equiv: " (delta end start))))


 Comments   
Comment by Stephen Nelson [ 16/Feb/16 6:06 PM ]

This bug only occurs when cljs.pprint has been required.

Comment by Stephen Nelson [ 16/Feb/16 6:38 PM ]

After a whole lot of binary search, here's a minimal reproduction. When cljs.pprint is loaded it constructs write-option-table. It seems that constructing a hash map with the keys :added and :ns causes a call to (= :added :ns), which is sufficient to cause string equality to become extremely slow.

(ns hello-world.core)

(enable-console-print!)

(def size (* 128 1024))

(defn now []
  (.getTime (js/Date.)))

(defn delta [b a]
  (str (/ (- b a) 1000) " seconds"))

(defn test [] 
  (let [str1  (apply str (repeat size "a"))
        str2  (apply str (repeat size "a"))
        start (now)
        _     (= str1 str2)
        end   (now)
        ]
      (println "cljs equiv: " (delta end start))))

(test)

(= :added :ns)

(test)
Comment by Peter Schuck [ 17/Feb/16 4:50 PM ]

Is the ClojureScript compiled with options :optimizations :advanced or :static-fns true? Compiling ClojureScript without those options results in call indirection for all function calls which might explain the slowdown. See http://swannodette.github.io/2015/03/16/optimizing-clojurescript-function-invocation/ for more information.

Comment by Stephen Nelson [ 17/Feb/16 9:06 PM ]

This happens with :advanced, :simple, and without optimisations. Stepping through the generated javascript seems to indicated that the slow down comes from the VM's internal function dispatch. Regardless, I don't think that the extra function calls related to dynamic dispatch in clojurescript could add minutes of overhead per call. Note that the test case above only uses 128k of string data, the case where I encountered this issue first used ~512K and took about 5 minutes to complete a single function call.

Comment by Francis Avila [ 17/Feb/16 9:14 PM ]

I have reproduced this in Chrome for Linux, on :none and :advanced optimization levels using different test code. I verified the result of the compare so the JIT won't optimize it away and I used performance.mark() and performance.measure() for timing, although none of this should have mattered.

Every subsequent string compare after the first -equiv-invoking use of equal is significantly slower for no reason I can see. There are no intermediate GCs or anything to suggest that it should be slower--it just takes longer! The only thing I can think of is maybe the keyword-equals triggers a deopt because it makes the equal-function megamorphic, but the code is run so few times that there should not be jit optimizations kicking it at all. Also, the keyword-compare itself remains fast.

I suspect a Chrome/v8 bug. Possibly a different internal string representation kicks in for some reason which has a slower compare? This is only an issue for largish, non-constant strings, and the slowdown is proportional to string size. I'm going to try and reproduce this with pure JS.

Comment by Francis Avila [ 18/Feb/16 12:33 AM ]

All you need to reproduce this is to use the strict equality operator in a function body non-monomorphically. Subsequent executions of the function with strings (at least) which have not been compared before the polymorphic call will be very slow.

If you replace strict equality (triple-equal) with normal equality (double-equal), this issue goes away.

This is clearly a Chrome/v8 bug, but I'm not sure where to report it.

Minimal pure-javascript reproduction:

function str(size) {
  var s = "";
  for (var i = 0; i < size; i++) s += "a";
  return s;
}

function eq(x, y) {
  performance.mark("start");
  x === y; // No slowdown if use == instead
  performance.mark("end");
}

function print_measures() {
  performance.getEntriesByType("measure")
  .forEach(entry => console.log(entry.name, entry.duration));
}

var s1 = str(64 * 1024);
var s2 = str(64 * 1024);
var s3 = str(64 * 1024);

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(0, 0);
performance.measure("eq(0, 0)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

eq(s1, s2);
performance.measure("eq(s1, s2)", "start", "end");

eq(s1, s3);
performance.measure("eq(s1, s3)", "start", "end");

print_measures();

Results with Chrome 48.0.2564.109 (64-bit) on a slow iMac with OS X 10.11.3

eq(s1, s2)   4.465000000000003     // fast string compare
eq(0, 0)     0.009999999999990905  // break monomorphism of eq()
eq(s1, s3) 259.665                 // Now string compare is slow
eq(s1, s2)   0.019999999999924967  // Repeated call still fast
eq(s1, s3) 232.52499999999998      // ... but not from after the polymorphic invoke
Comment by Francis Avila [ 22/Feb/16 3:14 AM ]

Issue added to v8: https://bugs.chromium.org/p/v8/issues/detail?id=4773

Comment by Francis Avila [ 03/Jan/17 9:23 AM ]

Update: This bug was fixed upstream today, so it should start showing up in releases eventually.

Comment by David Nolen [ 08/Jul/17 10:39 AM ]

We're not going to work around VM issues like this.





[CLJS-1678] variadic defn can be called for missing fixed arities, overlapping arity Created: 11/Jun/16  Updated: 08/Jul/17

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core


 Description   

For defns with a variadic arity: if invoked with a missing fixed arity, they use the variadic method instead of erroring; if invoked with a fixed arity that is the max fixed arity, variadic mathod instead of fixed form is invoked.

(defn f-hole
  ([a] 1)
  ([a b c d & args] "4 or more"))

(f-hole 1 2) ; =>"4 or more", should be error

(defn f-overlap-mfa
  ([a b] 2)
  ([a b & c] "2+"))

(f-overlap-mfa 1) ;=> "2+", should be error
(f-overlap-mfa 1 2) ;=> "2+", should be 2
(f-overlap-mfa 1 2 3) ;=> "2+", correct

A way to fix the f-hole bug is to emit a "case X:" into the switch statement for all X with no signature or less than max-fixed-arity.

The f-overlap-mfa I'm not sure why is happening and didn't investigate deeply.



 Comments   
Comment by Francis Avila [ 11/Jun/16 8:31 AM ]

Sorry, filed against CLJ instead of CLJS!

Comment by Rohit Aggarwal [ 12/Jun/16 9:41 AM ]

The behaviour I am seeing for f-overlap-mfa is:

(f-overlap-mfa 1) ;=> "2+"
(f-overlap-mfa 1 2) ;=> 2
(f-overlap-mfa 1 2 3) ;=> "2+"

So the two argument result is different for me than you, Francis Avila.

The call with just one argument does give a warning though:

WARNING: Wrong number of args (1) passed to cljs.user/f-overlap-mfa





[CLJS-1846] Range issues Created: 10/Nov/16  Updated: 08/Jul/17

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

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core

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

 Description   

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

Examples

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

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

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

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


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

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

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

Updated patch with performance tweaks.

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




[CLJS-2140] Add :simple tests to CI Created: 30/Jun/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

Attachments: Text File CLJS-2140.patch     Text File CLJS-2140-v2.patch    
Patch: Code

 Description   

CI is currently running only :advanced tests. We can also run :simple, copying behavior of script/test-simple.



 Comments   
Comment by Mike Fikes [ 30/Jun/17 7:59 AM ]

Attaching a v2 patch which adds a comment explaining why JSC is commented out.

Comment by David Nolen [ 08/Jul/17 10:22 AM ]

Doesn't seem that useful for CI. `:simple` is really just for local debugging.





[CLJS-2150] Expose simple webserver Created: 02/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

Approval: Screened

 Description   

Currently users cannot easily start a simple webserver to test ClojureScript. We ship a simplistic webserver in ClojureScript but this is tied to the browser REPL. We should support starting the webserver sans REPL.



 Comments   
Comment by David Nolen [ 08/Jul/17 10:21 AM ]

cljs.repl.browser is sufficient for now. The logging has been tweaked to make it more clear that it's useful for this purpose.





[CLJS-2189] Add test for :preloads Created: 07/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 08/Jul/17 9:59 AM ]

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





[CLJS-2190] Interaction with *print-namespace-maps* and js objects with slash in property name Created: 07/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

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

 Description   
$ java -jar cljs.jar -m cljs.repl.node
ClojureScript Node.js REPL server listening on 57795
To quit, type: :cljs/quit
cljs.user=> (set! *print-namespace-maps* false)
false
cljs.user=> (doto (js-obj) (goog.object/set "foo/bar" 33))
#js {:foo/bar 33}
cljs.user=> (set! *print-namespace-maps* true)
true
cljs.user=> (doto (js-obj) (goog.object/set "foo/bar" 33))
repl:17
throw e__5614__auto__;
^

Error: No protocol method IAssociative.-assoc defined for type cljs.core/EmptyList: ()
    at Object.cljs$core$missing_protocol [as missing_protocol] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:317:9)
    at Object.cljs$core$_assoc [as _assoc] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:1596:17)
    at Function.cljs.core.assoc.cljs$core$IFn$_invoke$arity$3 (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:6030:18)
    at Object.cljs$core$lift_ns [as lift_ns] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31737:32)
    at Object.cljs$core$print_map [as print_map] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31774:28)
    at Object.cljs$core$pr_writer_impl [as pr_writer_impl] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31182:18)
    at Object.cljs$core$pr_writer [as pr_writer] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31266:18)
    at Object.cljs$core$pr_seq_writer [as pr_seq_writer] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31270:11)
    at Object.cljs$core$pr_sb_with_opts [as pr_sb_with_opts] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31333:11)
    at Object.cljs$core$pr_str_with_opts [as pr_str_with_opts] (/Users/mfikes/Downloads/.cljs_node_repl/.cljs_node_repl/cljs/core.js:31347:63)


 Comments   
Comment by Mike Fikes [ 07/Jul/17 8:25 PM ]

It's not clear if there is a "correct" solution in this case as it appears to be a choice between two alternatives:

  1. Print such JS objects in a way that honors namespace map printing when the "Clojure" representation of the object is printed
  2. Just print the object with namespace maps turned off

The first alternative might seem "nicer" in some sense, while the second seems to more closely reflect the fact that the property name actually has a slash in it.

This seems to quickly go down a slippery slope with even attempting to print such pathological objects: What if it has more than one slash in it? In that case even the keyword representation becomes illegal:

cljs.user=> #js {:foo/bar/baz 33}
Invalid token: :foo/bar/baz

The above patch doesn't dwell on it too much and takes the easy way out, just turning off namespace maps when printing JS objects, and add a test ensuring that this is what is done.

Comment by Enzzo Cavallo [ 07/Jul/17 8:55 PM ]

I think we should only lift the ns if it is a clojure map.

Comment by Mike Fikes [ 07/Jul/17 8:58 PM ]

On further thought, perhaps the attached patch is too heavy-handed in the case that the JavaScript object has ClojureScript map values:

(js-obj "foo/bar" {:baz/quux 3})

prints as

#js {:foo/bar {:baz/quux 3}}

instead of

#js {:foo/bar #:baz{:quux 3}}

when *print-namespace-maps* is true.

Comment by Enzzo Cavallo [ 07/Jul/17 9:03 PM ]

I think that my PATCH will do

` #js {:foo/bar #:baz{:quux 3}} `

, not?
Should there be a test covering it?

Comment by Mike Fikes [ 07/Jul/17 9:08 PM ]

Enzzo: I suspect your patch has better behavior. (There might be a perf concern with any of these patches, given the area of code being changed.) Yes, I would propose adding a test.

Also, have you signed the C/A. (Your name doesn't appear to be listed here https://clojure.org/community/contributors)

Comment by Enzzo Cavallo [ 07/Jul/17 9:37 PM ]

I just signed
I write one more test for nested case.
Not sure if should be a comment on "(map? m)" saying "When it a javascript map, we should not try to lift ns"
I do not test this feature. Still reading about how to test.

Comment by David Nolen [ 08/Jul/17 9:19 AM ]

I don't think Enzzo patch has any big perf implications. Looks good to me, thanks.

Comment by David Nolen [ 08/Jul/17 9:27 AM ]

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





[CLJS-2188] Use :invalid-array-access instead of :invalid-aget / :invalid-aset Created: 07/Jul/17  Updated: 08/Jul/17  Resolved: 08/Jul/17

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

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

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

 Description   

From a dev ergonomics perspective it doesn't really make sense to have two different warning keys.



 Comments   
Comment by David Nolen [ 08/Jul/17 9:14 AM ]

fixed https://github.com/clojure/clojurescript/commit/50ee29659c78abec1d42629969613dc9aef136c9





[CLJS-2170] for macro does not support multiple body expr Created: 05/Jul/17  Updated: 08/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Isaac Zeng Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript


Attachments: Text File 0001-let-macro-for-support-multiple-body-expr.patch    
Patch: Code

 Description   

see
https://dev.clojure.org/jira/browse/CLJ-2200



 Comments   
Comment by Mike Fikes [ 08/Jul/17 7:14 AM ]

The Clojure analog of this ticket has been declined; this one should be as well.





[CLJS-2181] Can't compile string sources with modules Created: 06/Jul/17  Updated: 07/Jul/17  Resolved: 07/Jul/17

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

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

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

 Comments   
Comment by David Nolen [ 07/Jul/17 2:03 PM ]

fixed https://github.com/clojure/clojurescript/commit/970b76c8b38e11420eb90d4ba4f4f4a9a2fc2a5e





[CLJS-2187] Add tests for resolve Created: 07/Jul/17  Updated: 07/Jul/17  Resolved: 07/Jul/17

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

Type: Task Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Declined Votes: 0
Labels: core

Attachments: Text File CLJS-2187.patch    

 Comments   
Comment by António Nuno Monteiro [ 07/Jul/17 1:42 PM ]

Tests already exists. Closing





[CLJS-2184] Add `ns-publics` and `ns-imports` Created: 06/Jul/17  Updated: 07/Jul/17  Resolved: 07/Jul/17

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

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

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

 Comments