<< Back to previous view

[CLJS-2386] random-uuid should use a cryptographically strong PRNG if available Created: 19/Oct/17  Updated: 29/Oct/17

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

Type: Enhancement Priority: Minor
Reporter: Adrian Bendel Assignee: Adrian Bendel
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-2386-no-target-js-mod.patch     Text File 0002-CLJS-2386-remove-window.patch     Text File 0003-CLJS-2386-remove-node.patch     Text File 0004-CLJS-2386-join-array.patch     Text File 0005-CLJS-2386-join-aset.patch     Text File 0006-CLJS-2386-fix-join-aset.patch     Text File CLJS-2386.patch    
Patch: Code

 Description   

random-uuid currently uses Math/random via rand-int to generate random numbers for v4 UUIDs. This patch aims to use a cryptographically strong PRNG (pseudo random number generator) instead, if available.

The functions used are:

  • window.crypto.getRandomValues in most browsers
  • window.msCrypto.getRandomValues in IE11
  • Math/random in browsers not supporting the former or if the crypto module is not made available on Node.js

Currently not used:

  • crypto.randomBytes on Node.js

Google Closure doesn't seem to provide feature detection or wrappers for the crypto-APIs, so the attached patch proposal implements a shim based on feature detection.

One open question is how the Node.js crypto module can be made available, since ClojureScripts core.cljs doesn't seem to have conditional require of Node.js-modules and maybe it should be kept this way.



 Comments   
Comment by David Nolen [ 19/Oct/17 1:33 PM ]

Some initial feedback don't switch on target, just do feature detection please. Don't use `js/window`, use `goog.global`.

Comment by A. R [ 20/Oct/17 3:43 AM ]

Should we use `js-mod` since the `UInt8` array guarantees them to be positive numbers?

Possibly outside of scope but instead of using `str` we could get some speedup by manually using `(.join ... "")` and `(.join ... "-")`

Comment by Adrian Bendel [ 20/Oct/17 4:22 AM ]

Added 0001-CLJS-2386-no-target-js-mod.patch which removes the *target* switch and replaces js/window with goog.global as per Davids comment.

This patch also uses js-mod instead of mod like A.R. suggested and adds a docstring for random-uuid explaining the behavior.
I could implement the .join suggestion, too, just not right now.

The ticket description has been updated to reflect the current state.

Comment by A. R [ 21/Oct/17 4:42 AM ]

@Adrian : Don't use goog.global.window since goog.global is already the window object when in the browser. It happens to work since the window object also keeps a refernce to itself, so you could write js/window.window.window.window.crypto. Currently your code wouldn't work on nodejs.

So just use goog.global.crypto and use js/Uint8ClampedArray as you had it previously.

@David: We should probably add a bool-expr around the js-in macro, then we could use that for feature detection code.

Comment by Adrian Bendel [ 21/Oct/17 5:16 AM ]

Oh that search/replace went wrong and wasn't even intended, testing succeeded for the reasons you explained, will fix it.

Comment by Adrian Bendel [ 21/Oct/17 5:54 AM ]

0002-CLJS-2386-remove-window.patch fixes the problems pointed out by A. R. .join refactoring is still postponed.

@A. R. for (exists? crypto) on node i can't use goog.global because it is a module, right? Thanks for the help/reviews!

Comment by A. R [ 22/Oct/17 2:47 AM ]

1. There is no reason to check for goog.global, it'll exist
2. You're checking for goog.global.msCrypto.getRandomValues but then calling goog.global.crypto.getRandomValues
3. You can't check like this (exists? crypto) ;; nodejs for node modules. crypto has to resolve to something. Just omit this nodejs case for now. Unless David can give better advice on how to conditionally require a crypto libraray in node.

Comment by Adrian Bendel [ 22/Oct/17 10:25 AM ]

Sorry for the bad patches the last two days, always have been in a hurry.
Attached 0003-CLJS-2386-remove-node.patch fixes A. Rs latest finds.

Comment by Adrian Bendel [ 29/Oct/17 5:51 AM ]

Attached 0004-CLJS-2386-join-array.patch uses .join on an array instead of str

An alternative implementation could be to convert the Uint8ClampedArray to a regular array after generating the random numbers and then just aset the string conversions in place and .join the result. But I don't know if it would be more efficient and how to convert the typed array to a regular array.

Comment by Adrian Bendel [ 29/Oct/17 8:41 AM ]

0005-CLJS-2386-join-aset.patch should be more efficient because it doesn't use a closure to access the random values from the typed array. It just generates un/typed arrays with random values and then converts and transfers those into an untyped array to .join.

Comment by Adrian Bendel [ 29/Oct/17 9:08 AM ]

0006-CLJS-2386- fix-join-aset.patch fixes a bug in 0005-CLJS-2386-join-aset.patch and supersedes it.





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

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

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


 Description   

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

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

Order of work:

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

Extra stuff:

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


 Comments   
Comment by Ambrose Bonnaire-Sergeant [ 13/Sep/17 6:56 AM ]

In the body of this issue I have proposed a sequence of potential patches such that each patch will be easily screenable.

Can I have some feedback on this? Should some of these steps be combined/split?





[CLJS-2260] Convert :constant AST node to tools.analyzer format Created: 18/Jul/17  Updated: 23/Sep/17

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

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

Attachments: Text File const-ast-2.patch     Text File const-ast.patch    
Patch: Code

 Description   

Part of https://dev.clojure.org/jira/browse/CLJS-1461

Work on :const node:

  • rename :constant op to :const
  • add :val entry


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

This patch needs a rebase.

Comment by Amanda Walker [ 23/Sep/17 8:13 PM ]

Here is the rebased patch.





[CLJS-1902] Add support for compiler option :inline-source-maps Created: 24/Jan/17  Updated: 19/Nov/17

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

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

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

 Description   

1. refactor `emit-source-map` and break it into multiple functions
2. fix logic for relative path computation (see `strip-prefix-path`)
3. add support for `:inline-source-maps` option
4. add tests

Related: CLJS-1402, CLJS-1901



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:09 PM ]

Patches:
https://github.com/darwin/clojurescript/commit/de1da63072c8049b9812d41cce750e5a972c0b0b.patch
https://github.com/darwin/clojurescript/commit/2389fb7839a18299d842d381b135a669a7091869.patch
https://github.com/darwin/clojurescript/commit/3ba594d8c7215ef8c8276737b7e5d8008c6b3a98.patch

Comment by Antonin Hildebrand [ 24/Jan/17 8:27 PM ]

Full review: https://github.com/clojure/clojurescript/compare/darwin:inline-source-maps~3...darwin:inline-source-maps

Also please note that the first patch testing original functionality fails in one test because there was a bug in timestamp formatting in :source-map-url case:
https://github.com/clojure/clojurescript/compare/master...darwin:inline-source-maps#diff-55b85385d2d0bfb6dc20d59ed982d5c8L1239

Comment by Antonin Hildebrand [ 25/Jan/17 10:25 AM ]

Today when testing Dirac I realised we need to embed sources contents as well.

Additional patch:
https://github.com/darwin/clojurescript/commit/c1df38f14a33d02fe2d421f80db0b421b17286bb.patch

New review URL: https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps

Tested in DevTools and works like a charm.

Comment by Dusan Maliarik [ 06/Feb/17 10:00 AM ]

This would be helpful for us as well.

Comment by Andrea Richiardi [ 01/May/17 3:49 PM ]

I have run across this one as well by following this tutorial.

Without either this patch or Dirac's complicated setup it is not currently possible to use node --inspect and debug correctly. The symptom I see on our side is that source maps are detected but for some reason Chrome DevTools does not show them in the Tree View.

The content of one of it is:

{"version":3,"file":"\/Users\/user\/cqrs-engine-cljs\/out\/cqrs\/event_store.js","sources":["event_store.cljs"], ...
Comment by David Nolen [ 16/Jun/17 10:29 AM ]

Linking to patches outside of JIRA is not proper for tickets. Please add a single squashed patch to this ticket directly.

Comment by Antonin Hildebrand [ 16/Jun/17 2:25 PM ]

Attached it as a patch file.

Took https://github.com/darwin/clojurescript/compare/inline-source-maps~4...darwin:inline-source-maps.diff and applied it to current master. It applied cleanly without conflicts. Tests are still passing on my machine.

Comment by Mike Fikes [ 19/Nov/17 7:36 PM ]

Patch no longer applies; needs re-baseline.





[CLJS-2419] Self-host: `find-ns-obj` still broken for namespaces with 'a' as the first segment Created: 25/Nov/17  Updated: 30/Nov/17

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: António Nuno Monteiro
Resolution: Unresolved Votes: 0
Labels: None


 Description   

CLJS-2024 didn't fix this, which still has issues under Node.js



 Comments   
Comment by António Nuno Monteiro [ 25/Nov/17 3:54 PM ]

The attached patch removes the Node.js special case in `find-ns-obj` that uses `eval`, given that under simple optimizations the namespaces are in the `goog.global` scope.

The call to `js/eval` is no longer needed and was causing more problems that it solved.

Here's some more context on those problems: https://github.com/anmonteiro/lumo/issues/301

Comment by António Nuno Monteiro [ 25/Nov/17 4:21 PM ]

Removing the patch as it shows some regressions.





[CLJS-1908] Improve error messages by using pr-str instead of str when printing objects Created: 26/Jan/17  Updated: 05/Oct/17

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Daniel Compton
Resolution: Unresolved Votes: 6
Labels: errormsgs

Attachments: Text File 0001-CLJS-1908-Wrap-vars-in-throw-message-with-pr-str.patch    
Patch: Code

 Description   

Many error messages from ClojureScript include the invalid argument like this:

(throw (js/Error. (str "Doesn't support name: " x)))

If x is nil, then the error message produces is "Doesn't support name: " which is a bit mystifying to debug. If x was wrapped with pr-str then the error message would be the much more understandable: "Doesn't support name: nil".

If there's interest in this, then I can prepare a patch which wraps these kinds of errors with pr-str.



 Comments   
Comment by David Nolen [ 16/Jun/17 10:30 AM ]

Go for it

Comment by Mike Fikes [ 16/Jun/17 10:33 AM ]

I also pondered this for a bit with CLJS-2089. Seems like the right thing to do in general.





[CLJS-844] Optimize js->clj by switching to transients Created: 22/Aug/14  Updated: 19/Nov/17

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

Type: Enhancement Priority: Trivial
Reporter: Darrick Wiebe Assignee: Darrick Wiebe
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File js-to-clj-using-transients.patch     Text File use-transducers-in-js-to-clj.patch     Text File use-transducers-in-js-to-clj.patch    
Patch: Code

 Comments   
Comment by David Nolen [ 23/Aug/14 1:19 PM ]

Did you any benchmarks on other JS engines?

Comment by Darrick Wiebe [ 23/Aug/14 2:14 PM ]

No. My main concern was to check if there was any startup overhead that might offset the basic efficiency improvements for processing small collections. Using into with a transducer turned out to be faster in every scenario, significantly in most.

Comment by David Nolen [ 23/Aug/14 2:19 PM ]

It would be nice to see a jsperf of before and after on this ticket. Thanks.

Comment by Darrick Wiebe [ 23/Aug/14 2:23 PM ]

Is there a existing one that I can work from?

Comment by David Nolen [ 23/Aug/14 2:35 PM ]

There is not. I generally just make a simple project, create an advanced compiled artifact and copy and paste it into jsperf.

Comment by Darrick Wiebe [ 24/Aug/14 7:24 PM ]

Turns out reducing into a transient is considerably better than using a transducer (which was itself a little faster) for this.

http://jsperf.com/js-clj-transducer-test

The code is at:

https://gist.github.com/pangloss/591d77231fda460c2fbe

Let me know if you want me to prepare an updated patch.

Comment by David Nolen [ 25/Aug/14 7:34 AM ]

Thanks for putting this together. Yes please provide an updated patch.

Comment by Darrick Wiebe [ 26/Aug/14 11:19 AM ]

Not sure whether the convention is to comment that I've uploaded a new patch. Regardless, I uploaded it yesterday.

Comment by Marcin Kulik [ 07/Oct/15 1:46 PM ]

I have tested and benchmarked the patch on a big js objects (5MB+ json files) and I confirm that new-js->clj3 function is more than 2x faster. It runs now in the player on https://asciinema.org and all seems good so far.

Comment by David Nolen [ 08/Oct/15 2:33 PM ]

Marcin thanks for the feedback. Experience reports always help push tickets forward.

Darrick, the patch need to be rebased to master. Please remove all patches except the one that will be applied.

Comment by Rohit Aggarwal [ 08/Oct/15 3:06 PM ]

[ClojureScript beginner here. so do ignore this if I am mistaken]

The patch of 25/Aug uses `(aget x k)` instead of `(goog.object/get x k)` for getting the value of a key if x is an object. I believe it isn't the right way even though it works.

This contains why the latter is preferred over the former.

Comment by David Nolen [ 09/Oct/15 12:33 PM ]

Rohit good catch. Yes that should be changed.

Comment by Thomas Spellman [ 21/Oct/16 3:01 AM ]

Added a patch, "js-to-clj-using-transients.patch", on Oct 16, 2016 that supersedes "use-transducers-in-js-to-clj.patch" from Aug, 2014. This patch changes cljs.core/js->clj to use transients. Also included is a change to both js->clj and clj->js to use gobject/get and gobject/set instead of aget and aset on JS object.

The JSperf shows a 17% speed improvement in Chrome on the first run, but about equal on the second run: https://jsperf.com/js-clj-transient-perf-test

The code for the JSperf is here: https://gist.github.com/thos37/41c0b38bea3270988a3275332686ab49

What would be the ideal test data for this?

Comment by Mike Fikes [ 19/Nov/17 8:10 PM ]

js-to-clj-using-transients.patch no longer applies.





[CLJS-1067] Shared AOT cache for dependencies in JARs Created: 26/Feb/15  Updated: 22/Dec/17

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

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


 Description   

3rd party library code in JARs shouldn't be recompiled across dev and prod configurations. There should be a shared AOT cache for all builds within a project for all non-project local source.






[CLJS-1076] :nashorn target Created: 02/Mar/15  Updated: 27/Dec/17

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

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


 Description   

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






[CLJS-1501] Add :parallel-build support to REPLs Created: 05/Dec/15  Updated: 27/Jun/17

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

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-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 27/Jun/17

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

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-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 27/Jun/17

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

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-1896] Externs file validation Created: 23/Jan/17  Updated: 27/Jun/17

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

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


 Description   

Invalid externs file will fail silently and not provide the expected externs inference warnings. We should try to catch such issues when we parse the file and throw an exception.






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

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

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

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

 Description   

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

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

No *print-fn* fn set for evaluation environment

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

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



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

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

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

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

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

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





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

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

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

Approval: Vetted

 Description   

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






[CLJS-1300] REPLs do no write out updated deps.js when compiling files Created: 05/Jun/15  Updated: 27/Jun/17

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

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-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 19/Nov/17

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

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

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    
Patch: Code

 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

Comment by Mike Fikes [ 19/Nov/17 8:09 PM ]

Patch no longer applies.





[CLJS-1558] Code allowed to re-define referred var Created: 31/Jan/16  Updated: 27/Nov/17

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: None

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

 Description   

If you refer a var from another namespace, then you can def a new value for that var, and the def will mutate the other namespace, and other things will go wrong as illustrated in the example below.

FWIW, Clojure disallows this, and refuses to allow you to evaluate a def involving a referred var, and emits an error diagnostic like:

CompilerException java.lang.IllegalStateException: foo already refers to: #'some.name.space/foo in namespace: user, compiling:(NO_SOURCE_PATH:2:1)

Here is a complete example illustrating the issues:

Given:

(ns foo.core)

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

then do this in a REPL:

cljs.user=> (require '[foo.core :refer [square]])
nil
cljs.user=> (var square)
#'foo.core/square
cljs.user=> (square 3)
9
cljs.user=> (ns-interns 'cljs.user)
{}
cljs.user=> (defn square [x] (+ x x))
WARNING: square already refers to: foo.core/square being replaced by: cljs.user/square at line 1 <cljs repl>
#'foo.core/square
cljs.user=> (square 3)
6
cljs.user=> (var square)
#'foo.core/square
cljs.user=> (in-ns 'foo.core)
nil
foo.core=> (square 3)
6
foo.core=> (in-ns 'cljs.user)
nil
cljs.user=> (ns-interns 'cljs.user)
{square #'cljs.user/square}
cljs.user=> (cljs.user/square 3)
TypeError: Cannot read property 'call' of undefined
    at repl:1:105
    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:221:14)
    at Socket.<anonymous> ([stdin]:40:25)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
cljs.user=> #'cljs.user/square
#'cljs.user/square
cljs.user=> @#'cljs.user/square
nil


 Comments   
Comment by Mike Fikes [ 27/Nov/17 2:00 PM ]

The attached patch essentially turns the warning into an error for non-core names, throwing an exception that matches Clojure's.

Example:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 49203
To quit, type: :cljs/quit
cljs.user=> (def map 3)
WARNING: map already refers to: cljs.core/map being replaced by: cljs.user/map at line 1 <cljs repl>
#'cljs.user/map
cljs.user=> (require '[clojure.set :refer [intersection]])
nil
cljs.user=> (def intersection 3)
clojure.lang.ExceptionInfo: intersection already refers to: #'clojure.set/intersection in namespace cljs.user at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (def intersection 3)}, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4725)
	at clojure.core$ex_info.invoke(core.clj:4725)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:697)
...
Comment by Mike Fikes [ 27/Nov/17 2:05 PM ]

Here is an example showing that it is (as was before), OK to define a Var that shadows a core Var:

$ script/noderepljs
ClojureScript Node.js REPL server listening on 57077
To quit, type: :cljs/quit
cljs.user=> (defn quot [s] (str "'" s "'"))
WARNING: quot already refers to: cljs.core/quot being replaced by: cljs.user/quot at line 1 <cljs repl>
#'cljs.user/quot
cljs.user=> (quot "hello")
"'hello'"
cljs.user=> (cljs.core/quot 6 2)
3




[CLJS-2247] Warn when overwriting protocol method Created: 15/Jul/17  Updated: 29/Dec/17

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

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

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

 Description   

Warn when a protocol overwrites a method in another protocol.

Observe the warning in this Clojure REPL session:

user=> (defprotocol IAlpha
  #_=>   (-foo [this]))
IAlpha
user=> (defrecord Alpha []
  #_=>   IAlpha
  #_=>  (-foo [this] :alpha))
user.Alpha
user=> (defprotocol IBeta
  #_=>   (-foo [this]))
Warning: protocol #'user/IBeta is overwriting method -foo of protocol IAlpha
IBeta
user=> (defrecord Beta []
  #_=>   IBeta
  #_=>   (-foo [this] :beta))
user.Beta
user=> (-foo (->Alpha))

IllegalArgumentException No implementation of method: :-foo of protocol: #'user/IBeta found for class: user.Alpha  clojure.core/-cache-protocol-fn (core_deftype.clj:568)

Here is the same in ClojureScript:

To quit, type: :cljs/quit
cljs.user=> (defprotocol IAlpha
(-foo [this]))
nil
cljs.user=> (defrecord Alpha []
IAlpha
(-foo [this] :alpha))
cljs.user/Alpha
cljs.user=> (defprotocol IBeta
(-foo [this]))
nil
cljs.user=> (defrecord Beta []
IBeta
(-foo [this] :beta))
cljs.user/Beta
cljs.user=> (-foo (->Alpha))
repl:13
throw e__5612__auto__;
^

Error: No protocol method IBeta.-foo defined for type : [object Object]
    at cljs$core$missing_protocol (/Users/mfikes/.cljs_node_repl/.cljs_node_repl/cljs/core.js:317:9)
    at cljs$user$_foo (repl:25:34)
    at repl:1:94
    at repl:9:3
    at repl:14:4
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:242:14)
    at Socket.<anonymous> ([stdin]:49:25)

Note that they both result in the same runtime behavior, but Clojure emits a nice diagnostic when IBeta is defined.



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

This is an interesting one: While the attached patch produces nice warnings, even without it the :redef-in-file warning triggers if the forms involved happen to reside in files, and in that case you'd get two slightly different warnings being emitted:

WARNING: Protocol IDelta is overwriting method -bar of protocol IGamma in file /Users/mfikes/Desktop/src/foo/core.cljc
WARNING: -bar at line 6 is being replaced at line 7 /Users/mfikes/Desktop/src/foo/core.cljc

Each of the warnings above has their own strengths; but having both seems to be a negative. Here are my initial thoughts on variants:

  1. Leave both: Could be perceived to be noisy, albeit useful if it occurs.
  2. Have the warning disabled, but enabled for the REPL: Odd to have a warning that only happens while in the REPL (and you can still get two when loading files)
  3. Have the warning only be emitted for REPL-entered forms: This perhaps directly addresses the issue: (No double warnings, useful for people learning the language). But this seems even odder to have a warning limited to REPL-entered forms.
  4. Figure out a way to suppress the :redef-in-file warning in this case: Maybe cleaner.
  5. Same as above, but find a way to get the line numbers into the "overwrite" warning: Perhaps cleanest, but difficult to do?
  6. Do nothing, decline the ticket: Perhaps this leaves users learning the language at a loss if they encounter this while in the REPL where redefining forms is "normal", but not overwriting via protocol definitions.
Comment by Mike Fikes [ 29/Dec/17 10:19 AM ]

The attached CLJS-2247-2.patch rebaselines against master.





[CLJS-2460] Print MapEntry as vector Created: 08/Jan/18  Updated: 08/Jan/18

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: print, printing

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

 Description   

(pr-str (->MapEntry :a 1 nil)) yields {{"#object[cljs.core.MapEntry]"}} instead of "[:a 1]".



 Comments   
Comment by Mike Fikes [ 08/Jan/18 8:25 AM ]

The attached patch also tests that printing matches Clojure in the case that *print-length* happens to be less than 2.





[CLJS-2456] Have map types return MapEntry instead of vector Created: 03/Jan/18  Updated: 08/Jan/18

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

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

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

 Description   

Groundwork for this has been done.

CLJS-2013 adds the MapEntry type, but indicates "PAMs and PHMs don't emit them when you seq or iterate over them yet."
CLJS-2068 fixes an issue with MapEntry
CLJS-2001 adds a map-entry? predicate

So this ticket entails updating the PHM and PAM implementations so that, for example, (map-entry? (first {:a 1})) evaluates to true.

Note: One additional bit of groundwork that should be done prior to this ticket is CLJS-2460.



 Comments   
Comment by Thomas Mulvaney [ 06/Jan/18 6:46 PM ]

I've attached a patch for consideration. Besides updating `Seq` and `Iterator` types for PAM and PHM, it also updates implementers of `IFind` to return `MapEntry` rather than vectors. I can move the `IFind` changes into another patch though if that is prefered.





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

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

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

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

 Description   

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

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



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

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





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

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

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

Attachments: Text File CLJS-2389-1.patch     Text File CLJS-2389-3.patch     Text File CLJS-2389-4.patch     Text File CLJS-2389-5.patch     Text File CLJS-2389-wip-2.patch     Text File CLJS-2389-wip.patch    

 Description   

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Current patch status:

Latest v20180101 release processes CJS and ES6 OK.

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

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

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

And still seeing warnings about skipping passes.

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

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

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

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

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

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

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

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

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





[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: 4
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-2474] with-meta on lazy-seq causes separate realization Created: 20/Jan/18  Updated: 20/Jan/18

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

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

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

 Description   

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

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

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



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

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

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

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





[CLJS-1278] Asserts still fail while :require-ing .js file (either in :libs or in :source-paths) (same as CLJS-1196) Created: 20/May/15  Updated: 14/Jul/15

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

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

Attachments: Text File cljs_1278.patch    

 Description   

Following on CLJS-1196, I can't get it to work.

In version 0.0-3264 lein-cljsbuild crashed on weird eception `Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil"` but the current version 0.0-3269 gives the same failed assertion as previously.

I've put up a sample project to illustrate the issue.

Steps to reproduce:

`git clone https://github.com/tillda/stackone`
`cd stackone`
`git checkout 537e5c69b844bc53c159e85cafc24310543cc918`
`lein clean && lein cljsbuild once temp`

Expected behaviour: cljs compiled successfully with src/vendor/client/closure.js and env/stackone/helpersjs.js being included.

Actual behaviour:

```
Compiling "resources/public/lein-cljsbuild-temp/dev-mode-deps.js" failed.
Exception in thread "main" java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x)), compiling/private/var/folders/ym/l2qxd7l97kzfzftrdpqsclm40000gn/T/form-init3642888309490821030.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)
Caused by: java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x))
at cljs.util$ext.invoke(util.cljc:115)
at cljs.closure$source_on_disk.invoke(closure.clj:1206)
at cljs.closure$output_unoptimized$fn__3708.invoke(closure.clj:1235)
at clojure.core$map$fn__4551.invoke(core.clj:2622)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$filter$fn__4578.invoke(core.clj:2677)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$map$fn__4551.invoke(core.clj:2614)
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:674)
at clojure.core$next__4110.invoke(core.clj:64)
at clojure.core$str$fn__4186.invoke(core.clj:528)
at clojure.core$str.doInvoke(core.clj:526)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:628)
at cljs.closure$deps_file.invoke(closure.clj:1040)
at cljs.closure$output_deps_file.invoke(closure.clj:1060)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1243)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:630)
at cljs.closure$build.invoke(closure.clj:1514)
at cljs.closure$build.invoke(closure.clj:1426)
at cljsbuild.compiler$compile_cljs$fn__3884.invoke(compiler.clj:81)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:80)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:187)
at user$eval4018$iter_40544058$fn4059$fn_4077.invoke(form-init3642888309490821030.clj:1)
at user$eval4018$iter_40544058$fn_4059.invoke(form-init3642888309490821030.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:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$dorun.invoke(core.clj:3007)
at clojure.core$doall.invoke(core.clj:3023)
at user$eval4018.invoke(form-init3642888309490821030.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6792)
at clojure.lang.Compiler.eval(Compiler.java:6782)
at clojure.lang.Compiler.load(Compiler.java:7237)
... 11 more
Subprocess failed
```



 Comments   
Comment by David Nolen [ 20/May/15 10:21 AM ]

This issue is in danger of being closed. Please supply minimal steps to reproduce that do not involve anything other than the ClojureScript compiler. We no longer have time to wade through the indirection introduced by cljsbuild or any other downstream tooling. Thanks.

Comment by Michal Till [ 20/May/15 11:14 AM ]

@David Nolen: I have created a failing minimal testcase based on the Quick Start document. Here it is: https://github.com/tillda/cljs-testcase/

Comment by David Nolen [ 20/May/15 11:27 AM ]

Michal the failing example is not correct. You are not supplying any :libs option.

Comment by Michal Till [ 20/May/15 11:45 AM ]

Ah! Thank you very much! This additional issue was therefore my error. Now it seems to work even in my "big" example.

However it would be cool if there was a meaningful error message stating that a file path can't be resolved. If one is not an expert in the cljs compiler this is almost impossible to figure out. After all the error message in the CLJS-1196 issue and in this wrongfully reported one are exactly the same.

You may close this issue.

Comment by David Nolen [ 20/May/15 11:55 AM ]

We'll leave it open for the improving the error message.

Comment by Sebastian Bensusan [ 22/May/15 7:16 AM ]

Added the check in cljs.closure/source-on-disk where there is info for the error message.

For the supplied case, the error message is:

java.lang.IllegalArgumentException: The file file:/home/carlos/Playground/cljs-testcase/src/hello_world/closure.js 
lacks an associated source file. If it is a JavaScript library please add it to :libs}}

If a different wording or location of the check is needed, I'll submit a new patch with corrections.

Notes:

  • Changed:(:provides js) to (-provides js) in order to be consistent with IJavaScript.
  • cljs.clojure/source-on-disk takes a js argument that should satisfy with IJavaScript and ISourceMap if :source-map is enabled but the implementation is hardcoded to maps because :source-map and :source-url are used instead of ISourceMap methods -source-map and -source-url. I propose to extend PersistentMap and PersistentArrayMap to ISourceMap to make source-on-disk compliant with both protocols.




[CLJS-404] Automate Browser REPL testing Created: 23/Oct/12  Updated: 05/Jan/16

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

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


 Description   

It's worth investigating Selenium, PhantomJS, etc. as solutions to sanity check the Browser REPL when we run the other tests.



 Comments   
Comment by Robert Krahn [ 22/Dec/14 1:22 PM ]

An attempt: https://github.com/clojure/clojurescript/pull/42

Comment by David Nolen [ 24/Dec/14 8:57 AM ]

This looks like an interesting patch, thanks!

Comment by Robert Krahn [ 26/Dec/14 10:57 AM ]

I'll post a patch here, first I'll investigate the load-file issue, though.





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 05/Jan/16

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

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





[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-1924] The compiler cannot infer the target type of "(. RecordName -prototype)" expressions Created: 01/Feb/17  Updated: 01/Feb/17

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

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler, extern


 Description   

Repro:

Place

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

(defrecord Foo [])

anywhere in your source files, compile with :infern-externs true.

Expected:

Multiple warnings like:

  • WARNING: Cannot infer target type in expression (. Foo -prototype)
  • WARNING: Cannot infer target type in expression (. other__8838__auto__ -constructor)
  • WARNING: Cannot infer target type in expression (. user/Foo -getBasis)

There are also warnings for (. cljs.core/List -EMPTY), but this might be unrelated.






[CLJS-1970] Cannot infer target type for list/vector expressions Created: 08/Mar/17  Updated: 10/Mar/17

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

Type: Defect Priority: Minor
Reporter: Daniel Janus Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Ubuntu 16.04.2, Oracle JRE 8



 Description   

With

(set! *warn-on-infer* true)
enabled, attempting to compile functions like:

(defn foo [] (list))
(defn bar [] (vector))

results in a warning:

WARNING: Cannot infer target type in expression (. cljs.core/List -EMPTY) at line 2427 src/cljs/discann/core.cljs

WARNING: Cannot infer target type in expression (. cljs.core/PersistentVector -EMPTY) at line 2435 src/cljs/discann/core.cljs

The line number reported is totally unrelated to the line of code where the problematic fn appears.

Affects 1.9.456 and 1.9.494.






[CLJS-1164] quot and rem are inefficient Created: 24/Mar/15  Updated: 25/May/17

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

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: math

Attachments: Text File CLJS-1164-1.patch     Text File cljs-1164.patch    
Patch: Code and Test

 Description   

The implementation of the quot and rem functions are needlessly complicated. Currently they are:

(defn quot [n d] (fix (/ (- n (js-mod n d)) d)))
(defn rem [n d] (- n (* d (quot n d))))

However all numbers in js are doubles already, so all this is unnecessary:

(defn quot [n d] (fix (/ n d)))
(defn rem [n d] (js-mod n d)))

Notice that "rem" is simply js-mod, and I'm not sure why no one noticed this before. I keep js-mod for now since a lot of code uses it, and if cljs ever grows a number tower the distinction may be important.

Patch attached, which also:

  • Creates a macro version of quot and rem.
  • Updates documentation for quot, rem, js-mod and mod for clarity.
  • Implement fix (private function to round to zero) with ES6 Math.trunc() if available.

Existing quot and rem tests pass, although there could be some better tests of edge cases (negative decimal num or div, NaN and +-Infinity args).



 Comments   
Comment by Francis Avila [ 24/Mar/15 12:27 PM ]

Better tests found rounding errors in my updated rem, which should stay as-is. (Not simply js-mod after all! Seems to round args first? Not obvious from the spec.) Changed quot however is correct and introduces less error than the current one. Will update patch and tests when I get a chance.

Comment by Francis Avila [ 29/Mar/15 12:39 AM ]

Working patch with tests attached. Tests expanded to cover floating-point cases. rem is now fundamentally the same as master (was more accurate than js-mod!!), but returns results consistent with js-mod for non-finite args or zero divisor.

Comment by Mike Fikes [ 31/Jan/16 3:23 PM ]

cljs-1164.patch no longer applies on master

Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch now applies. I only tested with Nashorn:

V8_HOME not set, skipping V8 tests
SPIDERMONKEY_HOME not set, skipping SpiderMonkey tests
JSC_HOME not set, skipping JavaScriptCore tests
Testing with Nashorn

...

Ran 185 tests containing 17195 assertions.
0 failures, 0 errors.
Tested with 1 out of 4 possible js targets
Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch cleaned up

Comment by Mike Fikes [ 20/Feb/16 10:11 PM ]

Successfully ran Andrea's update to Francis's patch through V8, SpiderMonkey, JavaScriptCore, and Nashorn unit tests.

I also manually ran some of the unit tests in bootstrapped ClojureScript built with the patch.

LGTM.

Comment by Mike Fikes [ 20/Feb/16 10:23 PM ]

Since this is a low-level numerics update, also ran the unit tests through ChackraCore (successfully).

Comment by Mike Fikes [ 25/May/17 7:29 PM ]

CLJS-1164-1.patch no longer applies to master





[CLJS-2103]  clarify arg type and order constraints of keys and vals Created: 19/Jun/17  Updated: 19/Jun/17

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

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

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

 Description   

Backport CLJ-1302 to ClojureScript, while also making the argument name be map instead of hash-map.






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

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

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





[CLJS-2147] apply test suit Created: 01/Jul/17  Updated: 01/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: test

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

 Description   

Created an apply test suit.

Few tests are commented out which currently fail (nothing new and there are tickets for them).



 Comments   
Comment by David Nolen [ 01/Jul/17 1:56 PM ]

Please remove #_ following each test.

Comment by A. R [ 01/Jul/17 2:14 PM ]

Forgot about those. Done.





[CLJS-2127] Add invoke* helper macro Created: 26/Jun/17  Updated: 03/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: refactoring

Attachments: Text File CLJS-2127.patch    
Patch: Code
Approval: Screened

 Description   

This is a simple refactor around {IFn} protocol around core.cljc and core.cljs. We would like to hide the details of the invocation naming convention to avoid simple errors as well as to support changes more simply.



 Comments   
Comment by David Nolen [ 29/Jun/17 7:05 AM ]

The scope of this ticket needs to be narrowed to make it simpler for me to review. For the time being the only thing I would like to see is `invoke*` which hides the naming convention for direct invokes. No other higher level macro helpers should be provided in the resolution of this sissue.

Comment by A. R [ 29/Jun/17 12:29 PM ]

Patch updated. Much fewer changes to keep it simple for now.

Comment by David Nolen [ 29/Jun/17 2:08 PM ]

Looking better but lets have one helper for constructing the name, should just take number or :variadic.





[CLJS-2196] SpiderMonkey path needs quoting in test scripts Created: 08/Jul/17  Updated: 08/Jul/17

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

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

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

 Description   

The paths to the other engines (V8, Nashorn, etc.), are quoted, allowing paths with spaces. This needs to also be done for SpiderMonkey.






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

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

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

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

 Description   

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

Before:

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


After:

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


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

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

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

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





[CLJS-2268] clojure.string/escape in ClojureScript (unlike in Clojure) assumes cmap is a map Created: 23/Jul/17  Updated: 29/Oct/17

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

Type: Defect Priority: Minor
Reporter: Max Kreminski Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: cljs

Attachments: Text File 0001-CLJS-2268-Make-clojure.string-escape-constent-with-C.patch    
Patch: Code and Test

 Description   

The ClojureScript implementation of the clojure.string/escape function assumes that the cmap parameter will always be a map. This makes it different from (and specifically less general than) the Clojure implementation of the same function, which permits cmap to be anything callable.

Here's the relevant lines of the clojure.string/escape implementations in Clojure and ClojureScript. The ClojureScript implementation calls get on cmap, while the Clojure implementation invokes cmap directly.

Here's an example that works on Clojure, but doesn't work on ClojureScript, because it passes a function to clojure.string/escape instead of a map:

(defn regex-escape
  "Escapes regex special chars in the string `s`."
  [s]
  (let [special? #{\- \[ \] \{ \} \( \) \* \+ \? \. \\ \^ \$ \|}]
    (clojure.string/escape s #(when (special? %) (str \\ %)))))

Ideally, this discrepancy would be fixed by changing the ClojureScript implementation of clojure.string/escape to follow the Clojure one. This would also match the behavior described in the function's docstring, which is the same on both platforms.






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

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

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

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

 Description   

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



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

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


Before:

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

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

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

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

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


After:

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

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

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

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

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 37 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 43 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 73 msecs
Comment by Mike Fikes [ 01/Oct/17 9:27 AM ]

Added CLJS-2342-2.patch which rebases against current master.

Comment by Mike Fikes [ 29/Oct/17 9:03 AM ]

CLJS-2342-3.patch rebaselines against current master.





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

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

Comment by Mike Fikes [ 19/Nov/17 7:55 PM ]

CLJS-1627-5.patch no longer applies





[CLJS-1997] Outward function type hint propagation Created: 03/Apr/17  Updated: 20/Nov/17

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

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

 Description   

New feature: Detect the type of a function and arrange so that it is available for outward type hinting.

For reference, a small amount of discussion from Jan 2016 in the #cljs-dev Slack:

mfikes: Curious if "outward" ^boolean propagation has been debated / rejected. Example: If simple analysis detects that ^boolean can be applied as in for functions like (defn f [x] (not x)). Perhaps automatically propagating type hints in this direction would be unwelcome by users?

dnolen: @mfikes: we have already have outward propagation for numerics which broke because of changes for code motion

dnolen: absolutely nothing against that



 Comments   
Comment by Mike Fikes [ 03/Apr/17 7:38 PM ]

The attached patch will handle cases like

(defn foo? [x] (or (string? x) (number? x)))

and

(def baz (fn ([x] 1) ([x y] 2)))

but it doesn't attempt to address the case of multi-arity defn s, as these are implemented by dispatching to multiple single-arity functions that could have different return types.

It adds a :inferred-ret-tag to the AST only because the arguments to or on line 18 in the patch are not reversed (only to allow explicit hints to override inferred hints, but perhaps that is overcomplicating things and the inferred tag could be dropped directly into :ret-tag.)

Bronsa helpfully pointed out in Slack that outward type hint propagation is avoided in Clojure because it breaks things like:

(defn foo [] 1)
(defn bar [] (String/valueOf (foo)))
(defn foo [] "foo")
(bar)

for which an analogy in ClojureScript would be:

(defn foo? [] true)
(defn bar [] (if (foo?) :t :f))
(defn foo? [] "")
(bar)

where the patch causes it to yield :f.

This could be viewed as a REPL-only concern (perhaps addressed by another compiler flag along the lines of :static-fns).

But, arguably the problem already exists today, with type hinting in code like the following affecting the compilation of bar:

(def foo? true)
(defn bar [] (if foo? :t :f))
(def foo? "")
(bar)
Comment by David Nolen [ 07/Apr/17 11:15 AM ]

Thanks will think about it

Comment by Mike Fikes [ 16/Jun/17 10:30 AM ]

I've been soaking this for a couple of months (with local builds of Planck), and haven't seen anything break, FWIW. So, I think it is at least a safe change. Assigning to David for further consideration.





[CLJS-1975] Perf: Compare f with nil in Delay impl Created: 11/Mar/17  Updated: 21/Nov/17

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

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

 Description   

When a Delay has been realized, f is set to nil. We can avoid truth_ and not calls and directly compare with nil for a minor perf boost.

In script/noderepljs, this leads to these

(simple-benchmark [x (delay 1)] @x 1e9)
(simple-benchmark [x (delay 1)] (realized? x) 1e9)
(simple-benchmark [x (doto (delay 1) deref)] (realized? x) 1e9)

speeding up by 6%, 11% and 9%, respectively.



 Comments   
Comment by Mike Fikes [ 21/Nov/17 2:35 PM ]

Under :advanced across all engines, the 3 benchmarks in the ticket description produce these speedup numbers:

            V8: 1.18, 1.15, 1.09
  SpiderMonkey: 1.08, 1.04, 1.19
JavaScriptCore: 1.70, 0.86, 0.93
       Nashorn: 1.05, 1.15, 1.14
    ChakraCore: 1.21, 0.52, 0.81




[CLJS-2383] Improve perf of select-keys by using keyword-identical? Created: 17/Oct/17  Updated: 19/Nov/17

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

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

Attachments: Text File 0001-CLJS-2383-Speed-up-select-keys.patch     Text File 0002-CLJS-2383-Speed-up-select-keys-no-transient.patch     Text File CLJS-2383.patch    
Patch: Code and Test

 Description   

select-keys uses not= to compare keywords. Instead, using keyword-identical? results in decent speedups (an average of 1.34 across benchmarks and engines). Note that using identical? and lookup-sentinel doesn't appear to improve perf.

Speedup Summary:

V8: 1.15, 1.08, 1.08
  SpiderMonkey: 1.71, 1.48, 1.67
JavaScriptCore: 1.33, 1.35, 1.25
       Nashorn: 1.16, 1.04, 0.97
    ChakraCore: 1.59, 1.66, 1.72

Before:

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 179 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 121 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 183 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 251 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 201 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 290 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 112 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 73 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 119 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1277 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 524 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 635 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 463 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 268 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 414 msecs

After

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 155 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 112 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 169 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 146 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 135 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 173 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 84 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 54 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 95 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1099 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 502 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 648 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 292 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 151 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 240 msecs


 Comments   
Comment by Erik Assum [ 17/Oct/17 2:37 PM ]

Just want to bring your attention to CLJ-1789, where reimplementing `select-keys` in terms of reduce gave a significant speedup.
Added a patch to show that way.

Comment by Mike Fikes [ 18/Oct/17 6:46 AM ]

Here are the perf numbers for Erik's patch:

V8: 0.81, 1.14, 1.30
  SpiderMonkey: 1.49, 1.31, 1.58
JavaScriptCore: 1.02, 0.99, 0.96
       Nashorn: 1.13, 1.17, 1.21
    ChakraCore: 1.27, 1.35, 1.28

After:

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 220 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 106 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 141 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 169 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 153 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 183 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 110 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 74 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 124 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1128 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 447 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 524 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 366 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 199 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 323 msecs
Comment by Erik Assum [ 18/Oct/17 6:56 AM ]

Uploaded a patch without transients as well.

Comment by Mike Fikes [ 18/Oct/17 7:40 AM ]

Since the CLJ-1789 patch works better with larger maps, here is an additional perf test covering that case using the data from that ticket, testing both the original patch I had attached and Erik's subsequent patch. You can see the CLJ-1789 approach pays off for ClojureScript as well.

Erik, I see you attached a 3rd patch. I'd recommend adding perf numbers with each such patch, so the effect of the patch under advanced optimizations can be more readily assessed.

Engine          keyword-identical?  CLJ-1789
            V8:               1.13      1.29
  SpiderMonkey:               1.89      2.39
JavaScriptCore:               1.02      0.96
       Nashorn:               1.12      1.42
    ChakraCore:               1.68      1.82

Before:

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 373 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 668 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 200 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 2236 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1074 msecs

After (keyword-identical?)

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 330 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 353 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 197 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1991 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 640 msecs

After (CLJ-1789)

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 290 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 279 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 209 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1578 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 591 msecs
Comment by Erik Assum [ 18/Oct/17 7:54 AM ]

Both patches should have benchmarks now

Comment by Erik Assum [ 18/Oct/17 7:56 AM ]

oh, and as a comment to the comment about larger maps, I believe the performance `transient` bit is dependent on the size of the selected keys,
eg the more selected keys found in the map, the more we gain from `conj!`

Comment by Mike Fikes [ 23/Oct/17 12:04 PM ]

Running these 4 benchmarks

[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs

against all 5 engines with the 3 attached patches yields the following speedups on my machine:

CLJS-2383.patch:

            V8: 1.11, 1.10, 1.64, 1.18  Avg: 1.26
  SpiderMonkey: 2.36, 1.46, 1.79, 2.02  Avg: 1.91
JavaScriptCore: 1.28, 1.34, 1.23, 1.49  Avg: 1.34
       Nashorn: 1.19, 1.17, 1.06, 1.29  Avg: 1.18
    ChakraCore: 1.61, 1.78, 1.75, 2.11  Avg: 1.81
                                Overall avg: 1.50
         Avg excluding Nashorn & ChakraCore: 1.50

0001-CLJS-2383-Speed-up-select-keys.patch:

            V8: 0.70, 0.95, 1.05, 1.23  Avg: 0.98
  SpiderMonkey: 1.20, 1.09, 1.05, 2.03  Avg: 1.34
JavaScriptCore: 0.78, 0.84, 0.83, 1.02  Avg: 0.87
       Nashorn: 1.18, 1.08, 1.02, 1.48  Avg: 1.19
    ChakraCore: 1.00, 1.12, 1.19, 1.75  Avg: 1.27
                                Overall avg: 1.13
         Avg excluding Nashorn & ChakraCore: 1.06	

0002-CLJS-2383-Speed-up-select-keys-no-transient.patch:
	
            V8: 1.28, 1.45, 1.37, 1.33  Avg: 1.36
  SpiderMonkey: 1.54, 1.44, 1.70, 2.17  Avg: 1.71
JavaScriptCore: 1.01, 0.99, 1.03, 1.13  Avg: 1.04
       Nashorn: 1.39, 1.21, 1.14, 1.26  Avg: 1.25
    ChakraCore: 1.20, 1.23, 1.19, 1.39  Avg: 1.25
                                Overall avg: 1.32
         Avg excluding Nashorn & ChakraCore: 1.37
Comment by Mike Fikes [ 19/Nov/17 7:08 PM ]

Summary: If applied, CLJS-2383.patch would be the one to apply as it provides the greatest speedup of all the patches.





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

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

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

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

 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJS-2132] Optimize transient vector creation Created: 27/Jun/17  Updated: 27/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: performance

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

 Description   

This is a very simple optimization around transient []. It avoids copying the empty array.

Performance improvements, for mapv on smallish vectors (5-32) elements anywhere from 20% up to 100% across FF & Chrome.

(defn faster-editable-root
  [node]
  (if (identical? (.-EMPTY_NODE PersistentVector) node)
    (VectorNode. (js-obj) (make-array 32))
    (VectorNode. (js-obj) (aclone (.-arr node)))))
(def orig-editabe-root tv-editable-root)
(enable-console-print!)
(dotimes [_ 2]
  (doseq [size [5 10 40]]
    (let [xs (range size)
          sims 500000]
      (set! tv-editable-root orig-editabe-root)
      (prn "Size: " size)
      (simple-benchmark [] (mapv inc xs) sims)
      (set! tv-editable-root faster-editable-root)
      (prn "NEW:")
      (simple-benchmark [] (mapv inc xs) sims))))





[CLJS-2045] sort-by: Avoid re-creating comparator Created: 20/May/17  Updated: 25/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

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

 Description   

The fn->comparator call should be lifted:

(sort (fn [x y] ((fn->comparator comp) (keyfn x) (keyfn y))) coll)
(let [comparator (fn->comparator comp)]
  (sort (fn [x y] (comparator (keyfn x) (keyfn y))) coll))

Also, fn->comparator is again called on the function in sort, not sure how to avoid that unless we copy the sort code into sort-by.






[CLJS-1601] Optimize cljs.core dump Created: 12/Mar/16  Updated: 19/Nov/17

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: 2
Labels: bootstrap

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

 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.

Comment by David Nolen [ 16/Jun/17 12:41 PM ]

I'm questioning whether this actual valuable? It seems to me if you're serious about code size you would just use Transit and them load the analysis asynchronously?

Comment by Nikita Beloglazov [ 17/Jun/17 2:39 AM ]

Yes, if size is critical then there are better ways to hand-tune the way of loading analysis. At the same time having 3m vs 6m file for local/simple development is a nice win. The only downside is speed, but I feel like big reduction in size is better than small speed penalty.

Comment by Mike Fikes [ 19/Nov/17 7:56 PM ]

Patch no longer applies.





[CLJS-1587] Duplicate keys via quoting Created: 24/Feb/16  Updated: 19/Nov/17

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

Comment by Mike Fikes [ 13/Jun/17 9:40 PM ]

Attached patch no longer applies to master.

Comment by A. R [ 14/Jun/17 1:43 AM ]

It'd be nice if this patch/ticket also included the following case:

(hash-set "a" \a)
Comment by A. R [ 14/Jun/17 10:50 AM ]

Should we increase the scope of this ticket? The same issue exists for maps:

{'0 "a", 0 "b"}
{\a "a", "a" "b"}

I think one possible solution that solves both, quoting and the char/string, could be to to call emit-str in cljs.compiler on the keys/set-members and only then check for uniqueness. Not sure that's a good idea though.

Doesn't solve the hash-set, array-map macros.

Edit: Related ticket: CLJS-2087

Comment by David Nolen [ 14/Jun/17 1:45 PM ]

Increasing the scope of tickets is not desirable. Please move to a separate issue and cross-reference thanks.

Comment by Mike Fikes [ 19/Nov/17 7:57 PM ]

Patch no longer applies.





[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: 1
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-994] print a warning when :externs file paths can't be found. Created: 30/Jan/15  Updated: 19/Nov/17

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

Type: Enhancement Priority: Minor
Reporter: Crispin Wellington Assignee: David Nolen
Resolution: Unresolved Votes: 1
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.

Comment by Mike Fikes [ 19/Nov/17 8:06 PM ]

Patch no longer applies. (Also it doesn't work with git am—see https://clojurescript.org/community/patches.)





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

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

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

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

 Description   

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

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

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



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

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


Before:

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

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

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

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

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


After:

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

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

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

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

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




[CLJS-2120] Optimize keyword function Created: 24/Jun/17  Updated: 25/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: performance

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

 Description   

keyword function can be sped up. This matters when keywords are created dynamically when doing XHR.

The patch now matches Clojure more closely (using substring). It's also optimized for passing strings.

Results:

(enable-console-print!)
(let [sims 1000000]
  (dotimes [_ 2]
    (doseq [x ["foo" "foo/bar" [nil "foo"] ["foo" "bar"] [:foo :bar] [nil :foo]]]
      (prn "Testing keyword with: " x)
      (if (vector? x)
        (do (simple-benchmark [[a0 a1] x] (set! js/FOOO (keyword a0 a1)) sims)
            (simple-benchmark [[a0 a1] x] (set! js/FOOO (keyword2 a0 a1)) sims))
        (do (simple-benchmark [] (set! js/FOOO (keyword x)) sims)
            (simple-benchmark [] (set! js/FOOO (keyword2 x)) sims))))))


"Testing keyword with: " "foo"
[], (set! js/FOOO (keyword x)), 1000000 runs, 194 msecs
[], (set! js/FOOO (keyword2 x)), 1000000 runs, 71 msecs
"Testing keyword with: " "foo/bar"
[], (set! js/FOOO (keyword x)), 1000000 runs, 260 msecs
[], (set! js/FOOO (keyword2 x)), 1000000 runs, 104 msecs
"Testing keyword with: " [nil "foo"]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 278 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 188 msecs
"Testing keyword with: " ["foo" "bar"]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 379 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 215 msecs
"Testing keyword with: " [:foo :bar]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 351 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 207 msecs
"Testing keyword with: " [nil :foo]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 376 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 37 msecs


 Comments   
Comment by A. R [ 24/Jun/17 10:56 AM ]

Changes the behavior of:

((juxt namespace name) (keyword "foo/bar/hmm"))
=> [nil "foo"]
(.-fqn (keyword "foo/bar/hmm"))
=> "foo/bar/hmm"
((juxt namespace name) (keyword2 "foo/bar/hmm"))
=> ["foo" "bar/hmm"]
(.-fqn (keyword2 "foo/bar/hmm"))
=> "foo/bar/hmm"

Clojure 1.9:

((juxt namespace name) (keyword "foo/bar/hmm"))
=> ["foo" "bar/hmm"]

So: yay





[CLJS-2004] Minor fix for test-simple script Created: 10/Apr/17  Updated: 12/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Dejan Josifovic Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: clojurescript, script, test
Environment:

Lubuntu 16.10


Attachments: Text File CLJS-2004.patch     Text File CLJS-2004-rebase.patch    
Patch: Code

 Description   

On Ubuntu based Linux distributions $[] doesn't work.
Output is: Tested with $[ran+1] out of 4 possible js targets
and should be: Tested with 4 out of 4 possible js targets

As in CLJS-929 (for test script), $(()) will work for ash, dash, bash, and zsh.



 Comments   
Comment by Dejan Josifovic [ 10/Apr/17 2:42 PM ]

Add patch.

Comment by Mike Fikes [ 19/Nov/17 7:32 PM ]

Patch no longer applies; needs re-baseline.

Comment by Dejan Josifovic [ 12/Dec/17 5:33 PM ]

Hi,
Sorry for the late replay.
Rebased patch is uploaded.
Regards.





[CLJS-2409] Eliminate cljs.core/divide Created: 21/Nov/17  Updated: 14/Dec/17

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

 Description   

The cljs.core/divide inlining macro was introduced to work around an inability to specify cljs.core//.

For example: https://github.com/clojure/clojurescript/blob/d45012273029bd5df3973ea716394f368e1c44cc/src/main/cljs/cljs/core.cljs#L2593

This can now evidently be removed and thus avoid things like CLJS-594 and divide showing up as a completion in IDEs, etc.



 Comments   
Comment by Mike Fikes [ 21/Nov/17 9:38 PM ]

CLJS-2409-2.patch adds unit tests.

Comment by Mike Fikes [ 14/Dec/17 2:34 PM ]

Adding re-baselined patch.





[CLJS-2038] self calls do not optimize - regression Created: 15/May/17  Updated: 25/Jun/17

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is a regression of:

https://dev.clojure.org/jira/browse/CLJS-275



 Comments   
Comment by A. R [ 18/May/17 2:19 AM ]

This issue can be solve by changing the defn macro for the "simple" case and carrying the function name over to the function:

(core/list 'def (with-meta name m)
  (cons `fn (cons name fdecl)))

This isn't done in clojure because of: https://dev.clojure.org/jira/browse/CLJ-809

Though I don't think that's and issue in CLJS since we don't have real vars anyways and can't redefine {{def}}s.

Comment by A. R [ 17/Jun/17 10:07 AM ]

So the issue that the CLJ ticket has is emulated/shown below in CLJS:

(enable-console-print!)

(defn self-call-test
  [n]
  (prn "inner")
  (when (pos? n)
    (self-call-test (dec n))))

(let [orig self-call-test]
  (set! self-call-test
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test 2)

(def self-call-test2
  (fn self-call-test2
       [n]
       (prn "inner")
       (when (pos? n)
         (self-call-test2 (dec n)))))

(let [orig self-call-test2]
  (set! self-call-test2
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test2 2)

Output in with no optimizations:

"outer"
"inner"
"outer"
"inner"
"outer"
"inner"


"outer"
"inner"
"inner"
"inner"

So: It does seem this would also break the current behaviour, HOWEVER, the above with advance optimizations gives this:

"outer"
"inner"
"inner"
"inner"

*for both*. Given this, it seem better to not change behavior during advanced builds to avoid hard to track down production bugs for the users. Even if this is a slight deviation from CLJ behavior. Thoughts?

Comment by A. R [ 25/Jun/17 2:27 PM ]

Any thoughts on this? I can create a patch if that this change is ok. It could matter a bit (performance wise) since a few of the very low level data structure functions are recursive.





[CLJS-2424] Improve compiler munge performance Nr 2 Created: 28/Nov/17  Updated: 22/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: performance

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

 Description   

This is similar to CLJS-2065 and further improves the performance by avoiding reduce and using a key iterator instead.

Results for a large CLJS project with lots of namespaces are:

  • Initial compile (cold) Old: 11.4s New: 11.2s
  • First full recompile: Old: 6.8s New: 5.9s
  • After a few full recompiler (warmed up JVM): Old: ~6.1s New: 5.1s

lein count:

Ext Files Lines of Code Nodes
cljs 138 23388 424745


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

No difference for compiling ClojureScript tests, I will give a try with something else.





[CLJS-1965] letfn collisions across namespaces Created: 02/Mar/17  Updated: 05/Jan/18

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

Type: Defect Priority: Minor
Reporter: Jeremy R Sellars Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None
Environment:

Affects 1.9.495 and earlier. Probably only affects browsers (not nodejs).


Attachments: Text File CLJS-1965-failing-tests.patch     Text File cljs-1965_wrap_letfn_statements.patch    
Patch: Code

 Description   

If you `letfn` a fn with the same name in two namespaces, the wrong fn is used.

one.cljs
(ns hello-world.one)

(letfn [(answer [] "1")]
  (defn get-answer []
    (answer)))
two.cljs
(ns hello-world.two)

(letfn [(answer [] "2")]
  (defn get-answer []
    (answer)))
core.cljs
(ns hello-world.core
  (:require [hello-world.one]
            [hello-world.two]))

(println "one =>" (hello-world.one/get-answer))  ; one => 1
(println "two =>" (hello-world.two/get-answer))  ; two => 1      WHAT?!?

This issue seems to exist both on top-level `letfn`s and non-top-level `(let [] (letfn [...]))`.



 Comments   
Comment by Jeremy R Sellars [ 02/Mar/17 4:21 PM ]

This patch wraps `letfn` :expr and :statement forms in a function declaration (formerly, only :expr forms were wrapped).

I only did minimal testing. It fixes the code from the description.

Note: This is my first crack at the compiler and it is entirely possible I have not understood the entire problem.

Comment by Mike Fikes [ 16/Jun/17 10:54 AM ]

Confirmed that this fixes things downstream in self-hosted ClojureScript (Planck):

Without the patch:

cljs.user=> (require 'hello-world.core)
one => 2
two => 2
nil

With the patch:

cljs.user=> (require 'hello-world.core)
one => 1
two => 2
nil
Comment by daniel sutton [ 29/Dec/17 3:18 PM ]

This doesn't require different namespaces. The bug is that `let-fn` is putting its binding as a global variable.

And easy reproduction is
1. `lein new mies letfn-bug`,
2. update cljs version to `[org.clojure/clojurescript "1.9.946"]`
3. and then

(ns letfn-bug.core
  (:require [clojure.browser.repl :as repl]))

(enable-console-print!)

(letfn [(non-unique-name [] 4)]
  (defn f1 [] (non-unique-name)))

(letfn [(non-unique-name [] 5)]
  (defn f2 [] (non-unique-name)))

(println "should be 4-> " (f1))
(println "should be 5-> " (f2))

then `scripts/repl`.

results in:

cljs.user=> (load-file "letfn_bug/core.cljs")
should be 4->  5
should be 5->  5
nil
cljs.user=>

With the generated js:

// Compiled by ClojureScript 1.9.946 {}
goog.provide('letfn_bug.core');
goog.require('cljs.core');
goog.require('clojure.browser.repl');
cljs.core.enable_console_print_BANG_.call(null);
var non_unique_name = (function letfn_bug$core$non_unique_name(){
return (4);
});
letfn_bug.core.f1 = (function letfn_bug$core$f1(){
return non_unique_name.call(null);
});
var non_unique_name = (function letfn_bug$core$non_unique_name(){
return (5);
});
letfn_bug.core.f2 = (function letfn_bug$core$f2(){
return non_unique_name.call(null);
});
cljs.core.println.call(null,"should be 4-> ",letfn_bug.core.f1.call(null));
cljs.core.println.call(null,"should be 5-> ",letfn_bug.core.f2.call(null));
Comment by David Nolen [ 05/Jan/18 10:42 AM ]

Quick review of the patch, instead of always wrapping in a statement context it might be better if we only did it at the top-level where this is actually a problem.





[CLJS-2459] Compiler should emit warning about namespace and/or non-existent var Created: 05/Jan/18  Updated: 06/Jan/18

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

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

Approval: Vetted

 Description   

Made project as described in quickstart.

hello-world/core.cljs

(ns hello-world.core
  (:require [hello-world.foo]))

(enable-console-print!)

(println "Hello world!")
(hello-world.foo/foo)
;; no warning

hello_world/foo.cljs

(ns hello-world.foo2)
(defn foo []
  (println "o no"))

Note that `foo.cljs` has a namespace name that is not consistent with the file structure.

I expect
1) a warning about `(:require [hello-world.foo])` not being able to find the namespace, and/or
2) a warning about `(hello-world.foo/foo)` being a non-existent var.






[CLJS-2466] Faster reduce for lazy-seqs Created: 12/Jan/18  Updated: 13/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2466.patch    

 Description   

Lazy seqs that are built from vectors/chunked-seqs can sometimes take a slow first/next reduce.

Observation:

(def xs (vec (range 3000000)))

(time (reduce max xs)) ;; #1: 130ms, (reference)
(time (reduce max (lazy-cat xs))) ;; #2: 130ms, just as fast
(time (reduce max 0 (lazy-cat xs))) ;; #3: 500ms, 4x slower!!

;; Double them with concat and they're not 2x slower but 10x slower:
(time (reduce max (lazy-cat xs xs))) ;; #4: 1200ms
(time (reduce max 0 (lazy-cat xs xs))) ;; #5: 1200ms

Explanation for #3: The problem is that seq-reduce when called without init will properly call reduce again and take a fast path. With init given it won't ever escape to a faster reduce but will stick to first/next.

Note: In Clojure they scale "properly" (first 3 are ~45ms, last 2 are ~110ms).

The reason is that Clojure properly escapes to a fast path:

https://github.com/clojure/clojure/blob/2b242f943b9a74e753b7ee1b951a8699966ea560/src/clj/clojure/core/protocols.clj#L131-L143

An improved seq-reduce could look like this:

(defn seq-reduce
  ([f coll]
   (if-let [s (seq coll)]
     (reduce f (first s) (next s))
     (f)))
  ([f val coll]
   (loop [val val, coll (seq coll)]
     (if coll
       (if (chunked-seq? coll)
         (if (implements? IReduce coll)
           (reduce f val coll)
           (let [nval (reduce f val (chunk-first coll))]
             (if (reduced? nval)
               @nval
               (recur nval (chunk-next coll)))))
         (let [nval (f val (first coll))]
           (if (reduced? nval)
             @nval
             (recur nval (next coll)))))
       val))))

This reduces the times for #3: 160ms, #4: 380ms, #5: 380ms.

This is an RFC, since I'm not 100% certain about the implemenation. Need to be careful to not blow the stack here...

Questions:
1. Should ChunkedCons implement IReduce? I think so.



 Comments   
Comment by A. R [ 12/Jan/18 1:59 AM ]

Timings with advanced compilation:

CHROME:
"OLD"
#1: "Elapsed time: 21.870000 msecs"
#2: "Elapsed time: 25.485000 msecs"
#3: "Elapsed time: 134.900000 msecs"
#4: "Elapsed time: 317.155000 msecs"
#5: "Elapsed time: 313.225000 msecs"
"NEW"
#1: "Elapsed time: 23.290000 msecs"
#2: "Elapsed time: 19.445000 msecs"
#3: "Elapsed time: 20.075000 msecs"
#4: "Elapsed time: 102.895000 msecs"
#5: "Elapsed time: 100.430000 msecs"
"OLD"
#1: "Elapsed time: 19.585000 msecs"
#2: "Elapsed time: 19.475000 msecs"
#3: "Elapsed time: 87.805000 msecs"
#4: "Elapsed time: 303.340000 msecs"
#5: "Elapsed time: 291.680000 msecs"
"NEW"
#1: "Elapsed time: 20.760000 msecs"
#2: "Elapsed time: 19.690000 msecs"
#3: "Elapsed time: 18.960000 msecs"
#4: "Elapsed time: 101.385000 msecs"
#5: "Elapsed time: 101.290000 msecs"

FIREFOX:

"OLD"
#1: "Elapsed time: 7.880000 msecs"
#2: "Elapsed time: 7.820000 msecs"
#3: "Elapsed time: 69.460000 msecs"
#4: "Elapsed time: 197.800000 msecs"
#5: "Elapsed time: 209.300000 msecs"
"NEW"
#1: "Elapsed time: 7.380000 msecs"
#2: "Elapsed time: 7.360000 msecs"
#3: "Elapsed time: 8.100000 msecs"
#4: "Elapsed time: 110.640000 msecs"
#5: "Elapsed time: 89.960000 msecs"
"OLD"
#1: "Elapsed time: 6.520000 msecs"
#2: "Elapsed time: 7.360000 msecs"
#3: "Elapsed time: 106.920000 msecs"
#4: "Elapsed time: 252.300000 msecs"
#5: "Elapsed time: 249.800000 msecs"
"NEW"
#1: "Elapsed time: 7.360000 msecs"
#2: "Elapsed time: 6.940000 msecs"
#3: "Elapsed time: 6.880000 msecs"
#4: "Elapsed time: 99.380000 msecs"
#5: "Elapsed time: 53.220000 msecs"
Comment by A. R [ 12/Jan/18 2:14 AM ]

Impact on truly (unchunked) lazy-seqs is neglibile (hard to measure due to garbage collection causing a lot of variance).

Comment by A. R [ 12/Jan/18 10:22 AM ]

New ticket for reduce for ChunkedCons: CLJS-2468

Comment by A. R [ 13/Jan/18 7:00 AM ]

Depends on CLJS-2469 for tests to pass. Patch is attached.





[CLJS-2469] ChunkedCons chunk-next returns empty seq instead of nil Created: 13/Jan/18  Updated: 21/Jan/18

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

There is a bug in ChunkedCons. In Clojure ChunkedCons (correctly) always calls seq in chunked-next. In CLJS it's not done. But since ChunkedCons has to be lazy it pretty much always gets an (empty) lazy seq as the "more" part.

Bug:

(-> (map inc (vec (range 64)))
    seq
    chunk-next
    seq
    chunk-next)

Returns and empty sequence instead of nil. This hasn't surfaced yet since nothing calls chunk-next on a ChunkedCons (yet).



 Comments   
Comment by A. R [ 13/Jan/18 7:26 AM ]

Found another bug that surfaced: Current implementation calls -seq on more which could be nil and this would blow up. So the patch also make a tiny change to -next also just calling seq on more. Pretty straight forward.

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

This patch needs a test.

Comment by A. R [ 20/Jan/18 12:27 AM ]

Test's added.





[CLJS-1439] Add type annotations to goog-define defined vars Created: 01/Sep/15  Updated: 29/Oct/17

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

Type: Enhancement Priority: Trivial
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Unresolved Votes: 4
Labels: None

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

 Description   

Currently it's still required to annotate goog-define constants with ^boolean to allow the Closure compiler to safely remove dead branches. The macro could emit the var with ^boolean metadata to make this unnecessary.

In general it would be nice to have similar annotations for the already defined constants like goog.DEBUG although I'm not sure how/if that's possible.



 Comments   
Comment by Thomas Heller [ 28/Oct/17 5:25 PM ]

Added patch sets the proper :tag for the defined symbol for goog-define.

Inferring a type from a JS expression is not that simple and should probably be done in a separate ticket.

Comment by Thomas Heller [ 28/Oct/17 5:30 PM ]

Patch included a change from another ticket, fixed to only include the intended change.





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

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

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-1195] generic reusable command line argument parsing for REPLs Created: 10/Apr/15  Updated: 27/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Jason Courcoux
Resolution: 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.

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

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

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

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

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

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





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

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

Type: Enhancement Priority: Major
Reporter: Arne Brasseur Assignee: Juho Teperi
Resolution: Unresolved Votes: 1
Labels: None


 Description   

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

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

This becomes

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

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

This seems like something Google Closure should address.

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



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

To test

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

Compiler options

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

Code

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

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

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

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

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

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

This is what webpack generates

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

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

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

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





[CLJS-1047] externs checking for js/foo Created: 19/Feb/15  Updated: 27/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Maria Geller
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Worth looking into validating `js/foo` forms again the known externs set. Can probably be done by leveraging the Closure JS Parser.



 Comments   
Comment by Michael Griffiths [ 22/Feb/15 12:03 PM ]

Would you consider making the results of parsing available to tooling (e.g. in cljs.env/*compiler*)? I would use this to add support for autocompletion of js/ forms to CIDER.

Comment by David Nolen [ 23/Feb/15 8:15 AM ]

Definitely open to the idea of exposing this information to other tooling when we get there.

Comment by Thomas Heller [ 16/Jun/17 4:09 PM ]

Closure warns about with DiagnosticGroups/UNDEFINED_VARIABLES enabled.





[CLJS-246] Use protocol mask test in protocol fns Created: 09/May/12  Updated: 29/Jul/13

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Michał Marczyk
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-246-have-protocol-methods-check-bitmasks-for-fa.patch    

 Description   

This is a performance win on many browsers.

http://jsperf.com/direct-vs-chain/8



 Comments   
Comment by Michał Marczyk [ 10/May/12 1:11 PM ]

See CLJS-247 for comments relevant to this patch.

Comment by David Nolen [ 10/May/12 3:30 PM ]

Not seeing much of a perf benefit from this, though Michal reports differently. More investigation is needed.

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

patch no longer applies. I wonder if I see bad behavior because I was testing with node or it was prior to the fixes around avoiding deoptimization.

Comment by Michał Marczyk [ 17/Jun/12 9:28 PM ]

I'll bring it up to date with the recent changes, thanks for the prod!





[CLJS-1561] WARN if recur passes non-inferred type Created: 06/Feb/16  Updated: 13/Jun/17

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: Mike Fikes
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
Comment by Mike Fikes [ 13/Jun/17 9:31 PM ]

Assigning to take another look at this. The all of the examples provided in this ticket are still working apart from the first, which no longer appears to cause the diagnostic to be emitted.





[CLJS-2438] Self-host: script/test-self-parity abends sometimes Created: 08/Dec/17  Updated: 08/Dec/17

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


 Description   

Sometimes the script/test-self-parity tests terminate early with

Testing with Node

Testing self-host.test
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1

Ran 33 tests containing 206 assertions.
0 failures, 0 errors.
x

Full transcript:

$ script/test-self-host
Analyzing file:/Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/core.cljs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/core.cljs, elapsed time: 2548.700649 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/impl/inspect.cljs to builds/out-self/cljs/tools/reader/impl/inspect.cljs
Compiling builds/out-self/cljs/tools/reader/impl/inspect.cljs, elapsed time: 32.942144 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/clojure/string.cljs, elapsed time: 67.24495 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/impl/utils.cljs to builds/out-self/cljs/tools/reader/impl/utils.cljs
Compiling /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/env.cljc, elapsed time: 7.9361 msecs
Compiling builds/out-self/cljs/tools/reader/impl/utils.cljs, elapsed time: 52.978771 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/reader_types.cljs to builds/out-self/cljs/tools/reader/reader_types.cljs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/clojure/set.cljs, elapsed time: 63.896603 msecs
Compiling builds/out-self/cljs/tools/reader/reader_types.cljs, elapsed time: 65.685336 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/impl/errors.cljs to builds/out-self/cljs/tools/reader/impl/errors.cljs
Compiling builds/out-self/cljs/tools/reader/impl/errors.cljs, elapsed time: 47.949386 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/impl/commons.cljs to builds/out-self/cljs/tools/reader/impl/commons.cljs
Compiling builds/out-self/cljs/tools/reader/impl/commons.cljs, elapsed time: 26.952248 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader.cljs to builds/out-self/cljs/tools/reader.cljs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/source_map/base64.cljs, elapsed time: 4.063721 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/source_map/base64_vlq.cljs, elapsed time: 15.036982 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/source_map.cljs, elapsed time: 125.606578 msecs
Compiling builds/out-self/cljs/tools/reader.cljs, elapsed time: 213.744986 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/tools.reader-1.1.0.jar!/cljs/tools/reader/edn.cljs to builds/out-self/cljs/tools/reader/edn.cljs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/clojure/walk.cljs, elapsed time: 15.457761 msecs
Compiling builds/out-self/cljs/tools/reader/edn.cljs, elapsed time: 87.207366 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/reader.cljs, elapsed time: 69.046241 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/tagged_literals.cljc, elapsed time: 11.240989 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/gen/alpha.cljs, elapsed time: 260.765109 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/spec/alpha.cljs, elapsed time: 947.753769 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/analyzer.cljc, elapsed time: 1202.162467 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/nodejs.cljs, elapsed time: 3.422987 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/pprint.cljs, elapsed time: 1300.527383 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/compiler.cljc, elapsed time: 403.391288 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/test.cljs, elapsed time: 143.651189 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/cljs/cljs/js.cljs, elapsed time: 1554.368126 msecs
Compiling src/test/self/self_host/test.cljs, elapsed time: 1493.203925 msecs
Compiling /Users/mfikes/Projects/clojurescript/src/main/clojure/cljs/core.cljc, elapsed time: 7060.124722 msecs
Compile sources, elapsed time: 14324.531439 msecs
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/base.js to builds/out-self/goog/base.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/string/string.js to builds/out-self/goog/string/string.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/debug/error.js to builds/out-self/goog/debug/error.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/dom/nodetype.js to builds/out-self/goog/dom/nodetype.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/asserts/asserts.js to builds/out-self/goog/asserts/asserts.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/array/array.js to builds/out-self/goog/array/array.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/object/object.js to builds/out-self/goog/object/object.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/structs/structs.js to builds/out-self/goog/structs/structs.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/functions/functions.js to builds/out-self/goog/functions/functions.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/math/math.js to builds/out-self/goog/math/math.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/iter/iter.js to builds/out-self/goog/iter/iter.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/structs/map.js to builds/out-self/goog/structs/map.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/uri/utils.js to builds/out-self/goog/uri/utils.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/uri/uri.js to builds/out-self/goog/uri/uri.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/math/integer.js to builds/out-self/goog/math/integer.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/string/stringbuffer.js to builds/out-self/goog/string/stringbuffer.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/reflect/reflect.js to builds/out-self/goog/reflect/reflect.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/math/long.js to builds/out-self/goog/math/long.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/labs/useragent/util.js to builds/out-self/goog/labs/useragent/util.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/labs/useragent/platform.js to builds/out-self/goog/labs/useragent/platform.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/labs/useragent/engine.js to builds/out-self/goog/labs/useragent/engine.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/crypt/crypt.js to builds/out-self/goog/crypt/crypt.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/labs/useragent/browser.js to builds/out-self/goog/labs/useragent/browser.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/useragent/useragent.js to builds/out-self/goog/useragent/useragent.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/useragent/product.js to builds/out-self/goog/useragent/product.js
Copying jar:file:/Users/mfikes/Projects/clojurescript/lib/google-closure-library-0.0-20170809-b9c14c6b.jar!/goog/crypt/base64.js to builds/out-self/goog/crypt/base64.js
Applying optimizations :simple to 55 sources
Optimizing with Google Closure Compiler, elapsed time: 17638.02089 msecs
Optimizing 55 sources, elapsed time: 18894.849293 msecs
Testing with Node

Testing self-host.test
WARNING: double already refers to: cljs.core/double being replaced by: cljs.user/double at line 1

Ran 33 tests containing 206 assertions.
0 failures, 0 errors.
x





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

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

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Robin Hermansson
Resolution: Unresolved Votes: 0
Labels: newbie


 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.



 Comments   
Comment by Robin Hermansson [ 07/May/17 8:03 AM ]

I would like to work on this issue.

Comment by David Nolen [ 08/May/17 7:14 PM ]

Robin, go for it





[CLJS-1628] Make instances of js/Symbol printable Created: 20/Apr/16  Updated: 19/Nov/17

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

Comment by Mike Fikes [ 19/Nov/17 7:54 PM ]

Patch no longer applies.





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

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

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

Approval: Vetted




[CLJS-2344] de-dupe externs Created: 31/Aug/17  Updated: 16/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Sameer Rahmani
Resolution: Unresolved Votes: 1
Labels: closure

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

 Description   

We could possibly call distinct on externs to remove dupes in the common case where externs may appear multiple times accidentally on the classpath.



 Comments   
Comment by David Nolen [ 16/Jan/18 8:09 AM ]

I don't think we want to silently dedupe. We probably want to also warn so users can fix the issue. The only reason to even do this ticket is because the Closure warning is so unfriendly and fails the build.

Comment by Sameer Rahmani [ 16/Jan/18 8:33 AM ]

got it, I'll improve the patch





[CLJS-1466] Absolute paths in :output-dir break Node.js shim for :none Created: 11/Oct/15  Updated: 19/Nov/17

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

Type: Defect Priority: Major
Reporter: Sebastian Bensusan Assignee: Sebastian Bensusan
Resolution: Unresolved Votes: 0
Labels: bug

Attachments: Text File cljs_1466.patch    
Patch: Code

 Description   

When compiling a trivial example with the following script:

(require 'cljs.build.api)

(cljs.build.api/build "src"
  {:main 'hello.core
   :output-to "main.js"
   :output-dir "/home/carlos/Playground/node-abs/out"
   :target :nodejs})

It generates code that tries to resolve the following path:

/home/carlos/Playground/node-abs/home/carlos/Playground/node-abs/out/goog/bootstrap/nodejs.js

We should check if the provided path for :output-dir is absolute before resolving it in the Node.js :none shim. The shim has a related ticket in CLJS-1444.

Even if it's uncommon for users to have absolute paths, tooling might need to.



 Comments   
Comment by Sebastian Bensusan [ 11/Oct/15 4:28 PM ]

The attach patch cljs_1466.patch solves the issue by using path.resolve which takes into account relative vs absolute paths when joining paths. Successfully tested in the example repo with both relative and absolute :output-dir

Comment by Martin Klepsch [ 14/Oct/15 3:57 AM ]

Looking at the patch it seems that it might break current behaviour in some cases? Have you thought about that?

CLJS-1444 [1] would probably also break the shim in some way so would be good to get these in together and be very clear about what will break etc. As long as we come up with a robust and predictable impl this is something worth breaking imo.

[1] http://dev.clojure.org/jira/browse/CLJS-1444 for

Comment by David Nolen [ 14/Oct/15 8:05 AM ]

Yes would like to get feedback from people already heavily invested in ClojureScript + Node.js before moving forward on this.

Comment by Sebastian Bensusan [ 14/Oct/15 10:31 AM ]

Martin Klepsch: I did think about breakage but I couldn't find any cases. Do you have an example one? In the example repo I've put together some tests (by running ./script/test.sh) but it boils down to path.join(path.resolve("."),paths) being equivalent to path.resolve(paths) for all relative paths, since the "Resolve to absolute" method is the same for both (process.cwd() inside of path.resolve). When considering absolute paths, only the new version does the right thing.

On the other hand, those tests also reveal that the proposed patch doesn't cover CLJS-1446 as I originally thought since

node main.js

succeeds while:

cd ..
node node-abs/main.js

fails.

Comment by Mike Fikes [ 19/Nov/17 8:00 PM ]

Patch no longer applies.





[CLJS-2063] Auto-optimize equality check of goog-defines in advanced mode Created: 31/May/17  Updated: 16/Jun/17

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

Type: Enhancement Priority: Minor
Reporter: Shaun LeBron Assignee: Shaun LeBron
Resolution: Unresolved Votes: 0
Labels: None


 Description   

per discussion: https://clojurians-log.clojureverse.org/cljs-dev/2017-05-30.html






[CLJS-2043] Remove checks made redundant by CLJS-2040 Created: 20/May/17  Updated: 20/May/17

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

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


 Description   

CLJS-2040 added a check to see if we have analyzer metadata for a given ns. The old checks are still in there but would also be covered by the new check. We should probably remove them.

https://github.com/clojure/clojurescript/commit/77e01a01af9f45c76cfa34aa67bfae154b075544#diff-55b85385d2d0bfb6dc20d59ed982d5c8R953






[CLJS-1866] RangedIterator performance tweaks Created: 08/Dec/16  Updated: 19/Dec/16

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

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

Attachments: Text File CLJS-1866.patch     Text File CLJS-1866-updated.patch     Text File CLJS-1866-updated.patch    
Patch: Code

 Description   

The attached patch simplifies and speeds up the RangedIterator.

The benchmarks were run using the following function to test vector iteration:

(defn consume-iterator
  [v]
  (let [iter (-iterator v)]
    (loop []
      (when (.hasNext iter)
        (.next iter)
        (recur)))))

A series of "simple-benchmarks" were setup as follows:

(simple-benchmark [v (into [] (range N))] (consume-iterator v) I)

Where 'N' and 'I' were values from the 'Vector Size' and 'Iterations' columns of the table below .

Vector Size Iterations V8 Speed [msec] (master) V8 Speed [msec] (patch) JSC Speed [msec] (master) JSC Speed [msec] (patch)
1 100,000 15 11 13 7
2 100,000 14 10 7 4
4 100,000 18 10 9 5
8 100,000 27 12 14 6
16 100,000 43 17 19 9
32 100,000 74 24 37 15
100 100,000 217 59 105 45
1000 100,000 2008 524 1032 392
10,000 100,000 20390 5856 10249 4178
100,000 10,000 20334 5324 10324 4387

Javascript engine versions used:

  • V8 version 5.1.281.47
  • JSC version Unknown

The RangedIterator constructor function `ranged-iterator` was also made private.



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

Let's get a patch with the performance change without altering the constructor first. Thanks.

Comment by Thomas Mulvaney [ 17/Dec/16 7:15 AM ]

Rebased and constructor no longer private.

Comment by David Nolen [ 17/Dec/16 9:59 AM ]

Sorry for not being clear. Leave the fields of the deftype alone even if we aren't using them for now. We want the performance enhancements without changing the API at all.

Comment by Thomas Mulvaney [ 19/Dec/16 5:58 AM ]

Thanks that makes sense. Fixed in this patch.





[CLJS-1888] Seqs of PHMs and PAMs do not handle metadata correctly Created: 13/Jan/17  Updated: 16/Jun/17

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

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

Attachments: Text File CLJS-1888.patch    

 Description   

Metadata on parent seq ends up being passed to the next seq. Calling `empty` on a seq also ends up carrying metadata.

Examples:

(def s (with-meta (seq {:a 1 :b 2}) {:some :meta}))

(meta s) => {:some :meta} ;; Good
(meta (rest s))  => {:some :meta} ;; Bad, expected nil
(meta (next s))  => {:some :meta} ;; Bad, expected nil
(meta (empty s)) => {:some :meta} ;; Bad, expected nil


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

Patch no longer applies to master.





[CLJS-364] compiler needs to put all args of an invocation after 20 into an array-seq Created: 29/Aug/12  Updated: 29/Aug/12

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

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


 Description   

This ticket is related to CLJS-359






[CLJS-1479] Race condition in browser REPL Created: 03/Nov/15  Updated: 12/Feb/16

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

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

Attachments: File heavy-load.sh     File race-condition.clj     File race-condition.jstack    

 Description   

Evaluation in browser REPL occasionally hangs. It seems that repl environment and browser sometimes miss each other and their "randezvous" fails. Browser is waiting for POST reply and repl is trying to send a command, but they do not meet each other.

I found the issue when we switched our tests from nodejs to browser environment. Luckily I was able to find very small example which hangs during execution. It seems that (simulated) heavy load increases the chance of "hanging".

Minimal setup:

(ns race.condition
  (:require [cljs.repl.browser :as browser]
            [cljs.repl :as repl]
            [cljs.env :as env]
            [cljs.build.api :as api]))


(api/build '[(ns race.repl
               (:require [clojure.browser.repl]))
             (clojure.browser.repl/connect "http://localhost:9000/repl")]
           {:output-to  "target/cljs-race/main.js"
            :output-dir "target/cljs-race"
            :main       'race.repl})

(spit "target/cljs-race/index.html"
      (str "<html>" "<body>"
           "<script type=\"text/javascript\" src=\"main.js\">"
           "</script>" "</body>" "</html>"))

Now start the environment:

(def env (browser/repl-env :static-dir ["target/cljs-race" "."] :port 9000 :src nil))

(env/with-compiler-env (env/default-compiler-env)
  (repl/-setup env {}))

cross your fingers and start this endless loop:

(loop [i 0]
  (println (java.util.Date.) i)
  (dotimes [j 100]
    (let [result (repl/-evaluate env "<exec>" "1"  "true")]
      (when-not (= :success (:status result))
        (println i j result))))
  (recur (inc i)))

To simulate heavy load run heavy-load.sh from attachment.

After some iterations (eg 55 big loop i) execution stops. If you investigate stacks (see race-condition.jstack), you can see in one thread:

at clojure.core$promise$reify__6779.deref(core.clj:6816)
	at clojure.core$deref.invoke(core.clj:2206)
	at cljs.repl.browser$send_for_eval.invoke(browser.clj:65)
	at cljs.repl.browser$browser_eval.invoke(browser.clj:193)
	at cljs.repl.browser.BrowserEnv._evaluate(browser.clj:262)

The code is waiting for a promise with a connection (which already did arive).

My guess is suspicious code in cljs.repl.server functions connection and set-connection. Both functions access an atom in non-standard way. They deref a valua and make a swap! in two steps.

Can somebody with better understanding of REPL internals investigate? Thank you.



 Comments   
Comment by David Nolen [ 12/Feb/16 2:57 PM ]

A patch is welcome for this one.





[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-525] Allow hashtable lookup used for numbers and strings to be extended to other built-in types Created: 17/Jun/13  Updated: 17/Jun/13

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

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

...which would enable safe extension of key cljs protocols to types without modifying their prototypes, e.g. CLJS-523.



 Comments   
Comment by David Nolen [ 17/Jun/13 2:56 PM ]

Date is the only JS native case that I'm aware of that we don't handle. One tricky bit is that goog.typeOf won't give us the information we need, but I think instanceof should cover us here?

Comment by Fogus [ 17/Jun/13 3:05 PM ]

instanceof or the ever-gruesome toString.call(aDate) == '[object Date]' will work.





[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-1833] Consider moving int behavior to unchecked-int + clearer docstrings for limitations of other coercions (long etc) Created: 23/Oct/16  Updated: 23/Oct/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





[CLJS-1900] Source maps for processed JavaScript modules Created: 24/Jan/17  Updated: 24/Jan/17

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

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


 Description   

Currently we don't emit source maps for JS modules converted into Google Closure namespaces.






[CLJS-1918] case needs a type hint for keywords case when using *warn-on-infer* Created: 30/Jan/17  Updated: 30/Jan/17

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

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





[CLJS-2095] Nashorn runner Created: 16/Jun/17  Updated: 27/Jun/17

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

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


 Description   

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






[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: 27/Jun/17

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

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: 27/Jun/17

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

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-1402] Source Mapping Closure Error Logger Created: 08/Aug/15  Updated: 27/Jun/17

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

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



 Comments   
Comment by Antonin Hildebrand [ 24/Jan/17 8:06 PM ]

I believe this will be fixed by CLJS-1901 either using `--source_map_input` or inlining source-maps into generated js files (CLJS-1902).





[CLJS-1373] Generalize CLJS-1324, check invokes of all IFn implementors Created: 28/Jul/15  Updated: 27/Jun/17

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

Type: Enhancement Priority: Major
Reporter: 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-1147] reconnect logic for browser REPLs Created: 18/Mar/15  Updated: 27/Jun/17

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

Type: Enhancement Priority: Major
Reporter: 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-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 27/Jun/17

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

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-2165] port `clojure.string/sre-quote-replacement` to ClojureScript, prevent use the conditional reader in cljc file Created: 04/Jul/17  Updated: 04/Jul/17

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

Type: Feature Priority: Major
Reporter: Isaac Zeng Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript


Attachments: Text File 0001-port-re-quote-replacement-to-clojure.string.patch    

 Description   

Clojure has `clojure.string/sre-quote-replacement`, but not in ClojureScript.

for this now, we need use the conditional reader in cljc file
```
(defn foo [s]
#?(:clj (str/re-quote-replacement s)
:cljs (gstr/regExpEscape s)))
```






[CLJS-365] apply needs to put all args after the 20th into an array seq Created: 29/Aug/12  Updated: 05/Jul/17

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

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


 Description   

This ticket is related to CLJS-359






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

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

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


 Description   

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

Repro with QuickStart JAR:

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

Compare to Clojure:

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

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

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






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

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

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

Approval: Vetted




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

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core


 Description   

For defns with a variadic arity: if invoked with a missing fixed arity, they use the variadic method instead of erroring; if invoked with a fixed arity that is the max fixed arity, variadic mathod instead of fixed form is invoked.

(defn f-hole
  ([a] 1)
  ([a b c d & args] "4 or more"))

(f-hole 1 2) ; =>"4 or more", should be error

(defn f-overlap-mfa
  ([a b] 2)
  ([a b & c] "2+"))

(f-overlap-mfa 1) ;=> "2+", should be error
(f-overlap-mfa 1 2) ;=> "2+", should be 2
(f-overlap-mfa 1 2 3) ;=> "2+", correct

A way to fix the f-hole bug is to emit a "case X:" into the switch statement for all X with no signature or less than max-fixed-arity.

The f-overlap-mfa I'm not sure why is happening and didn't investigate deeply.



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

Sorry, filed against CLJ instead of CLJS!

Comment by Rohit Aggarwal [ 12/Jun/16 9:41 AM ]

The behaviour I am seeing for f-overlap-mfa is:

(f-overlap-mfa 1) ;=> "2+"
(f-overlap-mfa 1 2) ;=> 2
(f-overlap-mfa 1 2 3) ;=> "2+"

So the two argument result is different for me than you, Francis Avila.

The call with just one argument does give a warning though:

WARNING: Wrong number of args (1) passed to cljs.user/f-overlap-mfa





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

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

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

Approval: Accepted

 Description   

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






[CLJS-2155] building fileUrl on windows in repl.cljc results with java.net.UnknownHostException Created: 03/Jul/17  Updated: 19/Aug/17

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

Type: Defect Priority: Major
Reporter: Vojimir Golem Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows OS



 Description   

I think that the following line might cause the problem on windows:
https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/repl.cljc#L713

For file param e.g. "src\duct3\client.cljs"

(str "file://" (.getAbsolutePath file))

evaluates on windows as:
"file://C:\Projects\Playground\duct3\src\duct3\client.cljs"

which is not legal file Url (https://en.wikipedia.org/wiki/File_URI_scheme#Windows)

and final result is: java.net.UnknownHostException (java treat that URL as FTP address).



 Comments   
Comment by Vojimir Golem [ 19/Aug/17 10:02 AM ]

Permalink to mentioned line:
https://github.com/clojure/clojurescript/blob/98656d305e6447c62e36849ee615532d53211fdd/src/main/clojure/cljs/repl.cljc#L736





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

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

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

Approval: Vetted

 Description   

If you have this code,

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

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

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

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

This causes the compiler to derail with this:

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





[CLJS-1410] Support source maps in deps.cljs Created: 09/Aug/15  Updated: 12/Feb/16

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

Type: Enhancement Priority: Major
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None


 Description   

There should be support to package source maps with a foreign-lib using deps.cljs



 Comments   
Comment by David Nolen [ 12/Feb/16 3:00 PM ]

Patch welcome for this one!





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

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

Type: Defect Priority: Major
Reporter: Chad Killingsworth Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

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

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

Thanks,

Chad Killingsworth



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

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

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





[CLJS-2399] :foreign-libs with module conversion does not works properly (possible regression) Created: 16/Nov/17  Updated: 16/Nov/17

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

Type: Defect Priority: Major
Reporter: Andrey Antukh Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

OpenJDK 8, Linux, node-v8.7.0



 Description   

This is the same issue as https://dev.clojure.org/jira/browse/CLJS-1682, so all the instructions for reproduce the error can be found in the referenced issue.

Here some specific details of the different errors on different cljs versions:

With `cljs==1.9.845` the compilation terminates successfully in both cases (`:foreign-libs` in `build.clj` and in `deps.cljs`) but on executing the generated javascript, an exception is raised:

$ node out/main.js                            
out/src/vendor/greeter.js:3
exports.sayHello = function(name) {
                 ^

TypeError: Cannot set property 'sayHello' of undefined
    at out/src/vendor/greeter.js:3:18
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at Object.nodeGlobalRequire (/home/niwi/tmp/greeter/out/goog/bootstrap/nodejs.js:85:8)
    at Object.cljs$core$load_file [as load_file] (/home/niwi/tmp/greeter/out/cljs/core.js:341:13)
    at Object.<anonymous> (/home/niwi/tmp/greeter/out/testapp/core.js:5:11)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)

With `cljs==1.9.908` the compilation terminates successfully in both cases but on executing the generated javascript and other exception is raised:

$ node out/main.js                            
/home/niwi/tmp/greeter/out/testapp/core.js:28
return cljs.core.println.call(null,module$src$vendor$greeter.sayHello("Ciri"));
                                                             ^

TypeError: module$src$vendor$greeter.sayHello is not a function
    at Function.testapp.core._main.cljs$core$IFn$_invoke$arity$variadic (/home/niwi/tmp/greeter/out/testapp/core.js:28:62)
    at testapp$core$_main (/home/niwi/tmp/greeter/out/testapp/core.js:24:27)
    at Object.cljs$core$apply_to [as apply_to] (/home/niwi/tmp/greeter/out/cljs/core.js:12793:81)
    at Function.cljs.core.apply.cljs$core$IFn$_invoke$arity$2 (/home/niwi/tmp/greeter/out/cljs/core.js:13237:18)
    at cljs$core$apply (/home/niwi/tmp/greeter/out/cljs/core.js:13195:24)
    at Object.<anonymous> (/home/niwi/tmp/greeter/out/DF0FC10.js:9:17)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)

And with `cljs==1.9.946`, using the `:foreign-libs` in the `build.clj` file, the following error is raised:

$ java -cp cljs.jar:src clojure.main build.clj 
module.js:529
    throw err;
    ^

Error: Cannot find module '@cljs-oss/module-deps'
    at Function.Module._resolveFilename (module.js:527:15)
    at Function.Module._load (module.js:476:23)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at [eval]:3:13
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:624:30)
    at evalScript (bootstrap_node.js:462:27)

Copying file:/home/niwi/tmp/greeter/src/vendor/greeter.js to out/src/vendor/greeter.js
Copying jar:file:/home/niwi/tmp/greeter/cljs.jar!/cljs/core.cljs to out/cljs/core.cljs
Reading analysis cache for jar:file:/home/niwi/tmp/greeter/cljs.jar!/cljs/core.cljs
Compiling out/cljs/core.cljs
[...]

Independently of the error, the compilation continues and terminates, and the generated javascript raises the following error (very similat to the error with cljs==1.9.908):

$ node out/main.js                                                                 
/home/niwi/tmp/greeter/out/testapp/core.js:28
return cljs.core.println.call(null,module$home$niwi$tmp$greeter$src$vendor$greeter.sayHello("Ciri"));
                                                                                   ^

TypeError: module$home$niwi$tmp$greeter$src$vendor$greeter.sayHello is not a function
    at Function.testapp.core._main.cljs$core$IFn$_invoke$arity$variadic (/home/niwi/tmp/greeter/out/testapp/core.js:28:84)
    at testapp$core$_main (/home/niwi/tmp/greeter/out/testapp/core.js:24:27)
    at Object.cljs$core$apply_to [as apply_to] (/home/niwi/tmp/greeter/out/cljs/core.js:12785:81)
    at Function.cljs.core.apply.cljs$core$IFn$_invoke$arity$2 (/home/niwi/tmp/greeter/out/cljs/core.js:13229:18)
    at cljs$core$apply (/home/niwi/tmp/greeter/out/cljs/core.js:13187:24)
    at Object.<anonymous> (/home/niwi/tmp/greeter/out/AEF573C.js:10:17)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)

In case of setting the `:foreign-lib` in the `deps.cljs` file, the compilation terminates without error but the generated javascript raises the same error as with `cljs==1.9.845`.






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

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

Type: Defect Priority: Major
Reporter: Miroslav Kubicek Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: printing
Environment:

Win 7 64bit


Approval: Vetted

 Description   

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

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

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

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

THIS WORKS:

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

BUT THIS DOESN'T:

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

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

Thanks for help!



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

Patch must supply a test case.

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

Not sure if this is a bug. Running this at a Clojure REPL produces the same results:

user=> (defrecord MyRecord [value])
user.MyRecord
user=> (require '[clojure.pprint :as pprint])
nil
user=> (pprint/pprint (MyRecord. "a"))
{:value "a"}
nil
Comment by Miroslav Kubicek [ 09/Jul/17 12:23 AM ]

Good catch, that is indeed interesting... but I'd still argue that this is definitely a bug (which just seems to be present in clj as well) - pretty print should work in the same way as print does, only prettier (aka more readable). It should not have its own idiosyncrasies. Every other print function (even including spit) works this way, so I can not imagine what would be the reason (or a use case) for pprint not to honor tagged literals specification of clojure and print things its own way.

Comment by Steve Miner [ 05/Nov/17 8:14 AM ]

There's a new patch for CLJ-1890. If it's accepted for Clojure, it can be ported to CLJS.





[CLJS-2149] aget produces different results from Clojure for non-integer and out-of-bounds indexes Created: 02/Jul/17  Updated: 18/Nov/17

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

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


 Description   

Like CLJS-2113, but for aget:

Clojure:

user=> (aget (to-array [nil 1]) -1)
ArrayIndexOutOfBoundsException -1  clojure.lang.RT.aget (RT.java:2336)
user=> (aget (to-array [nil 1]) 0)
nil
user=> (aget (to-array [nil 1]) 0.5)
nil
user=> (aget (to-array [nil 1]) 1)
1
user=> (aget (to-array [nil 1]) 1.5)
1
user=> (aget (to-array [nil 1]) 2)
ArrayIndexOutOfBoundsException 2  clojure.lang.RT.aget (RT.java:2336)

ClojureScript

cljs.user=> (aget (to-array [nil 1]) -1)
nil
cljs.user=> (aget (to-array [nil 1]) 0)
nil
cljs.user=> (aget (to-array [nil 1]) 0.5)
nil
cljs.user=> (aget (to-array [nil 1]) 1)
1
cljs.user=> (aget (to-array [nil 1]) 1.5)
nil
cljs.user=> (aget (to-array [nil 1]) 2)
nil

Also note that Clojure acts as if rounding indices down to the nearest integer while ClojureScript does not:

(aget (to-array [1 2]) 0.5)

yields 1 in Clojure and nil in ClojureScript.

(Presumably, similar results hold for aset.)



 Comments   
Comment by Mike Fikes [ 18/Nov/17 10:02 AM ]

The fact that Clojure's aget happens to work on non-integer indices may not be intentional. An int cast may be present only to ease interop with the default use of long integral values in Clojure, and while this cast causes the observed behavior (rounding down of passed double s), this may not reflect the intended API.

Here is a commit that speaks to "hints": https://github.com/clojure/clojure/commit/742619e583400400e69cd46ab9e9536c10afb738





[CLJS-1846] Range issues Created: 10/Nov/16  Updated: 19/Nov/17

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

Type: Defect Priority: Major
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: core

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

 Description   

Problem
There are a number of bugs with Range which occur when the step size is 0 or where negative.

Examples

cljs.user=> (count (range 10 0 0))
-Infinity  ;Expect Infinity

cljs.user=> (nth (range 10 0 -1) -1)
11 ; Expected IndexOutOfBounds

cljs.user=> (take 5 (sequence identity (range 0 10 0)))
() ; Expected (0 0 0 0 0)

cljs.user=> (into [] (take 5) (range 0 10 0))
[] ; Expected [0 0 0 0 0]


 Comments   
Comment by David Nolen [ 10/Nov/16 4:37 PM ]

This patch is headed in the right direction but it needs to be more vigilant about performance. I'm more than happy to talk it over via IRC or Slack. Thanks!

Comment by Thomas Mulvaney [ 15/Nov/16 8:24 AM ]

Updated patch with performance tweaks.

  • Added the ^boolean annotation to the `some-range?` helper.
  • Removed calls to methods of Range where possible.
  • Improved 2-arity reduces performance over master significantly by replacing the call to ci-reduce.
Comment by Mike Fikes [ 19/Nov/17 7:39 PM ]

Patch no longer applies; needs re-baseline.





[CLJS-1297] defrecord does not emit IKVReduce protocol Created: 03/Jun/15  Updated: 19/Nov/17

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

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

Attachments: Text File CLJS-1297-19-July-2015.patch    
Patch: Code and Test

 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.

Comment by Mike Fikes [ 19/Nov/17 8:03 PM ]

CLJS-1297-19-July-2015.patch no longer applies.





[CLJS-2408] js->clj does not work on objects with null prototype Created: 21/Nov/17  Updated: 21/Nov/17

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

Type: Defect Priority: Major
Reporter: Kurt Harriger Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Version: 1.9.946
Follow up from issue CLJS-1998

cljs.user=> (js->clj #js {})
{}

cljs.user=> (js->clj (.create js/Object nil))
#object[Object]

cljs.user=> (keys (js->clj (.create js/Object nil)))
org.mozilla.javascript.EcmaError: TypeError: Cannot find default value for object. (.cljs_rhino_repl/goog/../.cljs_rhino_repl/cljs/core.js#9915)
	 (.cljs_rhino_repl/cljs/core.cljs:2930:10)
	 cljs$core$seq (.cljs_rhino_repl/cljs/core.cljs:1212:13)
	 cljs$core$keys (.cljs_rhino_repl/cljs/core.cljs:8648:3)
	 (NO_SOURCE_FILE <cljs repl>:1:0)
	 (NO_SOURCE_FILE <cljs repl>:1:0)


 Comments   
Comment by Kurt Harriger [ 21/Nov/17 8:44 PM ]

For more context the source of the object with null prototype is generated in a third party library I am using here https://github.com/ProseMirror/prosemirror-model/blob/cdded3d/src/schema.js#L13





[CLJS-2421] Referred type can be replaced by same-named type in referring namespace Created: 27/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Two namespaces:

(ns my.ns)

(defrecord A [foo bar])

(defn make-a [foo bar]
  (A. foo bar))
(ns other.ns
  (:require [my.ns :refer (A)]))

(println (my.ns/A. 1 2) (my.ns/make-a 1 2))

(defrecord A [baz quux])

(println (my.ns/A. 1 2) (my.ns/make-a 1 2))

Loading other.ns yields output:

#my.ns.A{:foo 1, :bar 2} #my.ns.A{:foo 1, :bar 2}
#my.ns.A{:baz 1, :quux 2} #my.ns.A{:baz 1, :quux 2}

The definition of other.ns.A is replacing that of my.ns.A.



 Comments   
Comment by Chas Emerick [ 27/Nov/17 12:39 PM ]

This may be a duplicate of CLJS-1558...?

Comment by Mike Fikes [ 27/Nov/17 12:40 PM ]

This ticket shares similarities with CLJS-1558 in that the compiler is essentially allowing you to re-def a referred Var. Perhaps for both tickets this should cause a compiler error as occurs with Clojure.

Comment by Mike Fikes [ 27/Nov/17 2:42 PM ]

I struggle a little with this one in terms of defining what should be going on, and at the core of my lack of clarity is on whether my.ns/A is a Var or simply a type.

In the compiler state, it does show up under :defs, but it is flagged with :type true. (In other words, perhaps just because it is under :def doesn't imply it is a Var.) In Clojure, even though an underlying constructor exists that can be accessed via interop, no actual Var is created. (Evidence backing this assertion is that (var A) fails in Clojure, and no Var appears if you use dir at the REPL—you only see the associated synthetically-generated positional and map factory Vars.

Another argument that "it is not a Var" is what defrecord returns. defrecord doesn't return a Var (even in Clojure), but instead returns a symbol (which evaluates to itself, much like the symbol java.lang.String).

If you go with the argument that the underlying constructor is not a Var, then perhaps some things could be cleaned up (fixing dir in ClojureScript, and perhaps causing var to balk if applied to types).

Also, if you go with the argument that it is not a Var, then perhaps you'd argue that import is appropriate, while require is not. Inspired by http://puredanger.github.io/tech.puredanger.com/2010/06/30/using-records-from-a-different-namespace-in-clojure/, this variation seems to work fine:

(ns my.ns)

(defrecord A [foo bar])
(ns other.ns
  (:require [my.ns])
  (:import (my.ns.A)))

(println (my.ns.A. 1 2) (my.ns/->A 1 2))

(defrecord A [baz quux])

(println (my.ns.A. 1 2) (my.ns/->A 1 2))

(println (A. 3 4) (->A 3 4))

with this result:

cljs.user=> (require 'other.ns)
#my.ns.A{:foo 1, :bar 2} #my.ns.A{:foo 1, :bar 2}
#my.ns.A{:foo 1, :bar 2} #my.ns.A{:foo 1, :bar 2}
#other.ns.A{:baz 3, :quux 4} #other.ns.A{:baz 3, :quux 4}
nil

One thing I don't like is that you have to write (:import (my.ns.A)) as opposed to {{(:import (my.ns A))}, but perhaps that is a minor quirk that could be cleaned up if the "not a Var" approach were taken.





[CLJS-1141] memoization of js-dependency-index and get-upstream-deps needs knobs Created: 18/Mar/15  Updated: 19/Nov/17

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

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

Attachments: Text File CLJS_1141.patch     Text File CLJS-1141-with-js-dep-caching-latest.patch    
Patch: Code

 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.

Comment by Mike Fikes [ 19/Nov/17 8:04 PM ]

Patch no longer applies.





[CLJS-871] .-default property access returns nil Created: 11/Oct/14  Updated: 01/Aug/17

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Unresolved Votes: 3
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.

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

Now that we have CLJS-1620, a warning seems like a good answer.





[CLJS-2432] docstring for cli->js should describe :keyword-fn Created: 04/Dec/17  Updated: 04/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, newbie


 Description   

CLJS-2215 added support in cljs.core/clj->js for a new :keyword-fn parameter. This feature should be described in the docstring.






[CLJS-1328] Support defrecord reader tags Created: 04/Jul/15  Updated: 04/Dec/17

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

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

Comment by Levi R. I. Tan Ong [ 04/Dec/17 11:23 PM ]

Trying my hand at this, but I'm stuck at an architecture conundrum.

Should the analyzer be able to add a tag reader to the tag table, or should the reader take a look at a the record-table to build tag readers on the fly?

The former seems easier to do with a macro, but it feels wrong to write reader code in the analyzer namespace.
The latter is more tedious because all the reader functions deref the tag reader table, so I'd have to go through all of them, but it feels less like a war crime than the former.

Or is there another option I'm not seeing?





[CLJS-2435] Don't warn if arithmetic operations done on ^int Created: 05/Dec/17  Updated: 05/Dec/17

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

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


 Description   

If CLJC is being used and the ClojureScript compiler sees Clojure type annotations, it should effecively ignore those where there isn't a portability concern.

The specific case for this ticket is ^int and arithmetic, and a motivating example is:

cljs.user=> (let [^int v (alength (into-array []))] (+ v 3))
WARNING: cljs.core/+, all arguments must be numbers, got [int number] instead. at line 1 <cljs repl>
3

On the other hand, ^long should still cause warnings to be emitted given that it is not a "subtype" of ^number (such code might overflow, so warnings are helpful in that case).






[CLJS-2055] Circular dependencies with parallel build cause the compiler get stuck Created: 27/May/17  Updated: 13/Dec/17

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

Type: Defect Priority: Major
Reporter: Miikka Koskinen Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None
Environment:

ClojureScript 1.9.562


Attachments: Zip Archive circular-dependency-deadlock.zip    

 Description   

If you have circular dependencies between namespaces, :parallel-build is enabled and :optimizations is something other than :none, the compiler won't show an error message about the circular dependencies and the compilation never finishes.

Reproduction: https://github.com/miikka/cljs-circular-deps-repro

Looking at the code, I think that this happens because the parallel builder threads wait for their dependencies to be built before they proceed. With circular dependencies, this never happens. I suspect that commit 9ad6d5d61c introduced this problem, as it disabled the circularity-checking code that was run before the parallel compilation.



 Comments   
Comment by Dieter Komendera [ 13/Dec/17 4:41 AM ]

We are running into same issue, but also with `:optimizations :none` on 1.9.946 which is nasty because it fails without any output in development. I'm attaching a minimal example build reproducing the problem.

Comment by Dieter Komendera [ 13/Dec/17 4:43 AM ]

example reproducing the issue

Comment by Dieter Komendera [ 13/Dec/17 5:02 AM ]

Was also reported in the original issue tracking parallel compilation with circular dependencies https://dev.clojure.org/jira/browse/CLJS-1539?focusedCommentId=45275&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-45275





[CLJS-2451] Unable to require scoped dependency Created: 23/Dec/17  Updated: 23/Dec/17

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

Type: Defect Priority: Major
Reporter: Kurt Harriger Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I've been trying to get @atlaskit/editor-core to work in clojurescript, awhile back I tried ran into issue with npm-deps required keywords that seems to be fixed, but now having what seems to be the same issue with require.

Assert failed: cljs.analyzer/foreign-dep? expected symbol got "@atlaskit/editor-core"
(symbol? dep)

I pushed a project to github that reproduces the issue:
https://github.com/kurtharriger/cljs-npm-dep-issue






[CLJS-650] Optimize all protocols Created: 01/Nov/13  Updated: 01/Nov/13

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   

We should be optimizing all protocols insteads of just putting the core protocols on the fast path. In the current design we put the protocol mask on instance - this wastes a considerable amount of space, instead we should be putting it on the prototype. This benchmark appears to show no performance hit for this approach jsperf.com/prototype-bit-mask.

In order to have fewer tests satisfies? and protocol fns should generate different code for the different compilation modes - in anything but advanced we should just use the boolean property on the prototype, in advanced we should use the bit mask approach.






[CLJS-720] #queue literal behavior is incorrect Created: 07/Dec/13  Updated: 07/Dec/13

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

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


 Description   

In order for queue to work we need to adopt an approach similar to the one for #js data literals - i.e. needs special casing in the analyzer since queues are not "atomic" values.






[CLJS-736] Functions folder and reducer broken for types nil and array + fix for typo Created: 29/Dec/13  Updated: 02/Dec/14

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

Type: Defect Priority: Minor
Reporter: Jonas De Vuyst Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-736-alt.patch     Text File CLJS-736.patch     Text File CLJS-736-patch-1-redux.patch     Text File CLJS-alt-satisfies.patch    
Patch: Code and Test

 Description   

1. This currently doesn't work:

(->> nil
(r/map identity)
(r/reduce + 0))
; org.mozilla.javascript.JavaScriptException: Error: No protocol method IReduce.-reduce defined for type null

The reason for this is that reducers created by r/reducer or r/folder, invoke -reduce (of IReduce) directly. They thereby bypass the special case for nil in the function r/reduce.

2. An entirely analogous problem exists for collections of type array.

3. The patch CLJS-700 mistakenly defined coll-fold for the type cljs.core/IPersistentVector. This should have been cljs.core/PersistentVector. (There exists no protocol IPersistentVector in ClojureScript.)

I will shortly attach a patch that addresses all of the above problems by implementing IReduce for nil and array. The patch also includes unit tests.



 Comments   
Comment by Jonas De Vuyst [ 29/Dec/13 2:22 PM ]

Alternative patch in which r/reduce and r/fold treat arrays and nil as special cases – as opposed to having arrays and nil implement IReduce and CollFold.

The functions r/reducer, r/folder, and the protocol methods of r/Cat now call r/reduce and r/fold instead of calling -reduce and coll-fold directly.

This patch also fixes a bug in the coll-fold implementation for Cat, which previously used (reducef) as the initial value rather than (combinef). The new code is copied and pasted from the Clojure implementation and uses the fork-join stubs.

Comment by David Nolen [ 30/Dec/13 8:23 AM ]

The implements? should probably be a satisfies? in the second patch. Have you run any benchmarks of before/after the patch?

Comment by Jonas De Vuyst [ 30/Dec/13 11:24 AM ]

If I understand correctly then (satisfies? x y) is roughly equivalent to (or (implements? x y) (natively-satisfies? x y)).

If native types (nil, array, object currently) are treated as special cases then implements? seems more appropriate.

satisfies? works also, however, so I have attached a new 'alt' patch.

Comment by Jonas De Vuyst [ 30/Dec/13 11:26 AM ]

The first patch is in fact faster when running the following code:

(time (->> (repeat 1000 (vec (range 1000)))
vec
(r/mapcat identity)
(r/map inc)
(r/filter even?)
(r/fold +)))

This takes about 700 msecs. Using the first patch this terminates 100-300 msecs faster. This is after repeated (but informal) testing.

I guess the worry is that the first patch would slow down other random code since it involves extending the types nil, array, and object. I'm not sure what exactly I should test for though.

(Note that the 2nd and 3rd patch also contain a fix for Cat and include more unit tests. The first patch should preferably not be applied as-is.)

Comment by David Nolen [ 30/Dec/13 11:35 AM ]

Yeah you're timing too many things, including vec, range, lazy sequences. Also testing a small N. Take a look at the reducers example on the Mori README - https://github.com/swannodette/mori. Thanks.

Comment by Jonas De Vuyst [ 30/Dec/13 12:52 PM ]

I tried running the following code:

(let [coll (vec (repeat 1000 (vec (range 10))))]
  (time (doseq [n (range 1000)]
               (->> coll
                    (r/mapcat identity)
                    (r/map inc)
                    (r/filter even?)
                    (r/fold +)))))

Some of the last results I got were:

1st patch: 75680 msecs
2nd patch: 76585 msecs

Truth be told, although the first patch seemed to win most of the times, sometimes the second patch was faster.

One other thing I tried was removing the implements?/satisfies? check from the second patch and overriding the protocol method coll-fold for the type object instead (as in the first patch). This 'hybrid' approach generally (but not always) seemed to result in a slowdown.

I'm not sure how I should proceed. Should I perhaps just run both patches simultaneously for several minutes?

Comment by David Nolen [ 30/Dec/13 1:21 PM ]

This is still a bad way to do timing, you're recording the cost of range and seq'ing. Use dotimes.

Comment by Jonas De Vuyst [ 30/Dec/13 4:33 PM ]

Hm. I guess the lazy sequence does lead to a lot of allocations.

Alright, I rewrote my test and ran it a few more times. I now also tested on both vectors and arrays.

Patch 1 needed a slight tweak. When coll-fold is invoked, patch 1 only specifies a fallback for type object (i.e. r/reduce is called). I had to add the same fallback for type array. (This is weird!)

So here are the results.

For vectors:

(let [coll (vec (repeat 100 (vec (range 100))))]
  (time (dotimes [n 3000]
          (->> coll
              (r/mapcat identity)
              (r/map inc)
              (r/filter even?)
              (r/fold +)))))

Patch 1: 205872 msecs
Patch 2: 210756 msecs

For arrays:

(let [coll (into-array (repeat 100 (into-array (range 100))))]
  (time (dotimes [n 3000]
          (->> coll
              (r/mapcat identity)
              (r/map inc)
              (r/filter even?)
              (r/fold +)))))

Patch 1: 123567 msecs
Patch 2: 119704 msecs

I ran my tests a few times and the results were pretty consistent. Patch 1 is faster for vectors and patch 2 is faster for arrays.

This makes sense.

In patch 1 reducer will call -reduce directly. In patch 2, reducer first calls r/reduce, which calls -reduce if the collection is a vector and array-reduce if it's an array. Hence patch 2 contains an extra function call in the case of vectors, but avoids invoking a protocol method on a native type in the case of arrays.

Using macros (or copy and paste) the extra function call can be avoided. Would that be worth trying or is it more important to keep the code clean?

I just realized that patch 2 is semantically slightly different from what Clojure does, although perhaps this is a bug in Clojure: <https://groups.google.com/forum/#!searchin/clojure-dev/kv-reduce/clojure-dev/bEqECvbExGo/iW4B2vEUh8sJ>. My suggestion to use a macro (or copy and paste) to avoid the extra function call in patch 2, could also fix this discrepancy.

Comment by David Nolen [ 30/Dec/13 4:42 PM ]

How are you benchmarking this? With V8? JavaScriptCore? SpiderMonkey? In the browser? What optimization settings, etc.

Comment by Jonas De Vuyst [ 30/Dec/13 4:48 PM ]

I used repljs (Rhino?). I'll test again in a more realistic setting tomorrow.

Comment by David Nolen [ 30/Dec/13 4:54 PM ]

Yeah, benchmarking with Rhino isn't informative.

Comment by Jonas De Vuyst [ 31/Dec/13 1:40 AM ]

I compiled the same code (with n=3000) using cljs with "{:optimizations :advanced}".

I then tested it in the latest stable releases of Firefox, Chrome, and Safari. I closed all my browsers. For each browser I then followed the following procedure:

  • Open the browser
  • Open the developer console
  • Run the benchmark for patch 1
  • Run the benchmark for patch 2
  • Run the benchmark for patch 1 and write down the result
  • Run the benchmark for patch 2 and write down the result
  • Close the browser

Firefox:

  • Patch 1. Vectors: 26057 msecs
  • Patch 1. Arrays: 25026 msecs
  • Patch 2. Vectors: 26258 msecs
  • Patch 2. Arrays: 36653 msecs
  • Summary: Patch 1 is faster for vectors and arrays

Chrome:

  • Patch 1. Vectors: 7804 msecs
  • Patch 1. Arrays: 7092 msecs
  • Patch 2. Vectors: 7754 msecs
  • Patch 2. Arrays: 6768 msecs
  • Summary: Patch 2 is faster for vectors and arrays

Safari:

  • Patch 1. Vectors: 167230 msecs
  • Patch 1. Arrays: 108780 msecs
  • Patch 2. Vectors: 173940 msecs
  • Patch 2. Arrays: 110012 msecs
  • Summary: Patch 1 is faster for vectors and arrays

I'm not sure what to make of this.

Comment by Jonas De Vuyst [ 31/Dec/13 2:47 AM ]

I have attached a new version of the first patch.

This patch fixes an issue with r/Cat. (This issue was also addressed in the second and third patch. A unit test is included.).

This patch also fixes r/fold for arrays.

To summarize, a choice needs to be made between the following patches.

  • CLJS-736-patch-1-redux.patch
  • CLJS-736-alt.patch (uses implements?) / CLJS-alt-satisfies.patch (uses satisfies?)

The implementation details are patch-1-redux is more similar in spirit to the Clojure source code. The alt patches are more similar in spirit to the ClojureScript source code.

As explained above, the alt patches are semantically a bit different from the original Clojure source—but it's not clear which behavior is 'right'.

Comment by David Nolen [ 16/Jan/14 5:27 PM ]

The benchmarks would be more informative if they explained the performance before and after that patch.

Comment by Jonas De Vuyst [ 18/Jan/14 11:55 AM ]

r/reduce previously didn't work for nil or JavaScript arrays.

One reason why I have trouble recommending a patch is that I don't know what use case you would like to optimize for.

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

Yes but now that we have new logic we can at least test the lack of regression on the other types.

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

Ok I tried to apply this patch and run ./script/benchmarks in the repo but the patch will no longer apply. Can we rebase the patch on master. Thanks. If you also want to give the benchmarks a shot follow these instructions to install the JS engines - http://github.com/clojure/clojurescript/wiki/Running-the-tests. Then you can also run the benchmarks at the command line. I see there aren't any reducers benchmarks, I will add some.





[CLJS-900] Parameterize caching strategy Created: 03/Dec/14  Updated: 03/Dec/14

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

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


 Description   

Currently the caching strategy is hard coded to a disk based one. It would be desirable in many situations for the caching to be in memory. We should decouple the caching strategy and support disk / memory out of the box.






[CLJS-1123] this-as unexpectedly binds js/window when used within function with post-condition Created: 15/Mar/15  Updated: 16/Mar/15

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

Type: Defect Priority: Minor
Reporter: J. S. Choi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Adding a post-condition to any function that uses cljs.core/this-as will unexpectedly cause this-as's "this" symbol to be bound to the root object (e.g., js/window) instead.

(defn f-no-post-condition [argument]
  (this-as this
    (js/console.log argument this)))

(defn f-with-post-condition [argument]
  {:post [true]}
  (this-as this
    (js/console.log argument this)))

(def test-object
  #js {:methodNoPostcondition f-no-post-condition
       :methodWithPostcondition f-with-post-condition})

(f-with-post-condition "A") ; Correctly prints js/window
(.methodNoPostcondition test-object "B") ; Correctly prints test-object
(.methodWithPostcondition test-object "C") ; Incorrectly prints js/window


 Comments   
Comment by David Nolen