<< Back to previous view

[CLJS-1346] Support require outside of ns Created: 18/Jul/15  Updated: 30/Sep/16

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

Type: Enhancement Priority: Major
Reporter: Jonathan Boston Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Makes http://dev.clojure.org/jira/browse/CLJS-1277 useful.



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

My initial thoughts to tackle this one would be:

  • add support for the `require` special form (along with `require-macros`, `use`, `use-macros`, `refer-clojure` and `import` for completeness) in the analyzer.
  • have `cljs.analyzer/parse-ns` recognize more than the `:ns` AST :op, merging `require`s/`uses` etc into NS requires. Use the 'cljs.user NS by default if no NS form found.
  • this means that `cljs.closure/find-cljs-dependencies` can get at the `require`d dependencies in the build pipeline.

Pending questions:

  • How to disallow `require` etc. except at the top level?
  • Possibly only even allow it strictly after the NS form, or one or the other?
Comment by David Nolen [ 28/Sep/16 4:26 PM ]

I think we could easily reify the notion of the top level in the analyzer, i.e. `:top true` in the env if we're at the top. To me a harder (hardest?) question is how to allow separately compiled files with no namespaces to see each other's definitions as one would expect from the user namespace?

Comment by António Nuno Monteiro [ 28/Sep/16 5:08 PM ]

As per the Slack discussion, solving this issue could probably be done by:

  • having a `:toplevel` key in the analyzer environment that is true only if we're at the top
  • add new special forms to the analyzer (`require`, etc.)
  • for files that don't have `ns` as its first form, generate a `cljs.user` (random) sub-namespace
  • create a single cljs.user file with several `goog.provide` statements of its sub-namespaces
  • the above needs a custom var resolver in order to actually work and be useful
Comment by Thomas Heller [ 29/Sep/16 5:18 AM ]

My question would be: What does this solve that ns doesn't already solve in a clearer/saner way?

My concern is that require can never work the way it does in CLJ due to the async nature of JS engines (+ what closure expects), but if we introduce something that has a counterpart in CLJ people will expect to use it this way and be confused if it doesn't (ie. conditional require, run code and expect the next require to see it).

I understand that the motivation for this is "for scripts" but wouldn't we be better off with a simple tooling change that can compile "scripts" by asking for a fn (ie. my.script/useful-fn) with the usual rules? The compiler inventing random ns names with a custom var resolver sounds a whole lot complicated for little to no gain?

Comment by António Nuno Monteiro [ 29/Sep/16 5:58 AM ]

I understand your concerns, but I think that the `:toplevel` key in env would let us produce useful error messages that would guide users in the right direction.

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

This isn't just for scripts. There are other typical patterns that needs this like data literal files. Having a significant difference from Clojure here when we can in fact largely close the gap is and has always been undesirable. The outlined plan does not introduce any significant complexity - with cljs.user subnamespaces (these should have stable names not random ones btw) + custom resolve for cljs.user vars we will produce code that is perfectly usable for both runtime dev and advanced compilation.

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

Turns out we don't need custom resolution. Google Closure doesn't care if we add definitions to some other namespace.





[CLJS-1195] generic reusable command line argument parsing for REPLs Created: 10/Apr/15  Updated: 30/Sep/16

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

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


 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJS-1761] Allow parallel Transit analysis cache writes Created: 19/Aug/16  Updated: 29/Sep/16

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

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

Attachments: Text File CLJS-1761.patch    

 Description   

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



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

I suspect these lines in transit are a race condition:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Thanks for the confirmation that the issue is not resolved.

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

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





[CLJS-1786] Add knob for controlling printing of namespaced maps Created: 21/Sep/16  Updated: 28/Sep/16

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

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

Attachments: Text File CLJS-1786.patch    

 Description   

See these Clojure commits:

https://github.com/clojure/clojure/commit/d57b5559829be8e8b3dcb28a20876b32615af0cb
https://github.com/clojure/clojure/commit/b49c1984a1527d17951fbb23ddf9406805a1343f
https://github.com/clojure/clojure/commit/05a8e8b323042fa043355b716facaed6003af324



 Comments   
Comment by Lauri Oherd [ 28/Sep/16 4:30 PM ]

Added print-namespace-maps flag for controlling printing of namespaced maps.
Unlike in Clojure repl the default value is false in Clojurescript repl.
Please comment if this needs to be fixed.

Comment by David Nolen [ 28/Sep/16 4:44 PM ]

Why should the default be different from Clojure?





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

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

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

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

 Description   

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



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

Attached patch with fix.





[CLJS-1795] Support more options in the `:closure-warnings` compiler option Created: 28/Sep/16  Updated: 28/Sep/16

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Support more options in the `:closure-warnings` compiler option as per https://github.com/google/closure-compiler/wiki/Warnings#warnings-categories






[CLJS-1762] Bump Closure Compiler Created: 22/Aug/16  Updated: 28/Sep/16

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

Attachments: Text File CLJS-1762-3.patch     Text File CLJS-1762.patch    

 Description   

A new version of the Closure Compiler features an optimization that results in faster compilation, as well as some some changes to ES6-related features.



 Comments   
Comment by Juho Teperi [ 27/Aug/16 4:39 AM ]

Here is my version of the change.

Updating cljs.closure was quite straightforward, just removed all the checks and changed a few constructor calls.

I think I found all places to update the Closure version: pom.template.xml, project.clj and script/bootstrap. Bootstrap script also needed a change to account for changes closure-compiler filename.

  • ES6ModuleLoader has been replaced with ModuleLoader, it no longer
    needs to be passed to ProcessCommonJSModules etc. constructors
  • This version requires the version 20160822 and any checks needed to
    work with older versions have been removed
  • Closure-compiler jar name has been changed so lib/ and closure/
    directories should be removed before running bootstrap
Comment by Juho Teperi [ 27/Aug/16 4:55 AM ]

While tests pass with the patch, looks like module processing is broken. Tested with https://github.com/mneise/circle-color

Comment by Juho Teperi [ 15/Sep/16 5:19 AM ]

Current WIP branch at https://github.com/clojure/clojurescript/compare/master...Deraen:cljs-1762

I started writing some tests for module processing based on Mneise's circle-color example. I heavily simplified React UMD module and JSX processing for the tests, but not yet very happy with this method. Probably best to write the JS for tests from scratch.

Currently stuck on getting Closure compiler to return the processed code: https://github.com/clojure/clojurescript/compare/master...Deraen:cljs-1762#diff-84ffd22349e5ca1fe6322cb0e379b3b1R1530

Comment by Juho Teperi [ 27/Sep/16 5:15 PM ]

Proposed patch. Contains several FIXME notes, some I know how to fix, on others I would like to have some comments what would be best way to solve them.

Comment by David Nolen [ 28/Sep/16 8:20 AM ]

This is fantastic. Will review!





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

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

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

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

 Description   

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

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



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

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

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

Sure thing, I'll do some more benchmarks.





[CLJS-1782] Self-host: allow namespaces to require their own macros Created: 19/Sep/16  Updated: 27/Sep/16

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

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

 Description   

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

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

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

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

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

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

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



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

Attached patch with proposed fix.

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

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





[CLJS-1794] incomplete alias created for namespace cljs.spec warning under advanced compilation Created: 26/Sep/16  Updated: 26/Sep/16

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

Type: Defect Priority: Major
Reporter: Michael Glaesemann Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

λ lein --version
Leiningen 2.6.1 on Java 1.8.0_45 Java HotSpot(TM) 64-Bit Server VM



 Description   

Under advanced compilation, use of cljs.spec can cause analyzer warnings.

out/cljs/analyzer.js:4740: WARNING - incomplete alias created for namespace cljs.spec
var mchk = (function (){var and__6935__auto__ = cljs.spec;
                                                ^

Sep 26, 2016 7:18:00 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 1 warning(s)
Successfully compiled "out/main.js" in 43.391 seconds.


 Comments   
Comment by Michael Glaesemann [ 26/Sep/16 6:28 PM ]

I came across this when upgrading Om from alpha44 to alpha45. The commit that changed the behavior moved to from cljs to cljc for server-side rendering support. (https://github.com/omcljs/om/commit/439802239fdfb95b143cde33df26be7801069bf2)

Test case: https://gist.github.com/grzm/0d2db1d1364daeb6118b610c2fc60178

António Nuno Monteiro pointed out:
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L2666

            (let [mchk  #?(:clj  (some-> (find-ns 'clojure.spec)
                                   (ns-resolve 'macroexpand-check))
                           :cljs (and ^::no-resolve cljs.spec
                                      ^::no-resolve cljs.spec/macroexpand-check))




[CLJS-1685] Incorrectly lazy subvec when start param is nil Created: 17/Jun/16  Updated: 26/Sep/16

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

Type: Defect Priority: Minor
Reporter: Alf Kristian Støyle Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 1.9.36 on Mac and Windows


Attachments: Text File cljs-1685.patch    

 Description   

subvec in ClojureScript does not fail when start param is nil. This is different than in regular Clojure.

In Clojure:

(def foo (subvec nil 1))
CompilerException java.lang.IndexOutOfBoundsException, compiling:(form-init4645269128697935824.clj:1:10) 
(def foo (subvec nil nil))
CompilerException java.lang.NullPointerException, compiling:(form-init4645269128697935824.clj:1:10)

In ClojureScript:

(def foo (subvec nil 1))
#object[Error Error: Index out of bounds]
   cljs.core/build-subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5316:16)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$3 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5328:7)
   Function.cljs.core.subvec.cljs$core$IFn$_invoke$arity$2 (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5326:7)
   cljs$core$subvec (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:5319:1)
=> nil
(def foo (subvec nil nil))
=> #'user/foo

foo is of course not usable after this:

foo
#object[Error Error: No protocol method IIndexed.-nth defined for type null: ]
   cljs.core/missing-protocol (jar:file:/Users/stoyle/.m2/repository/org/clojure/clojurescript/1.9.36/clojurescript-1.9.36.jar!/cljs/core.cljs:264:4)


 Comments   
Comment by Joshua Miller [ 26/Sep/16 1:37 PM ]

Added fix and test.





[CLJS-1793] Few misplaced docstrings Created: 24/Sep/16  Updated: 24/Sep/16

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

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

Attachments: Text File CLJS-1793.patch    

 Description   

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






[CLJS-1666] Flag to optionally disable transit analysis cache encoding Created: 03/Jun/16  Updated: 23/Sep/16

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

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


 Description   

We need a flag to explicitly disable the transit encoding - this is to work around code that creates unencodeable forms for one reason or another. EDN had become more lax about encoding random Objects which allowed this stuff to go under the radar in the past.



 Comments   
Comment by Wilker Lúcio da Silva [ 06/Jun/16 9:10 PM ]

I'm having a compilation issue with ClojureScript 1.9.36, I'm writing an app that uses untangled and this file https://github.com/untangled-web/untangled-client/blob/f42088c84b059562a48455a71daa6e4ea08d286c/src/untangled/client/data_fetch.cljs fails to compile with Caused by: java.lang.RuntimeException: java.lang.Exception: Not supported: class cljs.tagged_literals.JSValue this issue doesn't happen with 1.9.14

Comment by David Nolen [ 06/Jun/16 10:06 PM ]

We should investigate whether just handling JSValue + other cases like clojure.lang.Var is good enough.

Comment by David Nolen [ 08/Jun/16 8:53 AM ]

JSValue encoding/decoding landed in master fixing part of the issue. Still need more information about the Var case however.





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

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

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


 Description   

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

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

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

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






[CLJS-1791] :hierarchy option of defmulti is not used to determine if one dispatch value dominates on another Created: 22/Sep/16  Updated: 22/Sep/16

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

Type: Defect Priority: Major
Reporter: Andrey Zaytsev Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug

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

 Description   

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






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

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

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

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

 Description   

There is some duplication and redundant functions around List creation.

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






[CLJS-1536] REPL def symbol init collision Created: 03/Jan/16  Updated: 19/Sep/16

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

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

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

 Description   

In a REPL, if you try def where the init is a local matching the symbol being defined, then analysis fails.

(let [x 1]
  (def x x))

This can be verified in script/noderepljs and you can see it is some bad interaction with REPL var emission because if :def-emits-var false is added to the script, things work.



 Comments   
Comment by Txus Bach [ 04/Jan/16 3:01 PM ]

Confirmed that evaluating this breaks:

(require 'clojure.tools.reader
         'cljs.analyzer
         'cljs.compiler)

(let [env '{:ns {:name cljs.user}
            :def-emits-var true
            :locals {}}]
  (->> "(let [x 1] (def x 3))"
       clojure.tools.reader/read-string
       (cljs.analyzer/analyze env)
       cljs.compiler/emit-str))

(When emitting, not when just analyzing.)

Removing `:def-emits-var true` makes the bug disappear.

Looking into how to fix it – any clues are welcome, it's my first time around the codebase

Comment by Txus Bach [ 04/Jan/16 3:45 PM ]

Seems that var-ast is returning nil, because resolve-var doesn't return a map with an :ns key. Not sure if this code should work, but it returns nil:

(let [env {:ns {:name 'cljs.user},
 :def-emits-var true,
 :locals
 {'x
  {:init
   {:op :constant,
    :env
    {:ns {:name 'cljs.user},
     :def-emits-var true,
     :locals {},
     :line nil,
     :column nil,
     :context :expr},
    :form 1,
    :tag 'number},
   :name 'x,
   :binding-form? true,
   :op :var,
   :env {:line nil, :column nil},
   :column nil,
   :line nil,
   :info {:name 'x, :shadow nil},
   :tag 'number,
   :shadow nil,
   :local true}}}]
  (ana/resolve-var env 'x))

Will continue tomorrow. It's so much fun!

Comment by António Nuno Monteiro [ 16/Aug/16 4:29 PM ]

Attached patch with fix and test.

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

I tried António's patch with various expressions in the REPL and couldn't find a way to break it.

Comment by David Nolen [ 19/Sep/16 9:24 AM ]

Would like to address the fact that `((let [x 1] (defn x [] x))` does not appear to work in the REPL. I'm looking into it.

Comment by António Nuno Monteiro [ 19/Sep/16 9:28 AM ]

Yeah, that's weird. This is what I tested with and works:

cljs.user=> (let [x 1] (defn x [] x))
#'cljs.user/x
cljs.user=> (x)
1




[CLJS-1715] Self-host: Symbols in cljs.spec.test/instrument Created: 27/Jul/16  Updated: 16/Sep/16

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

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

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

 Description   

For bootstrapped ClojureScript, within cljs.spec.test/instrument:

1. There is a use of speced-vars that conditionally needs the $macros suffix (analogous to CLJS-1656).
2. The references to the runtime fns instrumentable-syms and distinct-by need to be qualified (otherwise their expansions use $macros.



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

Previous patch no longer applies. Attaching a rebased patch. This patch is still relevant to master.

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

Needs a rebase.

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

Does CLJS-1773 address this?





[CLJS-1696] Alias clojure.spec.gen => cljs.spec.impl.gen Created: 29/Jun/16  Updated: 16/Sep/16

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

Type: Defect Priority: Trivial
Reporter: Shaun LeBron Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-1696.patch    

 Description   

A special case for http://dev.clojure.org/jira/browse/CLJS-1692 to correctly alias clojure.spec.gen.



 Comments   
Comment by Shaun LeBron [ 29/Jun/16 12:02 PM ]

code and test

Comment by David Nolen [ 29/Jun/16 2:04 PM ]

I'm not excited about special casing this one. I'd rather wait to see if people actually use this namespace frequently enough for this to be desirable.

Comment by Shaun LeBron [ 29/Jun/16 10:37 PM ]

yeah, i guess the question is who should be responsible for handling this special case-- the user with reader conditionals or the compiler with this patch.

for extra context, I asked about this last week in slack: https://clojurians.slack.com/archives/clojurescript/p1466866626001218

Comment by Oliver George [ 13/Aug/16 4:41 AM ]

Just to bump this, I see references to requiring clojure.spec.gen in a few places now. The Clojure spec guide mentions it in project setup for generators, and the Clojure spec screencast on customizing generators.

One possibly related point. It seems necessary to require `clojure.test.check` in order for requiring `cljs.spec.impl.gen` to work.





[CLJS-1764] Double warning for undeclared Var Created: 26/Aug/16  Updated: 16/Sep/16

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

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


 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





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

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

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


 Description   

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

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

This becomes

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

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

This seems like something Google Closure should address.

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



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

To test

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

Compiler options

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

Code

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

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

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

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

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

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

This is what webpack generates

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

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





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

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

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

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

 Description   

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



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

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





[CLJS-1627] jsdoc parsing fails to recognize union types, breaking resolution Created: 18/Apr/16  Updated: 02/Sep/16

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

Type: Defect Priority: Minor
Reporter: Patrick Killean Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File CLJS-1627-4.patch     Text File CLJS-1627-5.patch    
Patch: Code and Test

 Description   

The Closure Spec For Union Types states that parentheses are necessary for union type expressions. Trying this ...

(defn foo
  "@param {(IBar|IMap)} x"
  [x] 
  ...)

Raises a Closure Error :

...ERROR - Bad type annotation. expected closing }
* @param {user.(IBar|user.IMap)}

This is because comp/resolve-types treats the parentheses as a part of the type tokens and incorrect var resolution occurs as a result. In addition, the compiler emits multiple resolved types separated by "|" characters but does not enclose them in parentheses to create a valid union type.



 Comments   
Comment by Patrick Killean [ 18/Apr/16 4:36 PM ]

This patch includes:

  • comp/resolve-types now removes parentheses when present and emits them when >1 type is detected. This makes parenthesis use optional and existing code remains unbroken (with the added benefit that it may work now)
  • changes to comp/resolve-type
    1. checks for js globals like document or window which are recognized by closure
    2. allows dot.delimited.forms to pass through so we can use types defined in externs and avoid unnecessary resolution
    3. uses ana/resolve-existing-var with a "unresolved jsdoc type" warning
    4. checks if a resolved var is a protocol and warns otherwise. This is more informative than Closure's standard unrecognized type error
  • a test for comp/resolve-types
Comment by David Nolen [ 21/Apr/16 12:45 PM ]

Thanks will try to look more closely at this tomorrow.

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

The patch is getting there, please remove the `js-doc-type` meta stuff. Just extend the signature of resolve-existing-var to take an additional parameter - the confirm-var-exists handler.

Comment by Patrick Killean [ 09/May/16 4:58 PM ]

CLJS-1627-1.patch:
resolve-existing-var now has an additional arity that accepts a missing-var handler passed to confirm-existing-var

Comment by Patrick Killean [ 10/May/16 6:16 AM ]

This has revealed a problem where deftype + defrecord using Object protocols emit resolved names when really they shouldn't. For example : "@implements {cljs.core.async.impl.timers.Object}" --> Bad Type Annotation

Since Object is a special case simply excluding it from the comments should fix it. Another patch incoming

Comment by Patrick Killean [ 10/May/16 7:42 AM ]

CLJS-1627-2.patch:
The emit* methods for deftype and defrecord now filter out Object protocols.

This produced an interesting result! With no more bad type annotations, static analysis can now proceed... and it has alot to say. Theres all kinds of info now about arity discrepencies (particularly cljs.core.IndexedSeq), type mismatches, and more. It even includes a type coverage percentage. Lots to parse here but very cool.

Comment by Patrick Killean [ 18/May/16 4:26 PM ]

CLJS-1627-3.patch:

  • fix require extern
  • add type application support for Array & Object
  • GC likes uppercase for Object & Array, lowercase for string, number.
  • support for explicit nullable types, variable typed arg
  • function type context modifiers this + new

Missing is the GC 'record type' . It also may be useful to fill out the node externs for common types

Comment by Patrick Killean [ 20/May/16 11:42 AM ]

CLJS-1627-4.patch:

  • fix a few problems in last patch
  • add record type support. Everything here should be covered
Comment by Patrick Killean [ 02/Sep/16 8:21 AM ]

update patch





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

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

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


 Description   

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

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






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

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

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

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

 Description   

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






[CLJS-1768] cljs.spec perf tweaks Created: 31/Aug/16  Updated: 31/Aug/16

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

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


 Description   

Same as the following commits:

https://github.com/clojure/clojure/commit/de6a2b528a18bcb4768e82d0d707d2cab26268a6
https://github.com/clojure/clojure/commit/defa7b8ef268ea2b8772658ade2010ca5ad00dc4






[CLJS-901] Protocolize compiler access to the file system Created: 03/Dec/14  Updated: 28/Aug/16

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Mike Fikes
Resolution: Unresolved Votes: 3
Labels: None


 Description   

Currently builds are based on files on disk. It is desirable to be able to instead get in memory builds, WebDAV based builds, S3 based builds, etc. Many of these alternative strategies are not in scope for the ClojureScript compiler but this does not mean we should not supply the needed hooks for users to control the behavior.



 Comments   
Comment by Thomas Heller [ 03/Dec/14 9:31 AM ]

This and some other issues opened recently (CLJS-900, CLJS-851, CLJS-899, ...) have some overlap with what I have done in shadow-build [1]. Memory based builds are actually already possible cause it will only touch the disk when asked to, although the API could use some cleanup.

Anyways, might be worthwhile to coordinate these efforts to make CLJS more accessible for everyone.

[1] https://github.com/thheller/shadow-build

Comment by Alan Dipert [ 04/Feb/15 11:36 AM ]

I too think it would be totally awesome to have builds based on sources from disparate places.

One alternative in this spirit I have been thinking about is a "SourceSet" approach. The idea is, instead of teaching CLJS how to consume various place-types directly via protocols, provide an API for building a "SourceSet" value and also a build function that takes the SourceSet as input. I imagine the SourceSet in its simplest form as a map of namespaces to string sources.

With a value to represent sources that is place-agnostic and immutable, 3rd party tools can consume/emit/transform these values before invoking a compile without knowledge or interest in CLJS internals. Conversely CLJS need not be concerned with how SourceSets are constructed.

This whole idea is inspired by boot's FileSets, which work basically the same but can't have the "it fits in memory" assumption.





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

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

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


 Description   

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

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

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






[CLJS-1634] Track bound dynamic variables to support binding in async mechanisms. Created: 26/Apr/16  Updated: 20/Aug/16

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

Type: Enhancement Priority: Minor
Reporter: Christian Weilbach Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: cljs, enhancement
Environment:

Any cljs version.



 Description   

The issue has been raised before:

While the reasoning behind the proposal is still valid, the original approach has made no progress due to the performance penalty. I have implemented a simplified approach with mutable JavaScript datastructures to minimize the performance impact. Because we are single-threaded we can use js assignment and don't need to port Clojure's binding frame. A small penalty is paid by the user of binding (see benchmark8) and a higher one by async mechanisms capturing and restoring the bindings (benchmark1-7):

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

I would provide patches to ClojureScript, if this looks like a worthwhile approach.



 Comments   
Comment by Antonin Hildebrand [ 30/Apr/16 6:05 AM ]

Just for record I commented on it here: https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a#gistcomment-1764489. Not sure if GitHub sends out notifications about new gist comments.

Comment by Christian Weilbach [ 30/Apr/16 6:18 AM ]

Thanks for pointing it out. David Nolen has also pointed out prototype chains to address this issue and now I see what he meant. I am not familiar enough with the internals of "this" in JavaScript, but one problem I see is that you need to distinguish dynamic vars at the call site. The advantage of using an object directly and capturing and restoring the frame explicitly when you enter and leave the code is that call sites are totally unaffected. The cost is only paid a little at the binding site and mostly in async libraries (bound-fn). But I might still need to look further into "this" . I have not got the gist comment from github.

Comment by Antonin Hildebrand [ 30/Apr/16 7:23 AM ]

Correct.

> you need to distinguish dynamic vars at the call site

I agree. My initial motivation was to solve a bit different problem without cooperation from library authors. I didn't want to modify ClojureScript behaviour and wanted to be just touching own code or doing trivial changes in library forks. Just wanted to share my thoughts about the implementation.

I have a feeling that solving this "async context" problem will be difficult. You will need async library authors to adapt their libraries. And users unaware of this will be running into issues anytime they step outside of bound-fn aware async libraries (for example using raw js interop). I believe Angular people solved this robustly in https://github.com/angular/zone.js. The implementation is quite scary monkey-patching, but if they were able to wrap all existing async calls at lowest level, maybe we could just build on top of their foundation and use zone.js as parallel mechanism for `binding`.

Comment by Christian Weilbach [ 02/May/16 4:58 PM ]

The angle I am coming from is roughly described here: https://github.com/fullcontact/full.monty/pull/9#issuecomment-131152058

I only found out at the very end when I had supervision of go-channels completely implemented, that the cljs binding was not behaving like the Clojure one. Arguments pro/contra binding in Clojure are also referenced. The zone monkey patching looks very heavy and prone to cause hairy bugs. It is noteworthy that Clojure does not embrace bindings, but keeps them always thread-local. So there you also have to use bound-fn or something similar whenever code is executed concurrently. core.async for instance uses the Clojure mechanism to push (capture) and pop (restore) bindings. I would like to have this in ClojureScript as well. I think one should not retain all bindings automatically, but rather allow the library author to handle dynamic bindings. I only track the supervisor binding for instance. For ClojureScript as for Clojure libraries and wrappers this should be fine. Pure JavaScript libraries usually have their own binding concepts like zone.js, right?

Tracking bindings is neither for free in Clojure nor in ClojureScript and it is an important design goal to embrace the host. In fact originally I tried to capture and restore all bindings. My benchmarks for tracking more dynamic vars (instead of just the currently active binding), were linearly more expansive than rebinding fewer selected vars and become prohibitive when you reach a few hundred.

Comment by Antonin Hildebrand [ 02/May/16 5:16 PM ]

I would be happy if your proposal went through. It would help my use-cases as well.

I'm going to explore zone.js when I get some spare time. I will try do write a wrapper library and implement an alternative mechanism to bindings using zone.js. I would like to provide this functionality as a library without a need to modify ClojureScript compiler or involvement from library authors.

Comment by Christian Weilbach [ 03/May/16 1:39 AM ]

Ok, I am curious how well this will work. Would this work with the state-machine transformation of core.async?

Comment by Antonin Hildebrand [ 03/May/16 3:25 AM ]

I believe so. Core.async state machine uses only setTimeout and goog.async.nextTick. We can teach zone.js to deal with nextTick by setting goog.async.nextTick.wrapCallback_ with zone wrapping. Also if user decided to use any async API in their go blocks it should work, because zone.js will carry proper zone binding over async boundaries.

Comment by Antonin Hildebrand [ 03/May/16 3:50 AM ]

I have opened this issue in zone.js: https://github.com/angular/zone.js/issues/342

Comment by Christian Weilbach [ 26/May/16 8:56 AM ]

Hey. Have you made any progress with implementing a small cljs demo with zone.js yet?

Comment by Antonin Hildebrand [ 27/May/16 5:57 AM ]

Hi Christian. No, unfortunately I didn't get to it.

Comment by Christian Weilbach [ 07/Aug/16 4:26 PM ]

Interestingly to implement the Common Lisp like condition system chouser presented here https://www.youtube.com/watch?v=zp0OEDcAro0 the dynamic binding working over async boundaries is also important.

Comment by Antonin Hildebrand [ 08/Aug/16 5:53 PM ]

Christian, please have a look at my implementation:
https://github.com/binaryage/cljs-zones

I have implemented the prototype trick as a library. It is just a gist of the idea, didn't spend time to make it robust yet and ES3-compatible. Re-binding frames should be as cheap as changing pointers (inside JS runtime).

Comment by Christian Weilbach [ 10/Aug/16 6:10 AM ]

Very nice work! I am checking it out atm. Nice that it is self-contained. (The Klipse version throws a goog.object not found error for me btw.)

Comment by Christian Weilbach [ 16/Aug/16 4:05 PM ]

I have updated the gist to incorporate cljs-zones in benchmark 8, 9 and 10. It is a bit faster in restoring dynamic bindings, but not much:

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

But there is a significant performance penalty on var access:

full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 1 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 2 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 6 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 3 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs

Is this penalty addressable?

One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism, but this might be acceptable.

Comment by Antonin Hildebrand [ 16/Aug/16 4:17 PM ]

Thanks for measuring it. I didn't really get to benchmarking yet.

Did you run the benchmarks under :advanced optimizations?

zones/get currently emits a call to goog.object/get, I'm not sure if this gets inlined by closure compiler, but if not, we can probably improve it by generating raw js object access:
https://github.com/binaryage/cljs-zones/blob/fdfa1421c39d64c9c6b9efbff474b7677f908197/src/lib/zones/core.clj#L94

> One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism
Can you elaborate? I don't understand how it would interfere.

Comment by Christian Weilbach [ 17/Aug/16 4:10 AM ]

With advanced compilation I get:

full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 2 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 3 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 1 msecs

But this is a very simple test. If you see a better way to benchmark it, go ahead . When the chains get prototype chains get longer, it might be more painful to walk the chain for each global lookup.

> Can you elaborate? I don't understand how it would interfere.
There has been a proposal somewhere to pass "this" in all cljs functions instead of null as first argument. Your explicit model of a zone is pretty safe I think, but if people would interefere with prototypes in JS, then this might break the "this" approach in subtle ways. Without passing "this" as a direct argument and doing it in a more JS way, you have to setup the zones all the time instead of using the scope chain, which might also contribute to the managing cost. So the "this" approach might help with performance, but I am not sure. You still have the chain overhead. But I am no JS expert, you know much better than me what you are doing.

Comment by Christian Weilbach [ 20/Aug/16 5:37 AM ]

My last comment was a little bit confusing. I only see the problem with an impact on dereferencing vars. In the benchmark above the prototype chain is very short (v1 can be directly resolved) and already has a significant penalty, but I think the penalty gets even worse for not rebound root vars if you have a higher nesting level for the binding and the chain gets longer. Can you address this somehow?





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

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

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


 Description   

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

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






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

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

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

Win 7 64bit



 Description   

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

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

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

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

THIS WORKS:

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

BUT THIS DOESN'T:

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

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

Thanks for help!






[CLJS-1544] cljs.test REPL reload support Created: 13/Jan/16  Updated: 14/Aug/16

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

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


 Description   

When adding a test to a test ns that uses cljs.test and re-loading (via require + :reload) that namespace in the REPL after saving the file - invoking run-tests does not include the newly added test.






[CLJS-1745] refer-clojure doesn't pull in previously excluded vars Created: 14/Aug/16  Updated: 14/Aug/16

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

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


 Description   

Observe this behavior in Clojure:

user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
CompilerException java.lang.RuntimeException: Unable to resolve symbol: juxt in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> partial
CompilerException java.lang.RuntimeException: Unable to resolve symbol: partial in this context, compiling:(NO_SOURCE_PATH:0:0)
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
#object[clojure.core$juxt 0x2f62ea70 "clojure.core$juxt@2f62ea70"]
foo.core=> partial
#object[clojure.core$partial 0x38aa816f "clojure.core$partial@38aa816f"]

Compare with ClojureScript:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 49402
To quit, type: :cljs/quit
cljs.user=> (ns foo.core (:refer-clojure :exclude [juxt partial]))
nil
foo.core=> juxt
nil
foo.core=> partial
nil
foo.core=> (refer-clojure :exclude [])
nil
foo.core=> juxt
nil
foo.core=> partial
nil





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

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

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

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

 Description   

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






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

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 1
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





[CLJS-1734] :import in ns silently discards imported classes with the same name Created: 11/Aug/16  Updated: 11/Aug/16

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(ns some.ns
  (:import goog.fx.Transition.EventType
           goog.history.EventType))

Both classes are named EventType and the second will effectively remove the first one without warning or error.






[CLJS-1630] Add unit test for static dispatch Created: 21/Apr/16  Updated: 10/Aug/16

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

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

Attachments: Text File CLJS-1630.patch    

 Description   

This unit test is an edge case that illustrates why in the code of `emit :invoke` we must stay with `call` for the high order case where static information is missing .



 Comments   
Comment by Yehonathan Sharvit [ 10/Aug/16 10:52 PM ]

Could someone take a look at this unit test?





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

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

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

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

 Description   

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

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

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

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



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

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

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

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

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





[CLJS-884] with-redefs don't restores previous values when used inside go block Created: 09/Nov/14  Updated: 07/Aug/16

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

Type: Defect Priority: Trivial
Reporter: Vladimir Iakovlev Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: None
Environment:

clojure 1.6, clojurescript 0.0-2371, core.async 0.1.346.0-17112a-alpha


Attachments: Text File cljs_884.patch    

 Description   

For example I have code and test:

(defn test-fn [] :original)

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))

And test failed in last `is`:

FAIL in (test-with-redefs-async) (:)
expected: (= (test-fn) :original)
  actual: (not (= :mocked :original))


 Comments   
Comment by Vladimir Iakovlev [ 09/Nov/14 1:26 PM ]

I attached little patch with fix. With it `with-redefs` forced to use previous value for restoring bindings.

But I don't know how to add test for this issue, because it requires core.async. So I tested with:

(defn test-fn [] :original)

(deftest test-with-redefs-sync
  (is (= (test-fn) :original))
  (with-redefs [test-fn (fn [] :mocked)]
    (is (= (test-fn) :mocked)))
  (is (= (test-fn) :original)))

(deftest ^:async test-with-redefs-async
  (go (is (= (test-fn) :original))
      (with-redefs [test-fn (fn [] :mocked)]
        (is (= (test-fn) :mocked)))
      (is (= (test-fn) :original))
      (done)))
Comment by Yehonathan Sharvit [ 07/Aug/16 2:49 AM ]

Why this bug is not adressed?

Comment by Antonin Hildebrand [ 07/Aug/16 4:57 PM ]

It might be related to CLJS-1634. The likely reason: it is technically very hard to solve without paying non-negligible runtime price for it.

Comment by Yehonathan Sharvit [ 07/Aug/16 11:26 PM ]

The problem mentioned in http://dev.clojure.org/jira/browse/CLJS-1634 is much wider.

The current issue only deals with the fact that `with-redefs` doen't restore previous values when used inside go block.

There is a very simple solution to this specific issue here: https://nvbn.github.io/2014/11/05/protocols-for-testing/ (also written by Vladimir).





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

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

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


 Description   

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

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

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

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

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

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

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

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






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

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

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

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

 Description   

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

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

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

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

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






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

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

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

Attachments: Text File CLJS-1724.patch    

 Description   

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

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

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






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

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

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

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

 Description   

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

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

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Just adding a motivational screenshot:

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

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





[CLJS-1474] Error if reserved symbol is defined Created: 21/Oct/15  Updated: 31/Jul/16

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

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

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

 Description   

Currently a definition like

(defn set! [] ...)

will not cause any warning. Any usage of it (without :as namespace aliasing) however will not use the defined var but the set! special form.

A warning seems appropriate.



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 1:19 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 2:09 PM ]

I know David suggested that a hard error is probably the right thing to do for this one, but one consequence is that the cljs.spec/def macro cannot be defined in bootstrap with this change. (I haven't investigated thoroughly, but this may simply be the result of macros being processed as ClojureScript in bootstrap, and thus being subject to this new guard.)

Regardless of the root cause, you'll see this if you try to run script/test-self-parity:

#error {:message "Could not eval cljs.spec", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't def special form at line 51 ", :data {:file nil, :line 51, :column 1, :tag :cljs/analysis-error}}}

For reference: Line 51 currently points at the def macro: https://github.com/clojure/clojurescript/blob/e2db5d9ff8cb6a099ebc2a8cd379385bf4649b38/src/main/cljs/cljs/spec.cljc#L51





[CLJS-1636] Mark some symbols in core macros ns as private Created: 27/Apr/16  Updated: 31/Jul/16

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

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

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

 Description   

There are some symbols in the core macros namespace that are not meant for external consumption. Some of these are marked private and some aren't. This ticket asks that the others be marked private as well.

An example of one symbol marked private is defcurried.
An example of one symbol not marked private is caching-hash.



 Comments   
Comment by Mike Fikes [ 27/Apr/16 8:21 AM ]

In CLJS-1636.patch, I checked and it appears nothing in the compiler codebase is explicitly using these symbols outside of the cljs.core namespace. But, it is still worth scanning through these to check if they make sense. For example js-debugger and js-comment are a couple that might actually be meant for public use, but it is difficult to tell.

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 27/Apr/16 2:43 PM ]

Note, that in #cljs-dev slack, there appears to be interest in caching-hash being public.

(I don't mind revising the patch to suit whatever is needed. At the same time, I'm certainly not in a position to take decisions on what is public API or not.)

Comment by Mike Fikes [ 31/Jul/16 1:33 PM ]

Patch no longer applies. Attaching updated patch.





[CLJS-1718] Foreign lib files should be placed in a location that matches their namespace Created: 29/Jul/16  Updated: 29/Jul/16

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

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

Affects master



 Description   

Using several foreign libs with the same file name ends up just including one of them, as the files are placed at the root of the `:output-dir`.

A solution for this would be placing those files in a location that matches their `:provides` namespace.






[CLJS-1297] defrecord does not emit IKVReduce protocol Created: 03/Jun/15  Updated: 27/Jul/16

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

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1297-19-July-2015.patch    

 Description   

Records are maps and in Clojure they support reduce-kv (IKVReduce protocol).
This is not true in ClojureScript:

(defrecord Foobar [x y])
 (reduce-kv assoc {} (Foobar. 1 2))

Fails wit Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]



 Comments   
Comment by David Nolen [ 03/Jun/15 7:25 PM ]

Just seems like an oversight. Patch welcome, this one is a relatively easy one.

Comment by Daniel Skarda [ 04/Jun/15 2:53 AM ]

OK

I checked Clojure implementation. Records do not implement any reduce protocol on their own. For IKVReduce records use default implementation using reduce and destructuring. Is this approach OK?

Recently Alex Miller implemented many optimizations of reduce protocols in Clojure. Eg range returns an object which implements IReduce protocol so reduce (and transducers in general) can take advantage of it. Any plans for such optimizations in ClojureScript?

;;clojure/src/clj/clojure/core.clj:6523
;;slow path default
clojure.lang.IPersistentMap
(kv-reduce 
  [amap f init]
  (reduce (fn [ret [k v]] (f ret k v)) init amap))
Comment by David Nolen [ 04/Jun/15 9:05 AM ]

Going with the Clojure implementation is fine. Yes all of the optimizations in 1.7.0 are on the table for ClojureScript but these are separate issues from this one.

Comment by Samuel Miller [ 16/Jul/15 10:39 PM ]

Mind if I take this as my first cljs bug? Poking around quickly I think I know what needs to happen.

Comment by David Nolen [ 17/Jul/15 5:21 AM ]

Sure! Have you submitted your CA yet?

Comment by Samuel Miller [ 17/Jul/15 7:13 PM ]

Yes, I did yesterday.

Comment by Samuel Miller [ 20/Jul/15 9:52 PM ]

Here is a potential patch. I implemented a basic IKVreduce based on Daniel Skarda's comment. Note: I am a little fuzzy on macros still so please look over what I have. There is probably a better way. Also added a test for reduce-kv on records.

I ran the test on Linux on V8 and SpiderMonkey. I plan to get JSC and Nashorn working and tested this week but if someone wants to test them out before that would be great.

Comment by Sebastian Bensusan [ 23/Jul/15 6:45 PM ]

Experience report:

I just tested the patch in the Node Repl and it seems to work:

cljs.user=> (defrecord A [a b])
cljs.user/A
cljs.user=> (reduce-kv (fn [m k v] (assoc m k (inc v))) {} (A. 1 2))
{:a 2, :b 3}

and the provided tests passed in Spidermonkey, V8, and Nashorn (I don't have JSC installed).

For completeness: before applying the patch the same code fails with:

Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]
Comment by David Nolen [ 10/Aug/15 10:22 PM ]

Is this the same approach taken by Clojure?

Comment by Samuel Miller [ 10/Aug/15 10:36 PM ]

You can see the relevant current Clojure code here...
https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L6526
I think it is the same. I literally just tried to translate it over into CLJS. I might of understood something wrong though.

Comment by David Nolen [ 11/Aug/15 6:10 AM ]

Yes that's the slow path. Please use the implementation used by defrecord instead. If defrecord doesn't have one then this patch is OK.

Comment by Samuel Miller [ 11/Aug/15 8:48 PM ]

As far as I can tell there is no implementation on defrecord itself however there are separate implementations on the the java classes PersistentVector, PersistentArrayMap, PersistentTreeMap, and PersistenHashMap in pure java. I am not sure if you would want to do something similar for Clojurescript.

I can also spend some time trying to make a more performant version.

Comment by António Nuno Monteiro [ 27/Jul/16 7:38 AM ]

Confirmed that Clojure uses the slow path via the IPersistentMap implementation in defrecord
https://github.com/clojure/clojure/blob/d920ad/src/clj/clojure/core.clj#L6712

Patch still applies and can also confirm it works for me.





[CLJS-1714] Clojurescript github README.md should identify source Clojure version Created: 24/Jul/16  Updated: 26/Jul/16

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

Type: Enhancement Priority: Trivial
Reporter: Marshall Abrams Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It would be helpful if the Clojurescript README at https://github.com/clojure/clojurescript/blob/master/README.md explicitly identified the Clojure version on which the current stable release of Clojurescript is based, or the Clojure version(s) which the Clojurescript release most closely tracks. This is especially helpful during periods when the Clojurescript release recommended in README.md is based on a Clojure alpha or even beta version, since there may be significant changes between Clojure versions in this case, so users might expect more similarity between the latest public releases of the two dialects than is warranted. Of course one can also go through changes.md to figure out what Clojure version is the basis of each Clojurescript release, but I think it's worth making it easier for users to notice.



 Comments   
Comment by David Nolen [ 25/Jul/16 2:08 PM ]

ClojureScript depends only on final releases of Clojure. This dependency is expressed in the pom.xml file which Maven based tooling and its users already understand. I don't see a compelling reason to include yet another thing we have to remember to update in the README.

Comment by Marshall Abrams [ 25/Jul/16 9:46 PM ]

First, I'm not sure if what I wrote was clear. I'm not talking about a dependency on the Clojure version used to compile to Javascript. I'm talking about the relationship between a Clojurescript version and the Clojure version that was ... ported to the Clojurescript release.

I agree it would a pain to try to remember to update this each time the Clojurescript version changed. That was one of my reservations, but I thought the point was important enough-especially for new users-that it seemed worth suggesting. I suppose that if there was already a line in the README about it, that will help, but I know that it's still easy to forget to update that kind of thing. Can't be helped.

Here are some reasons to consider updating the README with the Clojure version(s) that are the "source" of a Clojurescript version:

It's very useful information what version of Clojure the current Clojurescript release is most similar to. That way, information about the Clojure version is useful for Clojurescript.

At present, the official releases of Clojurescript listed in the README is based on what are currently alpha Clojure releases. Since the Clojure releases are alpha releases, they're changing quickly, so whatever's documented on the Clojure website may be out of sync with the Clojurescript release. One needs to know if that's so. (If all I want from the current Clojurescript release is what's in the previous non-alpha/beta Clojure release, knowing the Clojure version that was the source probably doesn't matter if there have been no breaking changes in the alpha releases on which the current Clojurescript version is based, but if I want to experiment in Clojurescript with new Clojure features-spec in this case-I need to know if it might differ from the latest Clojure version. (explain-data changed from Clojure 1.9.0-alpha7 to alpha10, at least.)

(I think that the comment about Maven and pom.xml was based on assuming that I was talking about the Clojure version used to compile Clojurescript code. If so, you can stop reading this paragraph. If not, then I would say that though Maven may be routinely used by Clojurescript developers-I don't know-but Clojurescript has grown beyond a core community of hardcore users and developers. It's used for a lot of real work, and it's a good language for new users coming from Clojure or Javascript. There are books for sale, and good online tutorials. The wiki is quite good for new users now. All of the templates on the wiki use Leiningen or Boot. There's no mention of Maven. There is no need for most users to use Maven with Clojurescript, or to even know what a pom file is. In my case, I do know what a pom file is, and have read quite a few, but wouldn't have thought to look there. I've never used Maven, however. Leiningen has been enough.)

Those are my thoughts on the matter. Thanks much for considering it.





[CLJS-1709] clojure.data/diff throws an exception when comparing map keys of different types when used on sorted maps Created: 19/Jul/16  Updated: 22/Jul/16

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

Type: Defect Priority: Minor
Reporter: Thomas Scheiblauer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug


 Description   

e.g.
(clojure.data/diff (sorted-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (sorted-map :foo 42) (sorted-map "x" 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map 1 42))
(clojure.data/diff (hash-map :foo 42) (sorted-map "x" 42))
will throw e.g.
Error: Cannot compare :foo to 1
while e.g.
(clojure.data/diff (hash-map :foo 42) (hash-map 1 42))
(clojure.data/diff (hash-map :foo 42) (hash-map "x" 2))
(clojure.data/diff (sorted-map :foo 42) (sorted-map :bar 42))
will not.

The same applies to Clojure with a different exception (e.g. "java.lang.Long cannot be cast to clojure.lang.Keyword")






[CLJS-1712] Make PersistentHashSet implement IReduce Created: 21/Jul/16  Updated: 21/Jul/16

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

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

Attachments: Text File difference-benchmark.txt     Text File into-benchmark.txt     Text File phs-reduce.patch     Text File union-benchmark.txt    
Patch: Code

 Description   

This improves speed of many reduce based operations on set which were falling back to seq-reduce, including code in `clojure.set` namespace such as `clojure.set/union` and `(into [] some-set)`.

I've included a few benchmarks I performed using `simple-benchmark` in a JavascriptCore environment (Planck REPL)



 Comments   
Comment by Rohit Aggarwal [ 21/Jul/16 3:35 PM ]

I think the code currently is faithful to Clojure's implementation of PersistentHashSet. So any change from that would probably require more thought and/or history.

Also someone else also raised a similar issue on ClojureScript mailing list.





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

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

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


 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-1701] cljs.spec impact on :advanced builds Created: 07/Jul/16  Updated: 07/Jul/16

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

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


 Description   

Investigate the impact of cljs.spec on :advanced builds.

Currently all specs are kept in the (private) cljs.spec/registry-ref atom. This atom is not understood by the Closure Compiler and cannot be eliminated as dead code. So even if specs are not used in "production" they still bloat the generated JS size. Some specs may be used at runtime and cannot not be removed, the gen parts however are probably never required in :advanced builds and should be omitted somehow.

In a test build (with 1.9.93) this adds 11kb (102kb vs 91kb) as soon as cljs.spec is :require'd somewhere and goes up with each defined spec.






[CLJS-1447] IFn implementors have a broken call implementation, all args after 20th argument should be collected into a seq Created: 11/Sep/15  Updated: 05/Jul/16

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

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


 Description   

Regular fns (which are just JavaScript fns) have no such limit. For IFn implementors we should not allow arities above 21 args, and we should transform the 21st arity into a var args signature.



 Comments   
Comment by François De Serres [ 18/Jun/16 9:13 AM ]

we should transform the 21st arity into a var args signature

Unless misunderstanding, can't do that. Var args sigs aren't allowed in protocols.

we should not allow arities above 21 args

Emitting an analyzer warning is what you want?

Comment by Antonin Hildebrand [ 05/Jul/16 6:07 PM ]

I believe I hit this problem in my code using core.async[1].

If it is not possible to implement ATM, I would kindly ask for a compiler warning at least. This thing manifested as a infinite recursive loop ending up in a cryptic stack overflow.

[1] https://github.com/binaryage/dirac/commit/cce56470975a287c0164e6f79cd525d6ed27a543





[CLJS-1446] autodoc + gh-pages for cljs.*.api namespaces Created: 11/Sep/15  Updated: 02/Jul/16

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

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


 Comments   
Comment by W. David Jarvis [ 11/Sep/15 6:07 PM ]

I just tried to get this working - unfortunately, autodoc doesn't currently have support for ClojureScript. An issue is currently open on the GH project here but it doesn't look like it's seen any movement in nearly two years.

Comment by Tom Faulhaber [ 13/Sep/15 2:26 PM ]

I would love to see this work as well and, as the author of autodoc, am happy to help move it forward. I've added some commentary to the issue in autodoc about how to do this. If it's going to happen soon, though, I will need some help from the ClojureScript community as outlined over there.

Comment by David Nolen [ 14/Sep/15 10:42 AM ]

This ticket is about generating docs for Clojure code. Getting autodoc to work for ClojureScript files is worth pursuing but unrelated to this ticket.

Comment by Sebastian Bensusan [ 11/Oct/15 5:54 PM ]

I took at stab at this and only got it running using autodoc-0.9.0-standalone.jar from the command line. My results are not useful at all but those issues should be sorted out in autodoc.

David, do you have a preference in how the docs and artifacts needed should be managed? Should it be a lein plugin or can it be a script that assumes that the correct jars have been installed?

Comment by Tom Faulhaber [ 12/Oct/15 12:37 AM ]

Oh, I did misunderstand this and then didn't see David Nolen's follow-up until now. Let me take a look at whether I can make this happen pretty easily. I wouldn't think it would be too difficult. (Famous last words!)

Comment by Tom Faulhaber [ 02/Jul/16 2:14 AM ]

I have just closed the blocking issue in autodoc Issue 21, andSebastian Bensusan has successfully built a version of doc for the src/main/clojure/... stuff.

The next step is to flesh out what we want to push to http://clojure.github.io/clojurescript. I don't think that this is too hard. Then we can integrate it with the autodoc robot and get automatic updates.





[CLJS-1693] rel-output-path produces wrong path for :lib files Created: 25/Jun/16  Updated: 25/Jun/16

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

Type: Defect Priority: Major
Reporter: Ruslan Prokopchuk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Bug presents in any environment, but break things only on Windows.



 Description   

Building with a cljsjs package where cljsjs package includes :libs in deps.cljs you find the "out" directory includes a subdirectory "file:".

An example from my specific case:
"out/file:/home/vagrant/.m2/repository/cljsjs/openlayers/3.3.0-0/openlayers-3.3.0-0.jar!/cljsjs/development/openlayers/ol/array.js"

':' is not permitted on Windows filesystem, which leads to compilation error.

It happens because rel-output-path here https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1515 detects closure-lib first (before taking in account that it is in jar) and passes it to lib-rel-path which acts as if file is located in project directory.

Another issue, but deeply related, is that on Windows, lib-rel-path breaks build even for :lib files located in project directory. It happens because of naive path prefix removal https://github.com/clojure/clojurescript/blob/17bcf2a091accb6f7caf1e8fa3954b490e9d34fa/src/main/clojure/cljs/closure.clj#L1500 because it tries to remove a subpath with native path separators, but path comes here in url-like form with forward slashes. Compiler then reports that path is not relative and aborts compilation.






[CLJS-1691] spec internal compiler APIs Created: 21/Jun/16  Updated: 21/Jun/16

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

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





[CLJS-1690] spec the ClojureScript AST Created: 21/Jun/16  Updated: 21/Jun/16

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

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





[CLJS-1497] `find` on an associative collection does not return collection key Created: 30/Nov/15  Updated: 15/Jun/16

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

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


 Description   

Instead find returns the passed in key. This means metadata on the key will appear to be lost. Related to CLJS-1496.



 Comments   
Comment by Rohit Aggarwal [ 15/Jun/16 3:39 PM ]

Proposed Plan

IAssociative protocol has a function called -entry-at which has been commented out. This function needs to be implemented which will return the necessary data structure, similar to the way it has been done in Clojure.

An example of its implementation for PersistentArrayMap is:

(-entry-at
 [coll k]
 (let [idx (array-map-index-of coll k)]
   (when-not (neg? idx)
     [(aget arr idx) (aget arr (inc idx))])))

We will need to implement this for all the collections which implement that protocol.

A failing test case:

(deftest test-find-meta-cljs-1497
  (let [k        [1 2 3]
        m        {:my "meta"}
        v        1
        xs       {(with-meta k m) v}
        [k' v']  (find xs k)]
    (is (= k k'))
    (is (= v v'))
    (is (= m (meta k')))))




[CLJS-1587] Duplicate keys via quoting Created: 24/Feb/16  Updated: 15/Jun/16

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

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

Attachments: File CLJS-1587.diff    
Patch: Code and Test

 Description   

For

#{1 '1}

you get

#{1 1}


 Comments   
Comment by Peter Schuck [ 03/Mar/16 10:01 PM ]

This happens for the has-set macro and the hash-set literal. Here's what I get from the repl

cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}

Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.

Comment by Rohit Aggarwal [ 15/Jun/16 8:02 AM ]

The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.

Comment by Rohit Aggarwal [ 15/Jun/16 9:32 AM ]

I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function.

Patch has the fix and a test.





[CLJS-1678] variadic defn can be called for missing fixed arities, overlapping arity Created: 11/Jun/16  Updated: 12/Jun/16

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

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


 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-1641] Multi-arity defn copies arguments unnecessarily for all cases Created: 16/May/16  Updated: 12/Jun/16

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

Type: Defect Priority: Minor
Reporter: Stephen Brady Assignee: Stephen Brady
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

Background:

Passing js arguments as a parameter to another function is a known performance issue. Copying arguments to an array addresses this, and this approach has been taken to handle args passing for variadic functions in previous patches such as:
https://github.com/clojure/clojurescript/commit/dccc5d0feab9809cb56bd4d7257599a62232ac0d
https://github.com/clojure/clojurescript/commit/f09bbe62e99e11179dec6286fbb46265c12f4737

This commit (https://github.com/clojure/clojurescript/commit/576fb6e054dd50ec458a3c9e4172a5a0002c7aea) introduced a macro path for `defn` forms.

Current Behavior and Impact:

In the case of multi-arity defn forms, the macro path generates an array copy of the arguments variable regardless of whether it is used or necessary. In the case of multiple arities but no variadic arity, copying arguments is unnecessary as arguments will not be passed to the variadic method for the given function. In the case of multiple arities with a variadic case, an args array copy is needed but should be isolated to that case alone; currently, the array copy is performed before checking the arguments length, causing all cases to incur an (unused) args array copy.

Relevant code: https://github.com/clojure/clojurescript/blob/master/src%2Fmain%2Fclojure%2Fcljs%2Fcore.cljc#L2827-L2843

Recommended Change:

  • Do not perform an args array copy before switching on arguments length
  • Perform an args array copy within the variadic dispatch case

Patch forthcoming.



 Comments   
Comment by David Nolen [ 21/May/16 5:03 PM ]

Thanks! Will take a look.

Comment by David Nolen [ 10/Jun/16 1:32 PM ]

This probably needs to be updated in the compiler as well.

Comment by Stephen Brady [ 10/Jun/16 2:43 PM ]

The compiler already isolates the args to array copying behavior in the variadic case. The unnecessary copying is isolated to the defn macro.

These are the only two calls to `emit-arguments-to-array`:

Variadic function: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L743
Multi-arity with variadic case: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/compiler.cljc#L812

Comment by Francis Avila [ 11/Jun/16 8:33 AM ]

From what I can see copy-arguments is only ever used with an immediately-constructed empty array as the dest in the pattern:

`(let [arr# (array)]
  (copy-arguments arr#)
  ;;...
  )

Perhaps it should change to have the exact same behavior as emit-arguments-to-array, i.e. it should take a start index and expand to the name of the destination array. The advantages of this approach are 1) it can preallocate the array to the correct size and 2) your patch no longer needs a slice call--you can avoid allocating two arrays instead of just one. (These two reasons are why I implemented emit-arguments-to-array the way I did.)

I can't think of a way to implement emit-arguments-to-array as a macro without emitting a wrapping js function which messes up the scope, but you could do this:

(defmacro copy-arguments [dest-arr startslice]
  `(loop [i# 0]
     (when (< i# (alength ~dest-arr))
       (aset ~dest-arr i# (aget (js-arguments) (+ i# startslice)))
       (recur (inc i#)))))

And then at the callsite:

`(let [variadic-args-arr# (make-array (- (alength (js-arguments)) ~maxfa))]
  (copy-arguments variadic-args-arr# ~maxfa)
    (let [argseq# (new ^::ana/no-resolve cljs.core/IndexedSeq
                       variadic-args-arr# 0 nil)]
      ;; ...
    ))

However, there are some bugs around arity handling that the above would not solve: CLJS-1678

Comment by Stephen Brady [ 11/Jun/16 8:27 PM ]

Francis, thanks for commenting on this. The patch that I submitted simply moves where/when `copy-arguments` is called. Other than that, I preserved all other aspects of the existing implementation, including how the array is built up and then made into an IndexedSeq. The line diffing in the patch implies that I changed a lot more than I really did. Agreed with your point that `emit-arguments-to-array` is more efficient and precise. Intentionally, I did not try to alter/improve/correct anything in this patch beyond solving the objective in the issue: not unnecessarily copying arguments.

Glad you've reported CLJS-1678 as I've observed this too. This buggy behavior shows up in several places. Beyond what this issue-patch attempts to address, in general, my observation is that we could probably clear out some under-brush that's accumulated as the compiler has matured with regards to arguments handling and code generated for multi-arity and/or variadic functions, apply / applyTo, and implementations of IFn. Seems like there are several opportunities to emit less javascript, create fewer intermediates, and shorten the call chain.

So to reiterate, this patch - despite its superficial appearance - changes very little and just moves the call to copy-arguments to the appropriate place. The benefit is:

For multi-arity functions with no variadic arity, no code for copying the arguments to an array is emitted at all (which in aggregate turns out to be a decent amount). Obviously, at runtime, no array will be created.

For multi-arity functions with a variadic arity, the code for copying the arguments remains but is isolated to the variadic case, and so if the function is called but will dispatch to one of the fixed arities, again, no array will be created.





[CLJS-1677] Requiring [goog] breaks an :advanced build, but the compiler exits successfully Created: 09/Jun/16  Updated: 10/Jun/16

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

Type: Defect Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A single file with the following in it is enough to break a build:

(ns goog-error.core
  (:require [goog]))

with this error

Jun 10, 2016 11:18:03 AM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: ERROR - Duplicate input: file:/Users/danielcompton/.m2/repository/org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.jar!/goog/base.js

Jun 10, 2016 11:18:03 AM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_DUPLICATE_INPUT. Duplicate input: file:/Users/danielcompton/.m2/repository/org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.jar!/goog/base.js at (unknown source) line (unknown line) : (unknown column)

however the ClojureScript compiler exits successfully without throwing an error. The build looks successful, but the file produced doesn't work. Should the ClojureScript compiler throw on these kinds of errors, or otherwise indicate failure?



 Comments   
Comment by David Nolen [ 10/Jun/16 8:27 AM ]

We should look into why the namespace validation that checks where a ns exists or not isn't already catching this case.





[CLJS-1676] Unused local in ObjMap / IKVReduce Created: 09/Jun/16  Updated: 09/Jun/16

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

Type: Defect Priority: Trivial
Reporter: Stuart Hinson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1676.patch    

 Description   

Local len isn't used in ObjMap / IKVReduce

https://github.com/clojure/clojurescript/blob/463de005b81d4a00951188e8b8d38a61d684c18e/src/main/cljs/cljs/core.cljs#L5792






[CLJS-1133] REPL require results in warnings to be emitted twice Created: 17/Mar/15  Updated: 08/Jun/16

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

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

Quick Start Browser REPL with :watch off



 Description   

Run through the Quick Start and go down through to the Browser REPL portion (https://github.com/clojure/clojurescript/wiki/Quick-Start#browser-repl), but exclude the :watch option from repl.clj.

Then further down, where the new symbol is introduced

;; ADDED
(defn foo [a b]
  (+ a b))

instead cause some duplicate symbols to be introduced in order to provoke compiler warnings:

(def a 1)
(def a 1)

(defn foo [a b]
  (+ a b))
(defn foo [a b]
  (+ a b))

Then evaluate the require statement in the tutorial and observe that the warnings are emitted twice:

ClojureScript:cljs.user> (require '[hello-world.core :as hello])
WARNING: a at line 11 is being replaced at line 12 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: foo at line 14 is being replaced at line 16 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: a at line 11 is being replaced at line 12 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
WARNING: foo at line 14 is being replaced at line 16 /Users/mfikes/Desktop/hello_world/src/hello_world/core.cljs
nil





[CLJS-1419] enhance numeric inference, if + number? test on local var should tag local var in the successful branch Created: 12/Aug/15  Updated: 08/Jun/16

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

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


 Comments   
Comment by David Nolen [ 12/Aug/15 6:44 AM ]

One small complication is dealing with and as it has an optimizing case.





[CLJS-349] cljs.compiler: No defmethod for emit-constant clojure.lang.LazySeq Created: 30/Jul/12  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Julien Fantin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-349-Allow-ISeq-to-be-emitted-by-macros-as-a-con.patch     File fixbug349.diff    

 Description   

The cljs compiler errors when trying to emit-constant for a clojure.lang.LazySeq.

Example : https://www.refheap.com/paste/3901

Here syms is defined as a LazySeq on line 3, then on line 7 it is quoted. The error is included in the refheap.

Emitting a cljs.core.list for this type seems to solve the issue.



 Comments   
Comment by David Nolen [ 31/Aug/12 9:27 AM ]

Can you identify precisely where a LazySeq is getting emitted here? A LazySeq is not literal so this seems like a bug in the macro to me. I could be wrong. Thanks!

Comment by Herwig Hochleitner [ 28/Oct/12 9:31 PM ]

The lazy seq seems to be introduced on line 7, the '~syms form

`(let [mappings# (into {} (map-indexed #(identity [%2 %1]) '~syms))

Clojure allows lazy-seqs to be embedded: https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4538

As an aside: The relevant protocol is not literality, but the print-dup multimethod. Do / Should we have print-dup in CLJS?

Comment by Herwig Hochleitner [ 31/Oct/12 10:10 PM ]

Attached patch 0001 doesn't add a case for LazySeq, but folds two cases for PersistentList and Cons into one for ISeq.

Comment by David Nolen [ 19/Nov/13 9:28 PM ]

This approach seems acceptable but this is an old patch can we update for master?





[CLJS-1453] cljs.compiler/load-libs does not preserve user expressed require order Created: 17/Sep/15  Updated: 08/Jun/16

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

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


 Description   

Due to putting the requires into a map the original order is lost. This is a problem primarily when order specific side effects are present in the required namespaces.






[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 871.patch     Text File 871.patch    
Patch: Code and Test

 Description   

Types defined with deftype/defrecord which have a default field will incorrectly return nil with property access. The following example will return nil.

(deftype Foo [default])

(let [foo (Foo. "bar")]
  (.-default foo))


 Comments   
Comment by Joel Holdbrooks [ 13/Oct/14 4:19 PM ]

Patch attached. I should point out that I had to borrow js-reserved from the compiler namespace and the warning message provided hard codes the munged symbol information instead of reusing the compiler's munge fn.

Comment by Joel Holdbrooks [ 13/Oct/14 9:41 PM ]

For the sake of history, I should provide more context to this patch (I'm unable to edit the issue title for some reason). It isn't just .-default it is any field name that is also a JavaScript identifier (eg. public, private, if).

Comment by David Nolen [ 14/Oct/14 5:26 AM ]

Please lift js-reserved and any helpers like munge into the shared namespace cljs.util so that logic an be shared and hard coding avoided. Thanks.

Comment by Joel Holdbrooks [ 14/Oct/14 5:03 PM ]

Are you sure, David? That might make this patch a bit more noisy. If it's not a problem I'm happy to do it.

Comment by David Nolen [ 14/Oct/14 6:06 PM ]

I'm sure, I'd like to avoid this kind of code duping. Cleaner in the end and better moving forward.

Comment by Joel Holdbrooks [ 18/Mar/15 11:43 AM ]

Updated to use new refactorings

Comment by David Nolen [ 18/Mar/15 11:46 AM ]

The warning is not desirable. Instead we should just munge and ensure property access always works.





[CLJS-1286] REPL environment should be able to provide advice if source mapping fails Created: 23/May/15  Updated: 08/Jun/16

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

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


 Description   

For example browser REPL will often need users to supply :host-port, :host, and :asset-path in order to correctly parse files from stacktraces.






[CLJS-1271] Missing warning when assigning namespaces via def Created: 17/May/15  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Sebastian Bensusan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently you can assign a Closure namespace to a var without getting a warning.

Minimal sample:

(ns import-names.core
  (:import [goog debug]))

(def debug goog.debug)


 Comments   
Comment by David Nolen [ 29/May/15 12:30 PM ]

The example case is a bit complected. Besides importing a name that matches a def you are also assigning a google closure namespace to a local. This will likely cause problems on its own. We need more information.

Comment by Sebastian Bensusan [ 29/May/15 12:46 PM ]

We should check that :require ed and :import ed namespaces are not used as values and then warn about it.





[CLJS-1139] Repeated applications of `ns` form at the REPL are not additive Created: 17/Mar/15  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Michael Griffiths Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Quick start guide with Node REPL



 Description   

In a Clojure REPL, it is possible to declare the same namespace again, without existing namespaces aliases being altered or removed:

user=> (ns my-test-ns.core (:require [clojure.string :as string]))
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=> (ns my-test-ns.core)
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=>

ClojureScript REPLs do not behave in the same way:

ClojureScript:cljs.user> (ns my-test-ns.core (:require [clojure.string :as string]))
true
ClojureScript:my-test-ns.core> (def a string/blank?)
#<function clojure$string$blank_QMARK_(s){
return goog.string.isEmptySafe(s);
}>
ClojureScript:my-test-ns.core> (ns my-test-ns.core)
nil
ClojureScript:my-test-ns.core> (def a string/blank?)
WARNING: No such namespace: string, could not locate string.cljs at line 1 <cljs repl>
WARNING: Use of undeclared Var string/blank? at line 1 <cljs repl>
repl:13
throw e__3919__auto__;
      ^
ReferenceError: string is not defined
    at repl:1:109
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:197:16)
    at Socket.<anonymous> ([stdin]:40:25)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)
ClojureScript:my-test-ns.core>





[CLJS-1159] compiled files with warnings that otherwise don't need recompilation will not emit warnings on the next compile Created: 23/Mar/15  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

The aggressive caching approach is odds with warning visibility. It probably makes sense for a compiled file with warnings to always return true for requires-compilation?.






[CLJS-1207] Emit a warning if multiple resources found for a ClojureScript namespace Created: 15/Apr/15  Updated: 08/Jun/16

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

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


 Description   

We should emit a simple warning if a namespace doesn't not appear to be unique on the classpath.






[CLJS-1415] Handling JSDoc param name [x] optional syntax Created: 10/Aug/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: type-check





[CLJS-773] Use unchecked-*-int functions for real 32-bit math Created: 26/Feb/14  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: Francis Avila
Resolution: Unresolved Votes: 0
Labels: numerics
Environment:

r2173



 Description   

Currently the unchecked-* functions and macros simply alias the primitive js operators. It would be nice if the unchecked-*-int family of functions and macros implemented C/Java-like signed int operations with silent overflows (just like in Clojure) using asm.js coersion idioms. This should also allow us to share such code between clojure and clojurescript without worrying about their different numerics.

A use case is that porting hash algorithms from java to clojurescript is trickier and more verbose than it needs to be.



 Comments   
Comment by David Nolen [ 08/May/14 6:43 PM ]

This sounds interesting, would like to see more thoughts on approach, benchmarks etc.

Comment by David Nolen [ 02/Dec/14 5:46 AM ]

Bump, this enhancements sound simple & fine.

Comment by Francis Avila [ 02/Dec/14 1:26 PM ]

I'll have time to do this in about a week. The implementation is straightforward (basically use xor 0 everywhere). The goal is correctness, but I expect performance to be as good as or better than it is now on most platforms. I'm not sure if advanced mode will drop intermediate truncations or what impact this has on performance.

Some higher-level numeric analysis using the asm.js type system is possible but I doubt it's worth it.

Comment by Francis Avila [ 16/Mar/15 11:14 AM ]

I completely forgot about this, sorry. I see you have scheduled it for the "next" release. Are you assigning it as well or will you still accept a patch?

Comment by David Nolen [ 16/Mar/15 11:26 AM ]

Be my guest





[CLJS-794] RegExp flags are being dropped by `string/replace` Created: 09/Apr/14  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Peter Taoussanis Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File CLJS-794.patch    

 Description   

`clojure.string/replace` accepts either a string or pattern argument to match against.

For pattern arguments, the current implementation discards the original RegExp and creates a new one:
`(.replace s (js/RegExp. (.-source match) "g") replacement)`

This is killing any flags on the original pattern (case insensitivity, for example). As a result, things like `(str/replace "Foo" #"(?i)foo" "bar")` currently fail. The result is "Foo", it should be "bar".

Can I submit a patch that'll check for and preserve other (i/m/y) flags?

Thanks



 Comments   
Comment by David Nolen [ 02/Dec/14 5:42 AM ]

A patch is welcome for this. Thanks.

Comment by lvh [ 27/Jul/15 11:55 AM ]

This appears to be identical to CLJS-485, which has a patch (by someone who hasn't signed the CLA yet).

Comment by Jake McCrary [ 04/Feb/16 6:43 PM ]

This patch changes string/replace-all to respect flags that were set on regexp passed as an argument.





[CLJS-1127] validate compiled file written to disk Created: 16/Mar/15  Updated: 08/Jun/16

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

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


 Description   

If we validate the file written to disk then we can catch common error of running multiple build processes and abort.






[CLJS-1222] Sequence of a stateful transducer is producing the wrong answer Created: 24/Apr/15  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Lucas Cavalcanti Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, cljs, collections
Environment:

OSX 10.10.3, java 1.8.0-ea-b124



 Description   

I'm producing more than one element on the 1-arity of the transducer, and sequence is only considering the last one.

Here is the transducer and the tests that fail for sequence:

(defn sliding-window [n]
  (fn [rf]
    (let [a #js []]
      (fn
        ([] (rf))
        ([result]
         (loop [] ;; Here I'm emitting more than one element
           (when (not-empty a)
             (rf result (vec (js->clj a)))
             (.shift a)
             (recur))))
        ([result input]
         (.push a input)
         (if (== n (.-length a))
           (let [v (vec (js->clj a))]
             (.shift a)
             (rf result v))
           result))))))

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))


 Comments   
Comment by Lucas Cavalcanti [ 24/Apr/15 11:18 AM ]

I could make it work by recurring on the result.

([result]
  (loop [res result]
    (if (not-empty a)
      (let [v (vec (js->clj a))]
        (.shift a)
        (recur (rf res v)))
      res)))

even so it's weird that the previous version behaves differently on core.async and sequences in cljs and clj

Comment by David Nolen [ 26/Apr/15 4:04 AM ]

Please demonstrate the problem without core.async. Thanks.

Comment by Lucas Cavalcanti [ 26/Apr/15 7:32 PM ]

Hi,

the last test I posted on the ticket, fails in cljs, but not in clj:

;;This test fails! =(
(deftest sliding-window-in-a-sequence
  (is (= [[5 4 3]
          [4 3 2]
          [3 2 1]
          [2 1]
          [1]]
         (sequence (sliding-window 3) [5 4 3 2 1])))

  (is (= [[2 1]
          [1]]
         (sequence (sliding-window 3) [2 1]))))
Comment by David Nolen [ 27/Apr/15 7:43 AM ]

I've removed the core.async bits from the description to clarify the issue.

Comment by David Nolen [ 10/May/15 2:40 PM ]

The implementation of sliding-window above does not appear to be correct, it doesn't return the result. This ticket needs more information.

Comment by Lucas Cavalcanti [ 10/May/15 3:51 PM ]

As I said on http://dev.clojure.org/jira/browse/CLJS-1222?focusedCommentId=38620&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-38620

changing the 1-arity of the sliding-window to that fixes the transducer.

The point of this ticket now is that the behavior of the same (wrong) transducer in clj (both core.async and sequence) and cljs (core.async) is different than cljs sequence.





[CLJS-1174] Simple warning if a namespace with dashes not found but a file path with dashes exists Created: 27/Mar/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: easy





[CLJS-1485] Error when requiring `goog` namespace in a ns declaration Created: 10/Nov/15  Updated: 08/Jun/16

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

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


 Description   

I wanted to use functions from goog namespace although--as I found out later, I didn't have to because goog is already exists in my namespace. So, I put (:require [goog]) in a ns declaration. Then, when I tried to reload that particular namespace by doing :require :reload in a cljs repl, I got:

Error: Namespace "x.x.x" already declared.

Doing :require :reload again in the cljs repl makes the repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)

I tested the steps below using clojurescript 1.7.145 and 1.7.170.

Here are the steps to reproduce which are taken from clojurescript quickstart-browser repl section:

1. Download the standalone clojurescript 1.7.170 jar https://github.com/clojure/clojurescript/releases/download/r1.7.170/cljs.jar

2. Create a directory hello_world and copy the JAR into that directory, then from inside the hello_world directory:

mkdir -p src/hello_world;touch repl.clj;touch index.html;touch src/hello_world/core.cljs

3. repl.clj content

(require 'cljs.repl)
(require 'cljs.build.api)
(require 'cljs.repl.browser)

(cljs.build.api/build "src"
  {:main 'hello-world.core
   :output-to "out/main.js"
   :verbose true})

(cljs.repl/repl (cljs.repl.browser/repl-env)
  :watch "src"
  :output-dir "out")

4. index.html content

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

5. src/hello_world/core.cljs content

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]))

(defonce conn
  (repl/connect "http://localhost:9000/repl"))

(enable-console-print!)

(println "Hello world!")

(defn foo [a b]
  (+ a b))

6. run clojurescript repl

java -cp cljs.jar:src clojure.main repl.clj

7. Open http://localhost:9000 in browser (I use google chrome). Open javascript console.

8. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)

10. Look the browser javascript console. Nothing new shown.

11. Quit from the repl using :cljs/quit

12. Add [goog] in ns declaration in src/hello_world/core.cljs so that the content of the file becomes:

(ns hello-world.core
  (:require [clojure.browser.repl :as repl]
            [goog]))

(defonce conn
  (repl/connect "http://localhost:9000/repl"))

(enable-console-print!)

(println "Hello world!")

(defn foo [a b]
  (+ a b))

13. Run the clojurescript repl again

java -cp cljs.jar:src clojure.main repl.clj

14. Now refresh the http://localhost:9000 in browser. Make sure the javascript console stays open.

13. enter expression below in the clojurescript repl

(require '[hello-world.core :as hello] :reload)
;;=> nil

it just returns nil

15. See the javascript console, it shows

Uncaught Error: Namespace "hello_world.core" already declared.

16. Executing this expression again (require '[hello-world.core :as hello] :reload) shows nothing new in the browser's javascript console while the clojurescript repl throws

Error: Namespace "cljs.user" already declared.
(NO_SOURCE_FILE)
(out/goog/base.js:273:40)





[CLJS-1237] ns-unmap doesn't work on refers from cljs.core Created: 01/May/15  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: ns-unmap

Attachments: Text File 0001-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch     Text File 0002-CLJS-1237-ns-unmap-adds-to-namespace-s-excludes.patch    
Patch: Code

 Description   

In ClojureScript, using ns-unmap on a symbol from cljs.core doesn't exclude it from the current namespace. Note that both a function and a macro still exist, even after unmapping:

To quit, type: :cljs/quit  
cljs.user=> (ns-unmap 'cljs.user 'when) ;; macro
true  
cljs.user=> (ns-unmap 'cljs.user 'not)  ;; function
true  
cljs.user=> (when 1 2)  
2  
cljs.user=> (not false)  
true  

This differs from the behavior of Clojure's ns-unmap. Note the appropriate errors when attempting to use unmapped symbols:

Clojure 1.7.0-beta1
user=> (ns-unmap 'user 'when) ;; macro
nil
user=> (ns-unmap 'user 'not)  ;; function
nil
user=> (when 1 2)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: when in this context, compiling:(NO_SOURCE_PATH:11:1) 
user=> (not false)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: not in this context, compiling:(NO_SOURCE_PATH:12:1) 

Somehow ClojureScript's ns-unmap needs to add the symbol to the current namespace's :excludes set. Note that the def special form does this already (after it displays a warning).

We have two solutions. 0001 extends the ns form's :merge behavior to support :excludes, and then uses this in ns-unmap. If the enhancement to ns isn't wanted, patch 0002 changes ns-unmap to update :excludes directly.



 Comments   
Comment by David Nolen [ 05/May/15 7:23 AM ]

The second patch is preferred. However it seems the second patch is too permissive. The :excludes logic should only be applied if the symbol identifies a core macro or fn.

Comment by Chouser [ 05/May/15 3:46 PM ]

The ns form's own :refer-clojure :exclude accepts arbitrary symbols and adds them to the namespace's :excludes set, which seems like the same permissiveness problem. Do you want a patch that addresses the permissiveness of both ns and ns-unmap in this ticket, or should such a patch go in a new ticket?

Comment by David Nolen [ 05/May/15 4:08 PM ]

New ticket to fix the bug that :exclude doesn't check the symbol list for cljs.core declared vars, and an updated patch here please.





[CLJS-1315] Warning on Google Closure enum property access with / Created: 18/Jun/15  Updated: 08/Jun/16

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

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


 Description   

Edge case in / usage, EventType/CLICK does not trigger a warning. Foo/bar always means that Foo is a namespace, it cannot be used for the static field access pattern common in Java as there's no reflection information in JavaScript to determine this.






[CLJS-1128] Describe internationalization strategies via Google Closure on the wiki Created: 16/Mar/15  Updated: 08/Jun/16

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation, newbie


 Description   

This can be done via Google Closure defines or via pulling a specific locale. A page should document how this can be done.






[CLJS-994] print a warning when :externs file paths can't be found. Created: 30/Jan/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Crispin Wellington Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: cljs, enhancement, errormsgs, patch,
Environment:

Linux 64bit

java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)


Attachments: Text File clojurescript-extern-missing-warning.patch    
Patch: Code

 Description   

clojurescript silently ignores missing externs files possibly leading a developer to chase their tail.

Presently it can be very confusing using advanced compilation if you have made a mistake in the path name of one of your :externs files. This patch makes the compiler print a warning on stderr so you can quickly determine the cause of the broken advanced compilation output.

As a side effect, when doing a basic lein-cljsbuild a warning is always printed:

```
WARNING: js resource path closure-js/externs does not exist
```

This is because lein-cljsbuild quietly adds this extra path to your :externs listing without you knowing.



 Comments   
Comment by David Nolen [ 31/Jan/15 1:59 PM ]

You need to bind *out* to *err*, or just print to it directly a la cljs.util/debug-prn.

Comment by Crispin Wellington [ 31/Jan/15 7:30 PM ]

I did bind out to err. Check the patch.

Comment by David Nolen [ 01/Feb/15 12:30 PM ]

Crispin, oops sorry you are correct. Thanks.

Comment by David Nolen [ 13/Mar/15 7:33 AM ]

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

Comment by David Nolen [ 14/Mar/15 5:55 AM ]

The solution does not work for cljsbuild. It's unclear why there so much machinery in place over the approach taken for deps.clj.

Comment by David Nolen [ 15/Mar/15 10:37 AM ]

Stalled on this cljsbuild issue https://github.com/emezeske/lein-cljsbuild/issues/383

Comment by Crispin Wellington [ 23/Mar/15 2:50 AM ]

This lein-cljsbuild issue is what made me make it just a warning initially, and not a hard error like raising IllegalArgumentException does. Though I agree it should be a hard error. If we start with a warning, it enables the immediate problem for the developer to be resolved, and leaves a wart that the cljs-build project can then see that need fixing on their end. Then when that end is fixed it could be made a hard error. If cljsbuild is fixed fairly soon then all is well, but if it takes a long time, a warning might be a good first step.





[CLJS-1134] Lift protocols from cljs.closure into cljs.protocols ns Created: 17/Mar/15  Updated: 08/Jun/16

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is task towards presenting a stable API to users without reaching into the implementation namespaces.






[CLJS-1129] :modules tutorial for wiki Created: 16/Mar/15  Updated: 08/Jun/16

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

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation, newbie


 Description   

The documentation is nice but something that walks people through the steps would be nicer.






[CLJS-1412] Add JSDoc type information to individual IFn methods Created: 10/Aug/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: type-check


 Description   

Propagate user supplied docstring type information to the various fn arities so that more code may be checked.






[CLJS-375] loop doesn't seem to preserve tag information as evidenced by extra cljs.core.truth_ calls Created: 06/Sep/12  Updated: 08/Jun/16

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

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





[CLJS-1070] top-level boolean inference does not work Created: 28/Feb/15  Updated: 08/Jun/16

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

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


 Description   

Problem for using boolean Closure defines



 Comments   
Comment by Francis Avila [ 30/Mar/15 12:02 PM ]

I am unsure if this is the same issue, but forms like ^boolean (js/isFinite n) also do not seem to analyze correctly: if, and, and or will still emit a call to truth_.





[CLJS-1125] Simple corrupted compiled file detection Created: 16/Mar/15  Updated: 08/Jun/16

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

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


 Description   

We should include a line at the end of the file that we check for to determine that the file was not corrupted due to either an incomplete write or a clobbered write. It should be be the SHA of the ClojureScript source it was generated from.






[CLJS-434] ClojureScript compiler prepends "self__" to defmulti forms when metadata in form of ^:field. Created: 01/Dec/12  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Andrew Mcveigh Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Mac OS X (10.7), java version "1.6.0_37", leiningen 2 preview 10, cljsbuild 0.2.9.
clojure/clojurescript master 01 December 2012 - 5ac1503



 Description   

Using the def form, with the specific metadata ^:field causes the cljs compiler
to prepend "self__" to the output js form.

The browser (latest chrome/firefox) does not recognize "self__".

Test Case: Tested against master: 5ac1503
-------------

(ns test-def)

(def ^:foo e identity)
e
; test_def.e = cljs.core.identity;
; test_def.e;

(def ^:field f identity)
f
; test_def.f = cljs.core.identity;
; self__.test_def.f;
; Uncaught ReferenceError: self__ is not defined

https://gist.github.com/4185793



 Comments   
Comment by Brandon Bloom [ 01/Dec/12 5:37 PM ]

code tags

Comment by David Nolen [ 20/Jan/13 12:54 AM ]

This one is a bit annoying. We should probably use namespaced keywords internally.





[CLJS-1136] Initial require fails to fully load added symbols Created: 17/Mar/15  Updated: 08/Jun/16

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

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

Quick Start Browser REPL (OS X / Safari)



 Description   

In the Quick Start, a portion runs the user through adding a symbol (a function named foo) and then requiring the namespace and using that symbol. I'm finding that require fails and that I need to add the :reload directive.

To reproduce:

  1. Run through the Quick Start up through the browser REPL section.
  2. Set src/hello_world/core.cljs so that it does not have the foo function defined.
  3. Remove the out directory: rm -rf out
  4. Start up the REPL: rlwrap java -cp cljs.jar:src clojure.main repl.clj
  5. Connect Safari by going to http://localhost:9000
  6. Show the error console in Safari. (You'll see Hello world.)
  7. Run tail -f out/watch.log
  8. Add the foo function that adds a b to src/hello_world/core.cljs and save it.
  9. Observe that watch.log reflects recompilation
  10. Do {{ (require '[hello-world.core :as hello]) }}
  11. Do {{ (hello/foo 2 3) }}

At this point you will get:
TypeError: undefined is not an object (evaluating 'hello_world.core.foo.call')

But:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{foo #'hello-world.core/foo, conn #'hello-world.core/conn}
ClojureScript:cljs.user> (source hello/foo)
(defn foo [a b]
  (+ a b))
nil

Now, if you :reload

ClojureScript:cljs.user> (require '[hello-world.core :as hello] :reload)
nil
ClojureScript:cljs.user> (hello/foo 2 3)
5


 Comments   
Comment by Mike Fikes [ 17/Mar/15 11:30 AM ]

Prior to step 8:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{}

Between steps 9 and 10:

ClojureScript:cljs.user> (ns-interns 'hello-world.core)
{foo #'hello-world.core/foo, conn #'hello-world.core/conn}

My guess: Watching is causing symbols to be interned, but not usable, and this is interfering with require forcing you to include :reload.

Comment by David Nolen [ 22/Mar/15 9:46 AM ]

I'm not sure that this is actually an issue, the browser has already required the namespace, it's the entry point. Thus you really do need a :reload. But the reason you see interned symbols is that the watch process shares the compilation environment with the REPL. It may be the case that with the dramatically improved REPLs the watch option becomes entirely unnecessary and counterintuitive, let's see how it goes.





[CLJS-1238] Setting *main-cli-fn* when using :target :nodejs shouldn't be manditory Created: 01/May/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Minor
Reporter: Jeremy Shoemaker Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File nodejs-main-cli-fn.patch    
Patch: Code

 Description   

Currently, when you use :target :nodejs in the build options for ClojureScript, the resulting code requires you to set *main-cli-fn* to a function.

This prevents someone from writing a library that can be used by JavaScript developers because it forces code execution on require. It also makes writing a CLI tool that can be distributed using NPM less straightforward. I ran into this issue trying to create a Leiningen template for writing CLI tools that could be installed using npm install or npm link. I had a wrapper script to take care of the CLI use-case, and intended to write the ClojureScript module in a more library oriented way, but ran into issues. I could work around this by not using the wrapper script, but it got me thinking about the more general library issue.

I don't see any reason why you should be forced to set *main-cli-fn* and so I'm suggesting making it optional.

Attached is a patch that makes it optional but retains the check for whether the value it is set to is a function in the case where it is set.

This is my first time submitting a change to a project using a git patch and not a pull request, so let me know if I've made the patch wrong.



 Comments   
Comment by Jeremy Shoemaker [ 01/May/15 7:27 PM ]

I just noticed the priority defaulted to "Major". I don't know if I'd say it's major, so feel free to bump it down if that doesn't seem appropriate.

Comment by Ning Sun [ 18/Feb/16 4:08 AM ]

+1.

I was working on a clojurescript library and going to build it as a node library. Currently blocked by this.

Comment by Mike Fikes [ 20/Feb/16 8:07 AM ]

Patch no longer applies.





[CLJS-485] clojure.string/replace ignores regex flags Created: 12/Mar/13  Updated: 08/Jun/16

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

Type: Defect Priority: Minor
Reporter: Esa Virtanen Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: bug, patch, test

Attachments: Text File 0001-Take-regex-flags-m-i-into-account-in-clojure.string-.patch     Text File CLJS-485.patch    
Patch: Code and Test

 Description   

The replace function in namespace clojure.string ignores regex flag provided in the match pattern. For example:

CLJS
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am NOT matched"
CLJ
clojure.string/replace "I am NOT matched" #"(?i)not " "")
=> "I am matched"

The attached patch fixes this by parsing the m and i flags, if set, from the match object, instead of explicitly setting only "g".



 Comments   
Comment by Chas Emerick [ 19/Mar/14 9:29 AM ]

I can confirm the bug. The attached patch applies cleanly, and works as expected.

Esa, sorry for the long delay (this one must have slipped through the cracks)! Could you please submit a contributor's agreement, so that your patch can be merged? More info is here:

http://clojure.org/contributing

Comment by lvh [ 26/Jul/15 5:56 PM ]

I got bit by this bug. Working on figuring out if I can sign that agreement.

Comment by lvh [ 27/Jul/15 11:55 AM ]

This is a duplicate of CLJS-794.

Comment by Jake McCrary [ 04/Feb/16 6:58 PM ]

This patch changes string/replace-all to respect flags that were set on regexp passed as an argument.

I originally attached this to CLJS-794 and then noticed there was this older issue. I was unable to figure out how to edit at ticket to mark the patch as having "Code and Test" so I'm adding it to this issue instead.

I've signed a contributors agreement.

Comment by Mike Fikes [ 04/Feb/16 9:31 PM ]

There is a "sticky" flag y that could be conveyed.

http://www.ecma-international.org/ecma-262/6.0/index.html#sec-get-regexp.prototype.sticky

Comment by Jake McCrary [ 06/Feb/16 10:50 AM ]

Reading a bit more about it and looks like both 'u' and 'y' are newly supported in ECMA6. Is there a way to write tests that exercise this functionality? I've got changes locally to get support of 'y' but felt the need to write a test for it (as its a bit more complicated than simply looking for a flag) and am hung up on having EMCA6 support while running the tests.

I'm actually wondering if having 'u' and 'y' in there is a bit premature. Any guidance on whether adding code for 'u' and 'y' should be done (or removing 'u' from my patch) or testing ClojureScript with ECMAScript 6 support?

Comment by Mike Fikes [ 06/Feb/16 12:51 PM ]

One thought: Whatever this is exercising passes on recent versions of V8, JavaScriptCore, SpiderMonkey, Nashorn, and ChakraCore: https://github.com/clojure/clojurescript/blob/628d957f3ecabf8d26d57665abdef3dea765151e/src/test/cljs/cljs/core_test.cljs#L1472

It does seem tricky to write a robust RegExp clone implementation, and if you do some googling you see people dealing with u and y as special cases. The patch seemed OK to me with respect to this, but I'm not a JavaScript expert.





[CLJS-1141] memoization of js-dependency-index and get-upstream-deps needs knobs Created: 18/Mar/15  Updated: 08/Jun/16

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

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

Attachments: Text File CLJS_1141.patch     Text File CLJS-1141-with-js-dep-caching-latest.patch    

 Description   

knobs should be exposed for more dynamic compilation environments like Figwheel which may desire to add dependencies to the classpath on the fly.



 Comments   
Comment by Bruce Hauman [ 21/Mar/15 3:51 PM ]

A patch that caches upstream dependencies in the compiler env.

Comment by Bruce Hauman [ 21/Mar/15 3:59 PM ]

Actually I'm going to submit another patch that includes the memoize calls in js-deps.

Comment by Bruce Hauman [ 28/Mar/15 12:50 PM ]

New patch that moves cljs.js-deps memoization to current env/compiler as well as get-upstream-deps.

Unfortunately there is a circular dep between cljs.env and cljs.js-deps, if we want to cache in env/compiler. I overcame this with a resolve.

Compile performance is either completely unchanged or slightly improved based on several test runs.

Comment by Bruce Hauman [ 28/Mar/15 2:22 PM ]

Hold off on this. Its not behaving as expected. Doesn't seem to be caching in certain situations.

Comment by David Nolen [ 28/Mar/15 2:26 PM ]

Thanks for the update. This will definitely not land until after the pending REPL/piggieback release anyhow.

Comment by Bruce Hauman [ 28/Mar/15 2:44 PM ]

Yeah there is an obvious bug and a subtle one. Hopefully will finish it up soonish.

Comment by Bruce Hauman [ 28/Mar/15 3:43 PM ]

Alright, this latest patch works. There was a subtle memoizing nil value bug.





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 08/Jun/16

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

Type: Defect Priority: Major
Reporter: Kevin Marolt Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When a this-as expression gets put in a "scoping function", e.g. in a let-binding, the value bound via this-as refers to the scoping function, and not to the outer scope.

Example:

(def foo
  (js-obj
    "bar" "baz"
    "getBarRight" (fn [] (this-as self (.-bar self)))
    "getBarWrong" (fn []
                    (let [bar (this-as self (.-bar self))]
                      bar))))
     
(.log js/console (.getBarRight foo)) ;; => "baz"
(.log js/console (.getBarWrong foo)) ;; => undefined

Whereas foo.getBarRight expands to something like

function() {
  var self = this; // this refers to foo
  return self.bar; // returns "bar"
}

foo.getBarWrong on the other hand expands to

function() {
  var bar = function() {
    var self = this; // this refers to enclosing function
    return self.bar; // returns undefined
  }();
  return bar; // returns undefined
}





[CLJS-1443] ES6 Module Processing at individual :foreign-lib spec Created: 09/Sep/15  Updated: 08/Jun/16

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

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


 Description   

ES6 module processing could probably benefit from processing at the individual :foreign-lib spec. Brings up questions wrt. source maps and merged source maps when applying other optimization settings.






[CLJS-1328] Support defrecord reader tags Created: 04/Jul/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reader, readertags


 Description   

Currently, defrecord instances print similar to how they do in clojure

> (pr-str (garden.units/px 5))
#garden.types.CSSUnit{:unit :px, :magnitude 5}

This representation cannot be read by the compiler, nor at runtime by cljs.reader/read-string

> #garden.types.CSSUnit{:unit :px, :magnitude 5}
clojure.lang.ExceptionInfo: garden.types.CSSUnit {:type :reader-exception, :line 1, :column 22, :file "NO_SOURCE_FILE"}
...
> (cljs.reader/read-string "#garden.types.CSSUnit{:unit :px, :magnitude 5}")
#<Error: Could not find tag parser for garden.types.CSSUnit in ("inst" "uuid" "queue" "js")>
...

Analysis

The two requirements - using record literals in cljs source code and supporting runtime reading - can be addressed by using the analyzer to find defrecords and registering them with the two respective reader libraries.

Record literals

Since clojurescript reads and compiles a file at a time, clojure's behavior for literals is hard to exactly mimic. That is, to be able to use the literal in the same file where the record is defined.
A reasonable compromise might be to update the record tag table after each file has been analyzed. Thus the literal form of a record could be used only in requiring files.

EDIT: Record literals can also go into the constant pool

cljs.reader

To play well with minification, the ^:export annotation could be reused on defrecords, to publish the corresponding reader tag to cljs.reader.

Related Tickets



 Comments   
Comment by David Nolen [ 08/Jul/15 12:00 PM ]

It's preferred that we avoid exporting. Instead we can adopt the same approach as the constant literal optimization for keywords under advanced optimizations. We can make a lookup table (which won't pollute the global namespace like exporting does) which maps a string to its type.

I'm all for this enhancement.





[CLJS-1194] data_readers.cljc Created: 10/Apr/15  Updated: 08/Jun/16

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

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


 Description   

Now that conditional reading has landed we can implement support for data_readers.cljc to get both compile time and runtime support.



 Comments   
Comment by David Nolen [ 10/Apr/15 7:45 PM ]

This needs http://dev.clojure.org/jira/browse/CLJ-1699 to be useful.

Comment by Nikita Prokopov [ 19/May/15 7:58 AM ]

CLJ-1699 has landed.

Right now CLJS tries to compile data_readers.cljc as a regular source code file:

Exception in thread "main" java.lang.AssertionError: No ns form found in src/data_readers.cljc, compiling:(/private/var/folders/0h/9vv4g3d955l6ctwwl4k9xjy40000gn/T/form-init3533791126017861878.clj:1:125)
	at clojure.lang.Compiler.load(Compiler.java:7249)
	at clojure.lang.Compiler.loadFile(Compiler.java:7175)
	at clojure.main$load_script.invoke(main.clj:275)
	at clojure.main$init_opt.invoke(main.clj:280)
	at clojure.main$initialize.invoke(main.clj:308)
	at clojure.main$null_opt.invoke(main.clj:343)
	at clojure.main$main.doInvoke(main.clj:421)
	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)
Comment by David Nolen [ 19/May/15 8:53 AM ]

This should be addressed first: http://dev.clojure.org/jira/browse/CLJS-1277





[CLJS-1402] Source Mapping Closure Error Logger Created: 08/Aug/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: type-check


 Description   

Current error reports generated by Google Closure point back to the generated JavaScript sources. For JavaScript source that originated from ClojureScript we should generated source mapped reports.






[CLJS-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 08/Jun/16

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

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: bug
Environment:

Originally found with [org.clojure/clojurescript "0.0-2496"]
Still reproducible with the latest cljsc (b5e9a5116259fc9f201bee4b9c6564f35306f9a5)



 Description   

Here is a minimal test case that produces the invalid Javascript:

(defn f []
  (let [a 0]
    ^{"meta" "data"}
    (fn [] true)))

The compiled Javascript includes the invalid token sequence "return return". (Per Chrome: Uncaught SyntaxError: Unexpected token return)

The problem does not occur if the metadata applies to a map literal instead of a function literal.
The problem only occurs when the function and metadata are inside of a let.



 Comments   
Comment by Bobby Eickhoff [ 07/Jan/15 9:45 PM ]

I forgot to try with-meta. Using with-meta does not produce this syntax error, so it's only a problem with the reader macro for metadata.

Comment by David Nolen [ 08/Jan/15 7:41 AM ]

Any quick thoughts about this one Nicola? Quite possibly a compiler issue on the CLJS side.

Comment by Nicola Mometto [ 08/Jan/15 8:07 AM ]

David, I understand why this happens but I don't know enough about how cljs's js emission to propose a fix.
The issue is that with this commit: https://github.com/clojure/clojurescript/commit/d54defd32d6c5ffcf6b0698072184fe8ccecc93a the following scenario is possible:

{:op :meta
 :env {:context :return}
 :expr {:op :fn
        :env {:context :expr}
        :methods [{:op :fn-method 
                   :env {:context :return} ..}]
        ..}
 ..}

i.e. analyze-wrap-meta changes the context of the :fn node to :expr but keeps the context of the :fn-methods to :return.

This causes both
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L575-L576
and
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L488 (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L233)

to be true and emit a "return".

Comment by David Nolen [ 06/May/15 7:15 PM ]

Hrm, it appears analyze-wrap-meta may need to defer to a helper to change the :context of the given AST node.

Comment by Herwig Hochleitner [ 11/Dec/15 10:52 AM ]

I just randomly ran into this, when upgrading an old project. There is also a duplicate already: http://dev.clojure.org/jira/browse/CLJS-1482

Comment by Jonathan Chu [ 28/Jan/16 6:19 PM ]

This issue occurs for me even without a let.

(fn []
  ^{"meta" "data"}
  (fn [] true))

gives me

#object[SyntaxError SyntaxError: Unexpected token return]




[CLJS-1300] REPLs do no write out updated deps.js when compiling files Created: 05/Jun/15  Updated: 08/Jun/16

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

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

Attachments: Text File cljs-1300.patch    
Patch: Code

 Description   

For example a user may edit a file including a new dependency. This will work at the REPL but if a browser refresh is made the emitted goog.require will fail due to the initial deps.js file being stale.



 Comments   
Comment by ewen grosjean [ 05/Dec/15 4:15 PM ]

load-file is broken into 4 sub-functions:
repl-compile-cljs: compile the cljs file beeing loaded
repl-cljs-on-disk: ensures all dependencies are on disk
refresh-cljs-deps: refreshes the cljs_deps.js file
repl-eval-compiled: eval the compiled file

Comment by David Nolen [ 05/Dec/15 9:02 PM ]

Thanks will review.

Comment by Mike Fikes [ 31/Jan/16 3:25 PM ]

cljs-1300.patch no longer applies on master





[CLJS-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 08/Jun/16

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

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


 Description   

Instead of forcing users to refresh browser and lose application state, the browser REPL should poll once a second to connect if connection is unreachable for some reason.



 Comments   
Comment by David Nolen [ 21/Mar/15 8:56 PM ]

This is firmly a major nice-to-have, but not a blocker.





[CLJS-1373] Generalize CLJS-1324, check invokes of all IFn implementors Created: 28/Jul/15  Updated: 08/Jun/16

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

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


 Description   

We currently track all IFn implementors but in order to do arity checking of statically analyzeable invokes of keywords, vector, etc. we need to do a bit more. extend-type should update the type in the compiler state with :method-params :max-fixed-arity and :variadic. Then we can just reuse the existing checks in cljs.analyzer/parse-invoke.






[CLJS-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 08/Jun/16

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

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


 Description   

We need to recur on the first segment passing an new additional argument to resolve-var indicating that we should not try to resolve in the current namespace and instead warn.






[CLJS-1277] relax requirement that files must declare a namespace, default to cljs.user Created: 19/May/15  Updated: 08/Jun/16

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

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


 Description   

This aligns better with Clojure itself supports.



 Comments   
Comment by David Nolen [ 14/Jun/15 10:30 AM ]

There are a few hurdles in order to make progress on this ticket. The first is that in order to be useful something like require etc. outside the ns needs to be supported in order to be useful.

Comment by Jonathan Boston [ 18/Jul/15 12:17 PM ]

Needs http://dev.clojure.org/jira/browse/CLJS-1346 to be useful.





[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 08/Jun/16

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

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

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    

 Description   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.



 Comments   
Comment by Samuel Miller [ 10/Aug/15 10:06 PM ]

Took some time to look at this issue. Originally thought "Do what loop/recur does" but that does not take into account multi-arity. It seems like maybe the best option is to somehow use the second pass of the analyze(analyze-fn-methods-pass2). The entire information about the function is present and the warning section of the code gets triggered but because of no-warn is ignored. Any other ideas for a solution to this?

Comment by Samuel Miller [ 14/Nov/15 7:47 PM ]

So I am looking for feed back on this patch and I will try to explain the reasoning for each section.

The issue is that a function only knows about it's arity after it has been parsed once.
So we need to check arity issues on the second pass

First off, added two new variables.
-activate-second-pass-warnings:Boolean Basically if you want to have second-pass warnings turned on
-second-pass-cljs-warnings:Set Right now we only have :fn-arity but I figure might as well make it generic.

So first up if the modifications to the analyze-fn-methods-pass2 function.
Instead of using no-warn marco here we have some new functionality.
The goal is to turn everything off except the second-pass warnings

So if activate-second-pass-warnings is false just use no-warn else it will use the new section of code.

The default-warning-handler was also modified. After checking if a warning is on, it checks if the warning is a second-pass warning and
if that warning can now be activated. If activate-second-pass-warnings is false AND a warning is still on that implies it is a second pass warning
in the second pass so we activate it.

Also I tried to keep all modifications in cljs.analyzer.

Originally I had the cljs-warnings :fn-arity to false and it would only be turned on in the second pass.
However the repl section just sets everything to true (and turns off select parts like ns errors).
So I decided to not touch those sections and instead keep how other files interface with the analyzer the same.

Comment by Samuel Miller [ 16/Nov/15 10:58 PM ]

Just realized that I have the patch marked as .md instead of .patch





[CLJS-1461] Convert analyzer to conform to tools.analyzer's spec Created: 28/Sep/15  Updated: 08/Jun/16

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

Type: Enhancement Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Ambrose Bonnaire-Sergeant
Resolution: Unresolved Votes: 4
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.






[CLJS-1501] Add :parallel-build support to REPLs Created: 05/Dec/15  Updated: 08/Jun/16

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

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


 Description   

The :parallel-build option does not currently work in REPLs due to the implementation of cljs.repl/load-namespace






[CLJS-1658] implements? may report false positives Created: 01/Jun/16  Updated: 02/Jun/16

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File implements-false-positives.patch     Text File implements-false-positives-with-sentinel.patch    
Patch: Code

 Description   

The cljs.core/implements? checks whether a protocol is implemented via a property on the tested object. When implementing the protocol this property will be set to true.

// the implementation
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = true;

// the check (implements? TheProtocol x)
((!((x == null)))?(((false) || (x.protocol$check$TheProtocol$))?true:false):false)

// only the relevant bit
x.protocol$check$TheProtocol$

This works fine under :none but :advanced may rename this to something like x.A. If you now try to check (implements? TheProtocol js/window) for example it may (or may not) report a false positive. For larger projects the likelyhood of creating a false positives goes way up since window contains all the variables created by the advanced compiled js.

The attached patch changes the patch the emitted code to check x.protocol$check$TheProtocol$ === true which reduces the chance of a false positive by a lot. We might chose another sentinel value instead to reduce the chance some more, this seems good enough though.



 Comments   
Comment by Thomas Heller [ 01/Jun/16 2:13 PM ]

Implemented the same change for cljs.core/satisfies?.

Comment by Thomas Heller [ 01/Jun/16 2:18 PM ]

Benchmark on my MBP indicate that the change comes with a slight performance cost.

// v8 with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 10 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 27 msecs

//v8 without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 8 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 19 msecs
Comment by Thomas Heller [ 01/Jun/16 2:28 PM ]
// spidermonkey with patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 68 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 146 msecs

// spidermonkey without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 63 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 149 msecs
Comment by Thomas Heller [ 01/Jun/16 2:35 PM ]

JavascriptCore

// jsc with path
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 7 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 14 msecs

// jsc without patch
;;; satisfies?
[coll (list 1 2 3)], (satisfies? ISeq coll), 1000000 runs, 6 msecs
[coll [1 2 3]], (satisfies? ISeq coll), 1000000 runs, 13 msecs
Comment by Thomas Heller [ 02/Jun/16 3:40 AM ]

Added a second patch that uses a sentinel value as the protocol marker. Currently just a plain empty javascript object.

Running the benchmarks shows no real difference to just checking "true".

I also tried using a number or string as the sentinel value but could not find any significant difference either.

// emitted code on protocol impls
protocol.check.Dummy.prototype.protocol$check$TheProtocol$ = cljs.core.PROTOCOL_SENTINEL;

// the check (identical?)
cljs.core.PROTOCOL_SENTINEL === x.protocol$check$TheProtocol$




[CLJS-1593] Self-host: Munged minus macro Created: 25/Feb/16  Updated: 28/May/16

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

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: bootstrap


 Description   

In bootstrap, the macro form of cljs.core/- is evidently available as _ so, for example

(_ 7 3)
works.

Repro:

cljs.user=> (require 'cljs.js)
nil
cljs.user=> (cljs.js/eval-str (cljs.js/empty-state)
  "(_ 7 3)" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value 4}





[CLJS-1651] Self-host: Cannot replace core macro-function Created: 28/May/16  Updated: 28/May/16

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

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


 Description   

Replace double to multiply by two in self host and you will see that operator-position resolution chooses the core macro:

ClojureScript Node.js REPL server listening on 54425
To quit, type: :cljs/quit
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (def st (cljs.js/empty-state))
#'cljs.user/st
cljs.user=> (cljs.js/eval-str st "(defn double [x] (* 2 x))" nil {:eval cljs.js/js-eval :context :expr} identity)
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1
{:ns cljs.user, :value #object[cljs$user$double "function cljs$user$double(x){
return ((2) * x);
}"]}
cljs.user=> (cljs.js/eval-str st "[(double 3) (apply double [3])]" nil {:eval cljs.js/js-eval :context :expr} identity)
{:ns cljs.user, :value [3 6]}

The correct result above would be [6 6].






[CLJS-1644] Recursive protocol method for a record never terminates Created: 23/May/16  Updated: 26/May/16

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.





[CLJS-1646] Internal compiler error when importing Showdown via CommonJS Created: 24/May/16  Updated: 26/May/16

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





[CLJS-1572] REPL doesn't give error for expressions with too many right parentheses. Created: 15/Feb/16  Updated: 24/May/16

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

Type: Defect Priority: Minor
Reporter: J David Eisenberg Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: repl
Environment:

Fedora 23, java version "1.8.0_40", javac 1.8.0_40, clojure 1.7.0


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

 Description   

I was expecting an error message from this; using [org.clojure/clojurescript "1.7.228"]; the Clojure REPL does produce an error.

To quit, type: :cljs/quit
cljs.user=> (+ 3 5)))))
8


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

A suggestion on a strategy to fix this: Make the ClojureScript REPL sequentially process all of the forms it can read on a line, just like the Clojure REPL does:

user=> 3 (+ 3 5) 7
3
8
7

If this is done, then the fix for this ticket will fall out “for free” and the ClojureScript REPL will error when it hits a form that appears to start with ).

Comment by Mike Fikes [ 21/Feb/16 4:01 PM ]

The REPL code is very close to working the way mentioned in the previous comment. It currently does not only because this line

https://github.com/clojure/clojurescript/blob/c59e957f6230c07e7a228070dd8eb393d5b8ce40/src/main/clojure/cljs/repl.cljc#L100

invokes code that causes a new PushbackReader to wrap things (discarding things):

https://github.com/clojure/clojurescript/blob/c59e957f6230c07e7a228070dd8eb393d5b8ce40/src/main/clojure/cljs/repl.cljc#L773-L775

If you either let the PushbackReader once and let that reader fn close over it, or otherwise comment out things so that a new PushbackReader is not created for each loop / recur, you will see that the code behaves as suggested in the previous comment, having the desired effect.

The only thing I see that would need to be additionally sorted out with such a patch is being a little more clever about when need-prompt evaluates to true, etc. (otherwise polishing thing so there are no missed corner cases).

Comment by Mike Fikes [ 21/Feb/16 11:02 PM ]

Attached a patch that, in essence makes the ClojureScript REPL behave like the Clojure REPL with respect to multiple items on a line and with respect to detecting malformed input. The patch is fairly straightforward, but could use some testing. I've tried things like

cljs.user=> 3_    ; where _ here is a space

cljs.user=> 3 4 5

cljs.user=> 3)

cljs.user=> 3))

cljs.user=> 3 [4
5]

cljs.user=> (let [x 1]
(+ 1 "a"))         ;; testing to make sure line numbers are right

All the above is looking good to me.

Here is the commit comment:

Take extra care to preserve the state of in so that anything beyond
the first form remains for reading. This fundamentally makes the
ClojureScript REPL behave like the Clojure REPL. In particular, it
allows entering multiple forms on a single line (which will be evaluated
serially). It also means that if malformed input lies beyond the initial
form, it will be read and will cause an exception (just like in the
Clojure REPL).

The bulk of the complexity in this commit has to do with the case where
a new line-numbering reader is established, so that errors in forms
can be associated with line numbers, starting with line 1 being the
first line of the form. This requires a little extra handling because
the source-logging-push-back-reader introduces an extra 1-character
buffer which must be transferred back to the original (pre-bound) in,
otherwise things like an unmatched extra paren right after a well-formed
form won't be detected (as the paren would be in the 1-char buffer and
discarded.)

Also, a Java PushbackReader needs to be eliminated, as it causes things
to fail to behave like the Clojure REPL.

Comment by Mike Fikes [ 21/Feb/16 11:14 PM ]

Note that one extremely useful thing this patch enables is pasting of multiple forms into a ClojureScript REPL!

This fails if pasted using the current cljs.jar, but works with the patch applied:

(def a 1)

(def b 2)

(def c (+ a b))

c
Comment by Mike Fikes [ 24/May/16 3:19 PM ]

I tested this with Figwheel [figwheel-sidecar "0.5.0-6"] and it worked properly evaluating multiple forms on a single line, evaluating pasted forms (as in the previous comment), and it properly indicates Unmatched delimiter ) for the case in the description.





[CLJS-1639] Invalid file path for cljs.core.load_file on windows Created: 13/May/16  Updated: 21/May/16

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

Type: Defect Priority: Minor
Reporter: Jay Lee Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug
Environment:

Windows 10, CLJS 1.8.51



 Description   

When I tried to build reagent based app with nodejs target,
I got an invalid file path generated case which is basically loading external javascript file.
I captured the image as following:

At line 4, the path should have double backward-slash on windows.

I has built a CLJS app which is based on reagent framework with nodejs target. The build environment is somewhat strange but I have a case to use it. Here is a reproduce steps.

  1. Open command prompt on windows 10 and execute command as following:
    lein new figwheel sample00 – --reagent
  2. Open project.clj file and update one of dependencies:
    project.clj
    ;; ...
    :dependencies [[org.clojure/clojure "1.8.0"]
                     [org.clojure/clojurescript "1.8.51"]
                     [org.clojure/core.async "0.2.374"
                      :exclusions [org.clojure/tools.reader]]
    
                     [reagent "0.6.0-alpha" :exclusions [cljsjs/react]]
                     [cljsjs/react-with-addons "0.14.3-0"]
                     ]
    
    ;; ...
    :cljsbuild {:builds [{:id "dev"
                          ;; ... 
                          :compiler {
                            ;; ...
                            :target :nodejs
                          }
                         }]
               }
    ;; ...
  3. Build with lein cljsbuild once dev
  4. Open <project_root>\resources\public\js\compiled\out\reagent\impl\util.js
  5. At line number 4 in my environment, the generated code is
    cljs.core.load_file("resources\public\js\compiled\out\react-with-addons.inc.js");
    However I believe the correct path string should be cljs.core.load_file("resources\\public\\js\\compiled\\out
    react-with-addons.inc.js");
    .

Backward-slash needs to be double on Windows env.
When I lunched doo test command with nodejs target, it complained about the given path cannot be loaded.

Thanks.



 Comments   
Comment by David Nolen [ 21/May/16 5:07 PM ]

This ticket is in danger of being closed. The ticket should demonstrate a reproducible bug without relying on any 3rd party tools or libraries. No Leiningen, Figwheel, or Reagent. Please demonstrate the Windows issue with only ClojureScript.

Thanks.





[CLJS-1643] Emit more informative error when emitting a type which has no emit multimethod case Created: 21/May/16  Updated: 21/May/16

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

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


 Description   

Users may accidentally splice in Clojure functions and other things in macros that work in Clojure which cannot work in ClojureScript. We may want to include a default warning for the most common/likely error cases.






[CLJS-1628] Make instances of js/Symbol printable Created: 20/Apr/16  Updated: 21/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Roman Scherer Assignee: Roman Scherer
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

At the moment instances of js/Symbol are not printable. Trying to
print them or enter them in the REPL causes an exception.

cljs.user> (.for js/Symbol "react.element")
#object[TypeError TypeError: Cannot convert a Symbol value to a string]

Symbols are supported in all major browsers, except Internet Explorer and Nashorn.
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol



 Comments   
Comment by Roman Scherer [ 20/Apr/16 10:23 AM ]

The attached patch adds an additional clause to `pr-writer-impl` and
implements the printing of Symbol instances, in the same way as it is
done for other JavaScript objects. Here's an example of a printed
js/Symbol:

(.for js/Symbol "react.element")
;;=> #object[Symbol "react.element"]

@david: Regarding your point about shims, do you think the
implementation of `js-symbol-defined?`, which was used for the ES6
iterator support, is enough for this patch? I'm not too familiar with
JavaScript and not sure if this already addressed the "shim" issue.

Another thing I stumbled upon is, that my test currently generates a
compiler warning when using the default compiler options. The warning
is generated when compiling the following ClojureScript form:

(.for js/Symbol "react.element")

The following snippet shows the warning and the generated code from my
test:

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.

try{var values__13328__auto__ = (function (){var x__6628__auto__ = cljs.core.pr_str.cljs$core$IFn$_invoke$arity$variadic(cljs.core.array_seq([Symbol.for("react.element")], 0));

I think this has nothing to do with this patch, but with the emitted
code not being legal Ecmascript 3, since "for" is a reserved word.

The warning goes away when changing the :language-in option to
something newer than Ecmascript 3, or doing something like this:

((gobj/get js/Symbol "for") "react.element")

So, the questions is: Should the ClojureScript compiler handle those
reserved words when they appear in a function call or property lookup?

If that's the case I would leave the warning in that patch, and open
another issue for this new problem.

What do you think?

Roman

Comment by Roman Scherer [ 20/Apr/16 11:33 AM ]

Ok, given advanced mode and renaming of symbols I think there's not much the ClojureScript compiler can do about this.
I think I'll change the test to use ((gobj/get js/Symbol "for") "react.element") to remove the warning.
https://github.com/google/closure-compiler/wiki/FAQ#i-get-invalid-property-id-errors-but-it-works-on-firefox

Comment by David Nolen [ 20/Apr/16 2:21 PM ]

ECMAScript 3 is not an issue for people who are ok with outputting ECMAScript 5. The goog.typeOf of check on "symbol" does not seem like it would work with Symbol shims, have you confirmed?

Comment by Roman Scherer [ 21/Apr/16 10:57 AM ]

David, yes you are right about goog.typeOf, it returns "object" when
js/Symbol is implemented via shims. I updated the patch and tested it
against the following shim implementation in Nashorn so far.

Start a Nashorn REPL.

./script/nashornrepljs

js/Symbol does not exist in Nashorn yet.

(exists? js/Symbol)
;;=> false

Load core-js shim into Nashorn.

(js/load "https://raw.githubusercontent.com/zloirock/core-js/master/client/shim.min.js")
(print (.for js/Symbol "x"))
;;=> #object[Symbol "x"]

I gave up on this shim implementation because I got an error when
trying to load the minified js into Nashorn.

https://github.com/medikoo/es6-symbol

This is another shim implementation, but it only enhances js/Symbol
when it already exists, so not useful in the context of Nashorn.

https://github.com/paulmillr/es6-shim

Comment by Roman Scherer [ 21/Apr/16 2:07 PM ]

Which implementations do support js/Symbol?

JavaScriptCore, NodeJS, SpiderMonkey, Chrome and Firefox support
js/Symbol. Evaluating (instance? js/Symbol (.for js/Symbol "x")) on
those engines returns false. On all those implementations symbols
generated via (.for js/Symbol "x") are primitive values and can be
identified via typeof. See the "Using the typeof operator with
symbols" section in [1].

Nashorn and Rhino do not support js/Symbol.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 2:27 PM ]

Looking around I found that the exception caused by printing symbols
happens because the str [1] function uses implicit string conversion
to convert it's argument into a string. This is explained in the
"Coercing a symbol to string" section of [2]. One way to solve the
problem is to use (.toString x) instead.

(.join #js [(.for js/Symbol "x")] "")
;;=> TypeError: Symbol is not a constructor

(.toString (.for js/Symbol "x"))
;;=> "Symbol(x)"

[2] https://github.com/clojure/clojurescript/blob/master/src/main/cljs/cljs/core.cljs#L2695
[2] http://www.2ality.com/2014/12/es6-symbols.html

Comment by Roman Scherer [ 21/Apr/16 2:38 PM ]

(= (goog/typeOf x) "symbol") vs (instance? js/Symbol x)

Symbols are primitive types and must be constructed via
Symbol("x"). They can't be constructed via new Symbol("x"), trying
to do so raises a TypeError. This is explained in more detail in the
"Safety checks" section of [1]. The "Using the typeof operator with
symbols" section of [2] explains how to identify symbols. It has to be
done via typeof, because symbols are primitive types and not
instances of Symbol.

[1] http://www.2ality.com/2014/12/es6-symbols.html
[2] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Comment by Roman Scherer [ 21/Apr/16 3:02 PM ]

I think we have 2 options now to fix exceptions when printing symbols:

  • Change str to handle symbols as well.
  • Implement custom printing for symbols.

I think I would go for the first option. I think calling str on a
symbol can't be seen as implicitly coercing a symbol to a string, I
would say this is as explicit as it can get and we don't have to raise
an exception in this case. I think str is basically the Clojure
equivalent to toString in JavaScript, at least this is what the out
of date doc string of str says.

The file CLJS-1628-fix-str has an implementation of the first option.

Comment by Francis Avila [ 21/Apr/16 3:35 PM ]

Changing str to use .toString is not possible because of CLJS-847: there are unconfirmable reports that .toString() causes TypeError on Safari 6.0.5.

This problem has spilled over into the unresolved CLJS-890. This is a long thread, so see my recap comment, my failed efforts to reproduce the bug and my proposed final patch, which can be easily extended for the symbol case with a new "symbol" clause in the case statement.

Comment by Roman Scherer [ 21/Apr/16 4:20 PM ]

Thanks Francis, I'll take a look at this discussion.

Comment by Roman Scherer [ 21/Apr/16 5:12 PM ]

Since one can still make so called Symbol wrapper objects via the
Object() function [1], I added one more test case for this:

(js/Object (.for js/Symbol "x"))

Also the updated patch defines the test case only if js/Symbol exists?
in the JavaScript engine.

[1] https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol





[CLJS-1631] The str function should handle JavaScript symbols Created: 21/Apr/16  Updated: 21/Apr/16

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

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


 Description   

The str function should handle primitive JavaScript symbols produced by Symbol.for. At the moment the str function raises an exception, because it runs into some JavaScript safety checks by using implicit string coersion via (.join #js [x] "").
More info on the safety check here: http://www.2ality.com/2014/12/es6-symbols.html
This ticket is also related to:

(def x (.for js/Symbol "x"))
(str x)
TypeError: Cannot convert a Symbol value to a string
    at Array.join (native)
    at Function.cljs.core.str.cljs$core$IFn$_invoke$arity$1 (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9561:12)
    at Object.cljs$core$str [as str] (/home/roman/workspace/clojurescript/.cljs_node_repl/cljs/core.js:9543:22)
    at repl:1:100
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:54:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:228:14)
    at Socket.<anonymous> ([stdin]:40:25)

Calling the toString method on a symbol directly works

(.toString x)
;;=> "Symbol(x)"





[CLJS-1519] Collection invoke errors off by 1 Created: 22/Dec/15  Updated: 19/Apr/16

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

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Mike Jackson
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

Runtime collection invokes will report arity that is off by one. This is because we use the generic function arity dispatching logic which doesn't account for the 1st self argument.



 Comments   
Comment by Mike Jackson [ 15/Apr/16 1:02 AM ]

Hey David,

Can I pick this one up? I'm a first time contributor and I wouldn't mind using this to get a lay of the land. I've already signed the Contributor Agreement.

Cheers

Comment by David Nolen [ 19/Apr/16 2:12 PM ]

Mike, I've updated your permissions. Please assign the ticket to yourself. Thanks!

Comment by Mike Jackson [ 19/Apr/16 2:42 PM ]

Awesome, thanks. Looking forward to it.





[CLJS-1625] Clojurescript macros used in named function are expanded two times because the analyzer performs a two pass analysis when analyzing named functions Created: 16/Apr/16  Updated: 19/Apr/16

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

 Comments   
Comment by Ewen Grosjean [ 16/Apr/16 9:41 AM ]

During the first analysis of named functions, only the function definition is analyzed in order to know its function-ness/arity. Its body is only analyzed during the second pass.

Comment by Kevin Downey [ 17/Apr/16 12:09 AM ]

http://dev.clojure.org/jira/browse/CLJS-1617 seems to add a similar issue





[CLJS-1604] Self-host: cljs.js/compile-str causes a javascript error Created: 19/Mar/16  Updated: 14/Apr/16

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap, bug


 Description   

When requiring `cljs.js` and calling `cljs.js/compile-str` with `:optimizations :advanced`
I get the following error in the browser:
"Uncaught TypeError: Cannot set property 'rd' of undefined"

Steps to reproduce:

1. Make a directory
2. Copy shipping cljs.jar into the directory
3. Make an index.html, src/hello_world/core.cljs, and build.clj file with contents as below.
4. java -cp cljs.jar:src clojure.main build.clj
5. Open index.html with Chrome and view the JavaScriptConsole in Chrome.

index.html:

<html>
<body>
<script type="text/javascript" src="out/main.js"></script>
</body>
</html>
src/hello_world/core.cljs:
(ns hello-world.core
(:require [cljs.js :as cljs]))

(set! (.. js/window -cljs -user) #js {})

(cljs/compile-str (cljs/empty-state) "" indentity)

build.clj:

(require 'cljs.build.api)

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

(System/exit 0)



 Comments   
Comment by Yehonathan Sharvit [ 19/Mar/16 5:31 PM ]

I need to fix the title of the issue: "Self-host: in advanced compilation - cljs.js/compile-str causes a javascript error"

Comment by Mike Fikes [ 30/Mar/16 11:14 PM ]

You can only use up to :optimizations :simple with self-host. See https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting#production-builds

Discussion: One rationale for this is that the emitted code, in order to be executable, needs access to non-Closure-munged/DCEd symbols from the standard ClojureScript lib. Perhaps this limitation need only exist for eval-str, (while not for compile-str, analyze-str, etc.)

Comment by Mike Fikes [ 14/Apr/16 7:02 AM ]

I'd recommend closing this as declined (no plans exist to support self-host with :advanced).





[CLJS-1622] `with-redefs` can cause `&` argument to be assigned incorrectly under advanced compilation Created: 12/Apr/16  Updated: 12/Apr/16

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

Type: Defect Priority: Minor
Reporter: Peter Jaros Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Given the following:

(ns with-redefs-bug.core)

(enable-console-print!)

(defn a-function [arg-1 arg-2 & args]
  nil)

(with-redefs [a-function (fn [& args] args)]
  (prn (a-function :arg-1))
  (prn (a-function :arg-1 :arg-2))
  (prn (a-function :arg-1 :arg-2 :arg-3)))

Under :optimizations :none, this code correctly prints:

(:arg-1)
(:arg-1 :arg-2)
(:arg-1 :arg-2 :arg-3)

However, under :optimizations :advanced, this code prints:

(:arg-1)
(:arg-1 :arg-2)
:arg-1

That is, as long as the function is called with exactly or less than the number of non-variadic arguments in the original function bound to a-function, args is (correctly) bound to a seq of all the arguments, but if any more arguments are given, args is bound to the first argument.

Also, under either compilation, the following warning is generated:

WARNING: Wrong number of args (1) passed to with-redefs-bug.core/a-function at line 9

That surprises me, but since it happens under both methods, perhaps it's intentional.



 Comments   
Comment by Peter Jaros [ 12/Apr/16 4:21 PM ]

Reproduced on r1.8.40 and current master (29eb8e0).

Comment by Mike Fikes [ 12/Apr/16 8:24 PM ]

This is actually not specific to :optimizations :advanced, but to :static-fns true.





[CLJS-1618] `extend-type string` doesnt work without wrapping the string object into `(str)` Created: 07/Apr/16  Updated: 10/Apr/16

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

Type: Defect Priority: Minor
Reporter: Yehonathan Sharvit Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(ns my.car)

(defprotocol Car
(drive [this]))

(extend-type js/String
Car
(drive [this] (map #({"a" "A"} %) [this (str this)])))

(drive "a"); (nil "A") expected ("A" "A")

See the reproduction of the bug in a live environment with KLISPE here: http://app.klipse.tech/?sourcce=jira&cljs_in=(ns%20my.car)%0A%0A(defprotocol%20Car%0A%20%20(drive%20%5Bthis%5D))%0A%0A(extend-type%20js%2FString%0A%20%20Car%0A%20%20(drive%20%5Bthis%5D%20(map%20%23(%7B%22a%22%20%22A%22%7D%20%25)%20%5Bthis%20(str%20this)%5D)))%0A%0A%0A(drive%20%22a%22)%0A



 Comments   
Comment by Francis Avila [ 07/Apr/16 6:27 PM ]

This is because of boxing and the implementation of cljs.core._EQ_

By extending type js/String (the String object/class in javascript) instead of "string", your "this" will be the boxed string rather than the primitive string (in non-strict js mode--in strict mode it will be the primitive also). The (str this) is coercing the boxed string back to a primitive string.

The core issue is really:

(= (js/String. "a") "a") ;=> false
;; thus
({"a" "A"} (js/String. "a")) ;=> nil

You should really use

(extend-type string ...)
Comment by Francis Avila [ 07/Apr/16 6:41 PM ]

BTW this appears to be different from Clojure where primitives and boxed-primitives appear equal:

;; Clojure code
(= (String. "a") "a")
=> true
(= (Long. 1) 1)
=> true
(= (Long. 1) (long 1))
=> true

Not sure if clojurescript should try to replicate this more closely or not.

Clojurescript bottoms out with triple-equals in most cases, which is why primitives and boxes do not compare equal. To get them to compare equal would require adding special (instance? js/BOXED x) checks and some modifications to existing -equiv implementations which extend primitive types. (e.g. (extend-type number IEquiv ...) uses identical? without checking if the right-hand side is boxed or not.)

Comment by Mike Fikes [ 10/Apr/16 12:24 AM ]

As Francis alludes to, this is not a bug. If you do (doc extend-type), it indicates the type-sym can be string to cover the base type js/String, and it elaborates with

Note that, for example, string should be used instead of js/String.
and if a user does try to use js/String as the type-sym argument, a diagnostic is issued:

WARNING: Extending an existing JavaScript type - use a different symbol name instead of js/String e.g string at line 1




[CLJS-1620] In JavaScript ES2015 modules default export name is munged to default$ Created: 08/Apr/16  Updated: 08/Apr/16

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

Type: Defect Priority: Minor
Reporter: Roman Liutikov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When using a foreign lib which is ES2015 module with default export, the value which is being exported is assigned to a property default on a namespace object. In ClojureScript code this means one should call to default var of that namespace. However in complied output of ClojureScript code the name default is getting munged to default$.

export default function inc(v) {
  return v + 1;
}
(ns cljs-example.core
  (:require [lib.inc :as lib]))

(lib/default 0)
goog.provide("module$lib$inc");
function inc$$module$lib$inc(v){return v+1}
module$lib$inc.default=inc$$module$lib$inc
// Compiled by ClojureScript 1.8.40 {}
goog.provide('cljs_example.core');
goog.require('cljs.core');
goog.require('module$lib$inc');
module$lib$inc.default$.call(null,(0));

//# sourceMappingURL=core.js.map


 Comments   
Comment by David Nolen [ 08/Apr/16 2:42 PM ]

One possible path around this is to respect the Closure Compiler language setting when munging instead of blindly munging ECMA-262 keywords. A patch that adopts this approach would be welcome.





[CLJS-1598] Honor printing of function values via IPrintWithWriter Created: 03/Mar/16  Updated: 08/Apr/16

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





[CLJS-1615] Inlining truth checks can lead to better optimisation results Created: 04/Apr/16  Updated: 08/Apr/16

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

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

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

 Description   

I had a situation when DCE was (naively) expected but didn't happen (in :advanced compilation mode). I did some exploration and discovered that inlined truth check code is better understood by Closure Compiler and leads to expected optimisation (for some reasons).

I believe understanding this behaviour and exploiting it where desirable could lead to more predictable code generation without resorting to using cljs type hints.



 Comments   
Comment by Antonin Hildebrand [ 04/Apr/16 2:54 PM ]

The backstory as posted to #cljs-dev in Slack:

When @domkm requested proper dead code elimination in cljs-devtools, I got burnt by the need to explicitly specify `(if ^boolean js/goog.DEBUG …)` hint to get `:closure-defines` working under :advanced build[1]. It was unexpected to me that closure compiler cannot see that optimization and does not inline truth test for js/goog.DEBUG “constant”. So I started poking around and found a way how to aggressively inline checked truth checks in a compatible way (I believe). I also believe this could potentially open optimizations in other places. I think we should explore `@nosideeffects` annotation[2] and tag core functions where appropriate.

[1] https://github.com/binaryage/cljs-devtools/releases/tag/v0.5.3
[2] https://developers.google.com/closure/compiler/docs/js-for-compiler?hl=en#tag-nosideeffects

Comment by Francis Avila [ 05/Apr/16 3:11 PM ]

`@nosideeffects` should only be relevant for extern files where the compiler cannot see the implementation and know if the function is pure. Normally the compiler just analyzes the function to see if it side-effects.

This may be a performance win as well: it looks like advanced compile will unwrap the function expression entirely in some cases (both expression and statement contexts), so no more megamorphic calls to truth_ or even function object allocations.

However, I don't think there's a guarantee that the closure compiler will always understand enough to remove the need for the ^boolean hint for defines in all cases.





[CLJS-1616] Self-host: improve documentation for compile-str Created: 06/Apr/16  Updated: 06/Apr/16

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

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


 Description   

It's not clear at all how to use the `opts` arguments for compile-str.

In the code - https://github.com/clojure/clojurescript/blob/c3899acf797eb6779c53b313f5606c5018360b83/src/main/cljs/cljs/js.cljs#L660 - we
only have
:load - library resolution function, see load-fn
:source-map - set to true to generate inline source map information

In fact, there is also :verbose and ::def-emits-var

They are not documented.

Are there more options?






[CLJS-1601] Optimize cljs.core dump Created: 12/Mar/16  Updated: 01/Apr/16

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

Type: Enhancement Priority: Minor
Reporter: Nikita Beloglazov Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: bootstrap

Attachments: Text File CLJS-1601.patch     Text File CLJS-1601.patch    

 Description   

When building cljs source that uses cljs.js namespace the final js file is quite huge: 6.4M. As described in wiki: https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting it mostly consists of analysis cache of the cljs.core namespace. As a workaround, the wiki article suggests dumping cache to a separate file and load it at runtime instead of bundling in js binary. I think it is possible to have something in between that doesn't require additional efforts from a user and also optimizes the size of the js file. The idea that instead of dumping cache as raw clojure data-structure it is serialized to string. This way compiler won't compile cache into js (which adds a lot of code) and leave it a string. At runtime, this string will be parsed back to clojure using tools.reader.

Here is the proposal: https://gist.github.com/nbeloglazov/0bf163fb62fa4b61d446

Checking locally it reduces the size of js file from 6.4M to 2.7M which I think quite good. The downside is that now js has to do more work on runtime (parse huge string) when today it simply read js code and evaluates it. But I don't think if it's a big concern. If it is desired to keep all behavior a new option can be added for :dump-core compiler setting, something like :dump-core :string that enables string serialization of the cache.

Does it sound reasonable?



 Comments   
Comment by Nikita Beloglazov [ 27/Mar/16 8:54 PM ]

Attaching suggested fix. Analysis cache is serialized to string and read back to clojure datastructure when cljs.js is initialized.

Comment by David Nolen [ 28/Mar/16 6:39 AM ]

Please change the patch so this optional as you've suggested.

Comment by David Nolen [ 28/Mar/16 6:40 AM ]

Also have you submitted your Clojure CA yet?

Comment by Nikita Beloglazov [ 28/Mar/16 1:35 PM ]

Will do. Yes, I've submitted CA. I used my official name, Mikita Belahlazau there.

Comment by Nikita Beloglazov [ 29/Mar/16 12:16 AM ]

Updated patch that adds option to serialize core analysis cache as string. Possible values of :dump-core are :raw, :string, :none. Old true/false values supported for backward compatibility.

As for default, current patch uses :raw, but I think it makes more sense to use :string. Saving extra few mb of final js is quite good. I think most devs won't go deep into figuring out why js is big and just leave it as it is. Additional one-time parsing performance hit :string introduces acceptable: when :string is used, page loads in 1s while with :raw the time is ~800ms.





[CLJS-1450] Arithmetic warning thrown for impossible condition Created: 15/Sep/15  Updated: 30/Mar/16

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

Type: Defect Priority: Trivial
Reporter: Quest Yarbrough Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, compiler


 Description   

The following code triggers an arithmetic warning even though the condition it's warning about is impossible to reach. I tested this code in Clojure and it did not generate a warning. I would guess that the CLJS compiler doesn't take note of the (js/Error) in the same way that the Clojure compiler treats (Error.)

The exact warning triggered is below, followed by the code.

WARNING: cljs.core/+, all arguments must be numbers, got [number clj-nil] instead. at line 22 src\spurious_arithmetic\core.cljs
(def x [0 1 2 3 4 nil])
(def index (atom -1))
(defn take-value []
  (->> (swap! index inc)
       (nth x)))

(-> (loop [result (take-value)
           prev nil]
      (if (= nil result prev) (throw (js/Error. "This condition prevents nil arithmetic.")))
      (if (some? result)
        (recur (take-value) result)
        (+ 1 prev)))                                        ; this triggers the [number cljs-nil] warning
    (print)) ; 5


 Comments   
Comment by Mike Fikes [ 30/Mar/16 11:33 PM ]

The type inference logic in the compiler expects that the type of a loop local is static. In fact, somewhat the opposite of this ticket is being done in CLJS-1561.





[CLJS-1610] Refs api Created: 26/Mar/16  Updated: 28/Mar/16

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

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


 Description   

It would be nice to support the refs API in clojurescript to ease porting of libraries which require it. Obviously usage of it would not bring concurrency benefits, but it would at least allow some existing clojure code to run.

I've satisfied myself it can be done reasonably easily (see https://github.com/jjl/clojurescript/tree/ref-support ), and I can't really see any downsides.

For the minute, this has been put into the cljs.stm namespace. I don't know whether it should be a feature you are automatically opted into, so I didn't put it into core. Thoughts?



 Comments   
Comment by David Nolen [ 28/Mar/16 6:44 AM ]

It's definitely worth considering. If you've done the work, attach an actual patch for review. In general we try to avoid links outside.





[CLJS-1575] Combination of - and _ params causes JSC_DUPLICATE_PARAM Created: 17/Feb/16  Updated: 18/Mar/16

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

Type: Defect Priority: Minor
Reporter: Peter Jaros Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following function definitions each cause ERROR: JSC_DUPLICATE_PARAM. Parse error. Duplicate parameter name "_" at compile time with :advanced compilation:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defn causes-duplicate-param [{_ :foo}]
  (reify
    Object
    (a-function [-]
      (fn [] "arbitrary function inside a-function body"))))

(defn causes-duplicate-param [{- :foo}]
  (reify
    Object
    (a-function [_]
      (fn [] "arbitrary function inside a-function body"))))

They generate, respectively:

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1601.prototype.a_function = ((function (map__1599,map__1599__$1,_){
return (function (){
var self__ = this;
var _ = this;
return ((function (_,map__1599,map__1599__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(_,map__1599,map__1599__$1,_))
});})(map__1599,map__1599__$1,_))
;

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1612.prototype.a_function = ((function (map__1610,map__1610__$1,_){
return (function (){
var self__ = this;
var _ = this;
return ((function (_,map__1610,map__1610__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(_,map__1610,map__1610__$1,_))
});})(map__1610,map__1610__$1,_))
;

(Notice the duplicate _ param on the 5th line of each.)

The following do not:

Unable to find source-code formatter for language: clojure. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
(defn causes-duplicate-param [{- :foo}]
  (reify
    Object
    (a-function [-]
      (fn [] "arbitrary function inside a-function body"))))

(defn causes-duplicate-param [{_ :foo}]
  (reify
    Object
    (a-function [_]
      (fn [] "arbitrary function inside a-function body"))))

They generate:

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1601.prototype.a_function = ((function (map__1599,map__1599__$1,_){
return (function (){
var self__ = this;
var ___$1 = this;
return ((function (___$1,map__1599,map__1599__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(___$1,map__1599,map__1599__$1,_))
});})(map__1599,map__1599__$1,_))
;

duplicate_param_name_demo.core.t_duplicate_param_name_demo$core1612.prototype.a_function = ((function (map__1610,map__1610__$1,_){
return (function (){
var self__ = this;
var ___$1 = this;
return ((function (___$1,map__1610,map__1610__$1,_){
return (function (){
return "arbitrary function inside a-function body";
});
;})(___$1,map__1610,map__1610__$1,_))
});})(map__1610,map__1610__$1,_))
;

(Notice that one of the {} params has become __$1.)

My guess, though I haven't looked into the compiler code, is that the compiler escapes {} to _$1 when it would conflict with another {}, and also it translates - to {}, but it doesn't notice the conflict when the _ClojureScript symbols are different.



 Comments   
Comment by Peter Jaros [ 17/Feb/16 11:44 AM ]

Forgive the weird formatting errors. I couldn't find a preview function and there doesn't appear to be a way to edit the issue now that it's posted.

Comment by David Nolen [ 18/Mar/16 1:46 PM ]

This is because of munging they will become the same thing. Patch welcome.





[CLJS-1562] WARN on hinted fn call type mismatch Created: 06/Feb/16  Updated: 18/Mar/16

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

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


 Description   

If a call is made to a function that has hinted arguments (especially {^boolean} and {^number}), with an expression that is known to not be of that type, emit a diagnostic type mismatch warning.

An example that should emit a warning is:

(defn f [^boolean b])
(f 0)





[CLJS-1444] Node.js shim requires `node` invocation to be in the same directory as shim script Created: 10/Sep/15  Updated: 14/Mar/16

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

Type: Defect Priority: Minor
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Don't have time to provide a proper repro now but the basic issue can be illustrated by this:

~/c/boot-cljs-example (master=) node target/main.js
module.js:338
    throw err;
          ^
Error: Cannot find module '/Users/martin/code/boot-cljs-example/out/goog/bootstrap/nodejs.js'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/martin/code/boot-cljs-example/target/main.js:6:1)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
~/c/boot-cljs-example (master=) cd target/
~/c/b/target (master=) node main.js
Starting...

This is compiled with boot because that was what I had at hand right now. The compiled shim looks like this:

var path = require("path");
try {
    require("source-map-support").install();
} catch(err) {
}
require(path.join(path.resolve("."),"out","goog","bootstrap","nodejs.js"));
require(path.join(path.resolve("."),"out","cljs_deps.js"));
goog.global.CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"nodejs"};
goog.require("boot.cljs.main");
goog.require("cljs.nodejscli");

The problem here is that path.resolve(".") will return the directory the node command was invoked in and not the directory of the shim. (See the "Cannot find module..." error above)

A solution could be to use __dirname which always resolves to the directory of the current file. This might result in some breakage for existing setups.



 Comments   
Comment by Sebastian Bensusan [ 14/Oct/15 11:31 AM ]

I have a proposed solution but I fear that supporting "run from anywhere" adds essential complexity to the resulting code. My thought process:

1. Relative paths are desirable to produce "context free code." If the user chooses absolute paths, then that behavior is respected and there's nothing to handle (no "path algebra") .

2. When dealing with relative paths, the whole system needs to establish a "frame of reference", a root path. The ClojureScript compiler assumes the path from which it is compiling to be that frame of reference, which usually coincides with the top root of the project. Though arbitrary, it is the only choice that makes sense.

3. The frame of reference is not explicit anywhere in the code, since it is defined as ".". If it were explicit, it would reveal context, as in "/home/some-user/their-folder/this-project/".

4. When we approach the code from another reference point (executing the script from another directory), we first need to find the original compiler path (reference point,) and then resolve all paths from there. The compiler uses `cljs.closure/path-relative-to` for this purpose.

Path algebra:
compiler-path = __dirname - output-to

Node.js

var compiler-path = __dirname.replace(output-to, "")
path.resolve (compiler-path, output-dir, "goog", "base.js")
path.resolve (compiler-path, output-dir, "cljs_deps.js")

which assumes that if output-to was given relatively, then output-dir is also relative. If they are not in sync, more work needs to be done to keep them that way.

It's not up to me to decide if the extra complexity is worth the use-case. I actually hope there is a simpler solution to solve this that I'm not seeing.

Comment by Karol Majta [ 14/Mar/16 10:43 AM ]

I find this behavior really weird and would opt for switching to __dirname. I am also not sure i fully understand consequences of such switch (I have little cljs experience, speaking more from the perspective of nodejs user). My point is: current behavior renders clojurescript hard to use for commandline and desktop applications (namely electron).

For command line and desktop applications assumptions about CWD cannot be made. For now i run my applications through a bootstrap script JS script:

process.chdir(__dirname);
require('./target/out');

I am lucky that my code does not have to use the real CWD, but it's a hack more than a real solution.

Speaking from nodejs perspective:

1. Using absolute paths is considered a bad practice anyway.
2. Nodejs programs that don't use external packages (don't depend on node_modules) can be run from any CWD
3. Nodejs programs that do depend on node_modules will break if run from a different directory than the one containing node_modules, but this is expected behavior.





[CLJS-1599] UUIDs are not equal for upper/lower case strings Created: 07/Mar/16  Updated: 11/Mar/16

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

Type: Defect Priority: Minor
Reporter: Nikolay Durygin Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 7x64



 Description   

UUIDs generated for strings in different case (one is upper and one is lower) are equal.

For example (= (uuid "071C600F-B72B-44AE-8A15-9366EA1BB9D9") (uuid "071c600f-b72b-44ae-8a15-9366ea1bb9d9")) returns false.

Spec http://www.itu.int/rec/T-REC-X.667/en says following:

6.5.4 Software generating the hexadecimal representation of a UUID shall not use upper case letters.
NOTE - It is recommended that the hexadecimal representation used in all human-readable formats be restricted to lower-case
letters. Software processing this representation is, however, required to accept both upper and lower case letters as specified
in 6.5.2.



 Comments   
Comment by Gary Fredericks [ 07/Mar/16 8:17 AM ]

Would this be a good time to change the internal representation from a string to either a pair of goog.math.Longs or a quartet of "32-bit" integer doubles?

Comment by Nikolay Durygin [ 09/Mar/16 2:22 AM ]

Is there any special need? Issue described above can be solved by lower casing all strings inside uuid. Another problem - the fact that uuid doesn't complain if non uuid format string is passed can be solved with regex.





[CLJS-1074] Externs inference Created: 02/Mar/15  Updated: 01/Mar/16

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3211
Fix Version/s: GSoC

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Maria Geller
Resolution: Unresolved Votes: 3
Labels: None


 Description   

Given all externs generally need to be supplied for js/foo we could probably automatically compute externs based on js/foo usage in user code. For this to work correctly we need to account for property access through js/foo i.e. (def Bar js/foo.Bar). This should infer that Bar is also a foreign object. Things gets more complicated for higher order cases, we probably want to support a ^js type hint.

Finally externs inference needs to account for externs likely already supplied by the user - i.e. don't emit dupes, Google Closure will complain.



 Comments   
Comment by Leon Grapenthin [ 27/Feb/16 3:17 PM ]

Is this still being worked on?

Here is an approach: https://gist.github.com/Chouser/5796967

A very lean first approach would be to generate a `var foo = {}` for every interop expression.

I. e. by experimentation I could observe that no nested statements or var foo = function() statements are required to prevent minification.

js/foo 
js/foo.Bar 
(js/foo.Bar) 
(.-Bar js/foo) 
(.-Bar x) 
;; etc... would all not be minified with 
var foo = {}; 
var Bar = {};

To prevent dupes a cheap way to go would be a CLJS compiler mode in which no extern files are loaded. We can disable Closures externs via the exclude_default_externs compiler flag.

IDK if the minification quality is in any way different if the externs are type annotated or declared nested of with =function() --?

At least it looks like doing this would automate the most common use case of externs in CLJS: Preventing minification.

Comment by David Nolen [ 29/Feb/16 9:05 PM ]

Not actively being worked on at the moment but Maria Geller has a pretty solid proof of concept in a branch that somebody else can pick up. It takes the basic idea from that gist much further.

Comment by Leon Grapenthin [ 01/Mar/16 12:41 AM ]

Branch for reference: https://github.com/mneise/clojurescript/commits/CLJS-1074

Thanks David. Will have a closer look asap.





[CLJS-1561] WARN if recur passes non-inferred type Created: 06/Feb/16  Updated: 23/Feb/16

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

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

 Description   

Take this code as an example:

(defn f [^boolean b]
  (loop [x b]
    (if x
      (recur 0)
      :done)))

The type of x is inferred to be Boolean, but there is a recur form that can be statically deduced to be passing a non-Boolean.

This ticket asks that a WARN be issued for this case, and perhaps others (where maybe x itself is directly type hinted).



 Comments   
Comment by Mike Fikes [ 06/Feb/16 2:59 PM ]

Attached a patch which warns on for the case of boolean and number, since those two types have special handling.

Some example usage:

cljs.user=> (defn f [^boolean b]
       #_=>   (loop [x b]
       #_=>     (if x
       #_=>       (recur 0)
       #_=>       :done)))
WARNING: recur target parameter x has inferred type boolean, but being passed type number at line 4 
#'cljs.user/f
cljs.user=> (loop [x 1 y true z :hi]
       #_=>   (when false (recur 'a "hi" nil)))
WARNING: recur target parameter x has inferred type number, but being passed type cljs.core/Symbol at line 2 
WARNING: recur target parameter y has inferred type boolean, but being passed type string at line 2 
nil
cljs.user=> (loop [x 1 y true]
       #_=>  (when false (recur nil nil)))
WARNING: recur target parameter x has inferred type number, but being passed type clj-nil at line 2 
WARNING: recur target parameter y has inferred type boolean, but being passed type clj-nil at line 2 
nil
cljs.user=> (loop [x 1]
       #_=>   (let [y (inc x)]
       #_=>     (when false (recur (inc y)))))
nil
cljs.user=> (loop [b true]
       #_=>   (when false (recur (inc 1))))
WARNING: recur target parameter b has inferred type boolean, but being passed type number at line 2 
cljs.user=> (loop [x 1] 
       #_=>   (inc x) 
       #_=>     (when false (recur :hi)))
WARNING: recur target parameter x has inferred type number, but being passed type cljs.core/Keyword at line 3 
nil
cljs.user=> (loop [x :hello] 
       #_=>   (inc x) 
       #_=>     (when false (recur :hi)))
WARNING: cljs.core$macros/+, all arguments must be numbers, got [cljs.core/Keyword number] instead. at line 2 
nil




[CLJS-1494] turn cljs.core/*assert* into a goog-define Created: 25/Nov/15  Updated: 22/Feb/16

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

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

Attachments: Text File goog-define-assert.patch    
Patch: Code

 Description   

This patch turns the cljs.core/*assert* boolean into a goog.define and also checks *assert* at runtime (instead of only at compile-time).

The closure define option allows the closure compiler to eliminate asserts in :advanced, while :none builds can keep the asserts. This is one of the few remaining issues that prevent :advanced builds to re-use :none compiled (cached) files.

:elide-asserts is unaffected to keep this as simple as possible, but could be built on top of the goog.define instead of actually affecting the compiled output.



 Comments   
Comment by Mike Fikes [ 20/Feb/16 8:02 AM ]

Patch no longer applies, probably owing to CLJS-970.

Comment by Thomas Heller [ 22/Feb/16 5:08 AM ]

There was one more issue I discovered with my approach. My goal was to enable the Closure Compiler to eliminate the asserts when using :advanced compilation. This works perfectly fine with using a goog.define for *assert* but the compiler will complain if you try to adjust the define later since goog.define vars are not allowed to be adjusted at runtime.

(binding [*assert* false]
  (something-that-asserts))

This works in CLJ but not in CLJS since *assert* is only checked at compile time. If compiled with :elide-asserts true you can't bind assert to true either since the code no longer exists.

So some compromise must be made either way, the best solution IMHO would be to have a goog.define which lets the compiler decide whether to eliminate the asserts or not, independent from the *assert* and then moving the assert check itself into js instead of the compiler.

Happy to write the patch if interested.





[CLJS-1574] CLJS string equivalence is very slow in Chrome Created: 16/Feb/16  Updated: 22/Feb/16

Status: Open
Project: ClojureScript