<< Back to previous view

[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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-1691] spec internal compiler APIs Created: 21/Jun/16  Updated: 21/Jun/16

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

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





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

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

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





[CLJS-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-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-2472] ES6 Iterators should use IIterable if possible Created: 15/Jan/18  Updated: 15/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

Motivation:

ES6 Iterables: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

Use in React: https://github.com/facebook/react/blob/30dac4e78de02fb427ee82013160ae875128d7a2/packages/react/src/ReactElementValidator.js#L195-L204

(defn es6-iterator**
  [coll]
  (if (implements? IIterable coll)
    (let [it (-iterator ^not-native coll)]
      #js{:next (fn []
                  (if ^boolean (.hasNext it)
                    #js{:value (.next it), :done false}
                    #js{:value nil, :done true}))})
    ;; Fallback to naive first/next iterator:
    (ES6Iterator. (seq coll))))

;; Tests can use round trip:
(es6-iterator-seq (es6-iterator (hash-map 0 1 2 3)))

(defn es6-iter-consume
  [it]
  (while (not (.-done (.next it)))))

(dotimes [_ 3]
  (let [xs (vec (range 3000))
        runs 1000]
    (simple-benchmark []
      (es6-iter-consume (es6-iterator xs)) runs)
    (simple-benchmark []
      (es6-iter-consume (es6-iterator** xs)) runs)))

About a 4x speedup in Chrome. Also note that much less garbage is produced.






[CLJS-2471] ChunkedSeq should implemented ICounted Created: 15/Jan/18  Updated: 15/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

ChunkedSeq in clojure implements Counted, should do the same in CLJS. Implementation is straight forward.






[CLJS-2470] ArrayChunk.reduce doesn't honor end param Created: 15/Jan/18  Updated: 15/Jan/18

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

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


 Description   

Ex:

(reduce max (array-chunk #js[0 1 2 3] 0 1))
;; => 3

Currently not an issue since end is always arr.length for our usage of ArrayChunk, but since it's a public API users might pass a different end param.

This can easily be fixed after CLJS-2466 which properly implements the reduce for ArrayChunk.






[CLJS-2469] ChunkedCons chunk-next returns empty seq instead of nil Created: 13/Jan/18  Updated: 19/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: A. R
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2469.patch    

 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.





[CLJS-2468] Implement reduce for ChunkedCons Created: 12/Jan/18  Updated: 12/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Forked from CLJS-2466

Title says it all.






[CLJS-2467] Small PVs are never chunked Created: 12/Jan/18  Updated: 13/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2467.patch    

 Description   

A PersistenVector that has <=32 elements and is seq'ed will return an IndexedSeq. Though IndexedSeq always fails the chunked-seq? check.

This means a:

(def xv (vec (range 32)))
(reduce + (map inc xv))

is about 4x slower than a map over a vector with 33 elements.

Options:
1. Return a ChunkedCons with the "rest" set to nil in PersistentVector.seq()
2. Implement IChunkedSeq for IndexedSeq:

(extend-type IndexedSeq
    IChunkedSeq
    (-chunked-first [x] x)
    (-chunked-rest [x] ())
    IChunkedNext
    (-chunked-next [x] nil)
    IChunk
    (-drop-first [coll]
      (if-some [n (-next coll)]
        n
        (throw (js/Error. "-drop-first of empty chunk")))))

I think option #2 is better since IndexedSeq is used quite a bunch throughout the code base, so the chunking will also kick in for many other code paths.



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

Note:

This is different from Clojure (which does not consider an IndexedSeq a ChunkedSeq) since we use IndexedSeq a lot more where Clojure often uses a ChunkedSeq. For instance:

(apply (fn [& a] (type a)) [1 2 3 4 8])

will return IndexedSeq in CLJS but ChunkedCons in CLJ.

Since these IndexedSeq's will be passed around wildly in a normal CLJS app it makes sense to extend them as a IChunkedSeq since otherwise all these will never be chunked and get a slow first/next treatment.





[CLJS-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-2465] Using var-args as arguments name in variadic function overrides first argument Created: 11/Jan/18  Updated: 11/Jan/18

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

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


 Description   

The following returns (nil "b"):

((fn [& var-args]
   var-args) "a" "b")

var-args should to be shadowed by default.






[CLJS-2463] js calls are mistakenly treated as higher order invokes Created: 10/Jan/18  Updated: 10/Jan/18

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

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


 Description   

Code that invokes js/ a lot (like Sablono) will currently treat the JS invokes as higher order invokes and bind all arguments in a let beforehand. This generates a ton of IIFEs which are all unnecessary.

Fix:

HO-invoke? (and (boolean *cljs-static-fns*)
                        (not fn-var?)
                        (not (js-tag? f))
                        (not kw?)
                        (not (analyzed? f)))


 Comments   
Comment by A. R [ 10/Jan/18 2:11 PM ]

Actually this has a much larger scope. Currently also Math,goog (and more?) are treated like HO invokes. This also affects compile time when using these a lot since it generates a lot of new {{let}}s. Will probably also be affected by JS modules? So we should do a more careful check here to avoid this.

Comment by A. R [ 10/Jan/18 2:45 PM ]

Another one: all-values predicate should also include keyword? check, no reason to bind the argument when passing keywords.

To illustrate what the current code gen looks like:

(defn foooooooo
  [x y]
  (x :fooo :bar)
  (goog/mixin (js/fooo x) (js/bar y))
  (Math/floor (js/fooo x) (js/fooo x))
  (goog.mixin (js/fooo x) (js/fooo x)))

Generated:

foooooooo = (function (x,y){

var G__21625_21633 = cljs.core.cst$kw$fooo;
var G__21626_21634 = cljs.core.cst$kw$bar;
(x.cljs$core$IFn$_invoke$arity$2 ? x.cljs$core$IFn$_invoke$arity$2(G__21625_21633,G__21626_21634) : x(G__21625_21633,G__21626_21634));

var G__21627_21635 = fooo(x);
var G__21628_21636 = bar(y);
goog.mixin(G__21627_21635,G__21628_21636);

var G__21629_21637 = fooo(x);
var G__21630_21638 = fooo(x);
Math.floor(G__21629_21637,G__21630_21638);

var G__21631 = fooo(x);
var G__21632 = fooo(x);
return goog.mixin(G__21631,G__21632);

});

All of them don't need to bind the passed arguments.

Obviously in this simple case GCC will just inline everything and the cost is 0. But if any of these calls happen inside a function, like {{let [x (the-call...] ...}} then an IIFE is created that isn't broken up by GCC.





[CLJS-2461] Minor shadowing intricacies Created: 09/Jan/18  Updated: 09/Jan/18

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

Type: Defect Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler


 Description   

Examples that currently fail:

(ns a-b.core)
(defn doit [])

(ns a-b.other
  (:require [a-b.core :as core]))

(let [a_b 2]
  (core/doit)) ;; FAILS, a_b isn't shadowed properly

;; Related:
(let [a_b 1, a-b 2] a_b) ;; Collide, returns 2!
(let [a-b 1, a_b 2] a-b) ;; Collide, returns 2!
(let [goog "2"] (js-obj goog 2) ;; Fails. Overrides goog!

Given the shadowing logic of the compiler is pretty expensive and further dynamically computing the munged name of each "first namespace segment" could potentially slow down the compiler by a factor of 2x (or more) we should probably re-introduce this approach:

https://github.com/clojure/clojurescript/commit/1c71ab3bff27dc4a099b06e122ec3fdf5a2a8ba8

Ie, maintaining a set of first ns segments and do s simple set lookup in shadow-depth. We should probably only store munged names and do the lookup after munging to fix the above bugs. We should also add "goog".






[CLJS-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-2454] Inconsistent macro require behaviour Created: 02/Jan/18  Updated: 03/Jan/18

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

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


 Description   

It is my understanding that a namespace has to :require-macros itself to have its macros accessible automatically but this no longer seems to be accurate.

Given this minimal example

;; src/demo/lib.cljs
(ns demo.lib)

;; src/demo/lib.clj
(ns demo.lib)

(defmacro foo [& args]
  `(prn ::macro))

;; src/demo/test.cljs
(ns demo.test
  (:require [demo.lib :as lib]))

(lib/foo 1 2 3)

I would expect a warning for lib/foo since demo.lib has no :require-macros for its macros and the demo.test did not use :include-macros or :refer-macros.

Compiling this via the build API does in fact produce the expected warning but only iff the demo.lib CLJ namespace was not required anywhere else.

WARNING: Use of undeclared Var demo.lib/foo at line 5 src/demo/test.cljs
;; build.clj

(require '[cljs.build.api :as cljs])
;; (require 'demo.lib) ;; requiring this here removes the warning

(cljs/build
  "src"
  {:output-dir "out"
   :optimizations :none
   :verbose true})

Enabling the (require 'demo.lib) in the build.clj causes the warning to disappear and the code uses the macro properly.

Some popular libraries (eg. devcards) do not self-require but still work if the macro file was at least required somewhere else.

Is this intended behaviour or just accidental?



 Comments   
Comment by David Nolen [ 03/Jan/18 8:44 AM ]

It's accidental in a sense - the macro resolution is simple to the point of being a bit naive. I'm not sure if we should do anything about it at this point. It would probably cause quite a bit of breakage and little benefit.





[CLJS-2446] with-meta doesn't work for variable arguments functions Created: 19/Dec/17  Updated: 19/Dec/17

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

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


 Description   

Currently with-meta doesn't work for varargs fns (which works in Clojure):

(apply (with-meta #(-> %&) {}) 0 1 2 (range 30))

Given JS is so flexible I'd propose the following implementation of meta fn:

(defn meta-fn
  [f m]
  (let [new-f (goog/bind f #js{})]
    (goog/mixin new-f f)
    (specify! new-f IMeta (-meta [_] m))
    new-f))

The goog/bind creates a copy, and the goog/mixin is just for performance reasons (copy any IFn protocol or applyTo).

Benefits:

  • Slightly faster
  • Much simpler and smaller code (the 20 arities of MetaFn are emitted 3 times (.call, .apply, prototype.IFn)
  • Works with varargs.





[CLJS-2444] Refering replaced cljs.core macro symbols Created: 17/Dec/17  Updated: 22/Dec/17

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

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

Lumo 1.8.0-beta, Clojurescript 1.9.946



 Description   

Overwriting symbols that are defined in cljs.core works fine and prints an expected warning. One case where this fails is when a symbol that was replaces was originally a macro and it's invoked without namespace qualification. An example:
code
;;;;;;; file a ;;;;;;
(ns file.a)
(defn + [& vals]
(prn "Called!")
(apply cljs.core/+ vals))
;;;;;;; file b ;;;;;;
(ns file.b
(:require [file.a :refer [+]]))
(+ 1 2 ) ;; Doesn't print "Called!"
(file.a/+ 1 2) ;; Prints "Called!"
(var ) => file.a/
(`+ 1 2) ;; Prints "Called!"
code

The same symbol `` alone by itself in file b calls cljs.core/ despite it being referred and all var resolution resolves to file.a/+. This seems not to be the case when overwriting functions from cljs.core, for example overwriting the function `map` works fine.
code
;;;;;;; file a ;;;;;;
(ns file.a)
(defn map [f coll]
(prn "map called!")
(cljs.core/map f coll))
;;;;;;; file b ;;;;;;
(ns file.b
(:require [file.a :refer [map]]))

(map inc [1 2 3]) => "map called!" (2 3 4)
code






[CLJS-2442] `set` and `vec` performance enhancements Created: 14/Dec/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: Alex Gunnarson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

On the master branch (and in all previous versions of ClojureScript I've seen), the version of `set` and `vec` in core.cljs do not do an instance check before creating a new set or vector. That is, `set` does not check (as in the Clojure 1.8 version) whether the input is already a set before creating a brand-new one, and `vec` does not check (as in the Clojure 1.8 version) whether the input is already a vector before creating a brand-new one. In addition, `set` appears not to support reducibles (`vec` does not share this defect).

The enhancement is short and simple: the addition of one `instance?` check per function.

Relates to, but does not duplicate, CLJS-1784.






[CLJS-2440] Re-running watch on CLJS source using native modules results in JS error Created: 12/Dec/17  Updated: 22/Dec/17

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

Type: Defect Priority: Minor
Reporter: Dave Kozma Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None
Environment:

Mac OS X 10.13.1, ClojureScript 1.9.968-gebdaf6c0 running on Java SE 1.8.0_144, built with Maven 3.5.2



 Description   

I'm trying to use a native NPM module in my CLJS code (in this case, date-fns), and the initial time I try to watch, it works, however on successive watches, it fails with the following error in my console:

base.js:1357 Uncaught Error: Undefined nameToPath for date_fns
    at visitNode (base.js:1357)
    at Object.goog.writeScripts_ (base.js:1369)
    at Object.goog.require (base.js:706)
    at index.html:8

To reproduce, I've created a build script as such:

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

(b/watch "src"
  {:main 'npm.core
   :output-to "out/npm.js"
   :output-dir "out"
   :npm-deps {:date-fns "1.29.0"}})

as well as a minimal test file:

(ns npm.core
  (:require [date-fns :as dfn]))

(def now (js/Date.))
(.write js/document "Today is " (dfn/format now "dddd"))

I'm also testing with a local `package.json` file rather than using `install_deps true` - my `package.json` (generated by CLJS) looks like this:

{
  "dependencies": {
    "@cljs-oss/module-deps": "^1.1.1",
    "date-fns": "^1.29.0"
  }
}

When I run:

npm install
java -cp ../clojurescript/target/cljs.jar:src clojure.main watch.clj

everything works fine, however if I CTRL+C and run the same exact command again, I get the error outlined above.

However, if (and only if) I delete the `out` directory and run the command a third time, it works again.

Please let me know if you need any other details.



 Comments   
Comment by Dave Kozma [ 12/Dec/17 11:47 AM ]

Sorry, seemed to mess up on the title and can't seem to edit. The title should be "Re-running watch on CLJS source using native modules results in JS error"

Comment by Dave Kozma [ 12/Dec/17 11:49 AM ]

Additional environment info:

npm -v 5.5.1
node -v v8.4.0

Comment by Andy Parsons [ 22/Dec/17 12:13 PM ]

Noticed that this was switched to priority 4, but this issue is preventing us from using npm dependencies entirely. What does the lower priority reflect?





[CLJS-2439] FileNoteFoundException thrown when have URL configed in :foreign-libs/:file Created: 12/Dec/17  Updated: 12/Dec/17

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

Type: Defect Priority: Minor
Reporter: Shark Xu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs, compiler
Environment:

OS: Ubuntu 16.04



 Description   

When I have URL configed in :foreign-libs/:file (via build option or deps.cljs), I get FileNoteFoundException.

URL example: https:/raw.githubusercontent.com/mrdoob/three.js/dev/examples/js/controls/OrbitControls.js

Error Log:

51. Unhandled java.io.FileNotFoundException
6 /home/shark/git/apps/https:/raw.githubusercontent.com/mrdoob/three.js/dev/examples/js/controls/OrbitControls.js
7 (No such file or directory)
8
9 FileInputStream.java: -2 java.io.FileInputStream/open0
10 FileInputStream.java: 195 java.io.FileInputStream/open
11 FileInputStream.java: 138 java.io.FileInputStream/<init>
12 io.clj: 238 clojure.java.io/fn
13 io.clj: 235 clojure.java.io/fn
14 io.clj: 69 clojure.java.io/fn/G
15 io.clj: 165 clojure.java.io/fn
16 io.clj: 69 clojure.java.io/fn/G
18 io.clj: 102 clojure.java.io/reader
19 io.clj: 86 clojure.java.io/reader
20 RestFn.java: 410 clojure.lang.RestFn/invoke
21 closure.clj: 422 cljs.closure/eval7540/fn
22 js_deps.cljc: 121 cljs.js_deps$eval2733$fn_2756$G2724_2765/invoke
23 closure.clj: 418 cljs.closure/eval7540/fn
24 js_deps.cljc: 121 cljs.js_deps$eval2733$fn_2756$G2724_2765/invoke
25 closure.clj: 1723 cljs.closure/write-javascript
26 closure.clj: 1699 cljs.closure/write-javascript
27 closure.clj: 1748 cljs.closure/source-on-disk
28 closure.clj: 1743 cljs.closure/source-on-disk
29 closure.clj: 2604 cljs.closure/build/fn
30 core.clj: 2646 clojure.core/map/fn
31 LazySeq.java: 40 clojure.lang.LazySeq/sval
32 LazySeq.java: 49 clojure.lang.LazySeq/seq
33 Cons.java: 39 clojure.lang.Cons/next
34 RT.java: 688 clojure.lang.RT/next
35 core.clj: 64 clojure.core/next
36 core.clj: 3033 clojure.core/dorun
37 core.clj: 3039 clojure.core/doall
38 closure.clj: 2604 cljs.closure/build
40 closure.clj: 2507 cljs.closure/build
41 api.clj: 205 cljs.build.api/build
42 api.clj: 189 cljs.build.api/build
43 api.clj: 192 cljs.build.api/build
44 api.clj: 189 cljs.build.api/build
45 REPL: 62 apps.cljs-rt-browser/-main



 Comments   
Comment by Shark Xu [ 12/Dec/17 8:30 AM ]

Add my config code:

1{:foreign-libs
2 [{:file "https://raw.githubusercontent.com/mrdoob/three.js/dev/examples/js/controls/OrbitControls.js"
3 :provides ["cljsjs.three-orbitcontrols"]
4 :requires ["cljsjs.three"]
5 }
6 {:file "https://raw.githubusercontent.com/dataarts/dat.gui/master/build/dat.gui.js"
7 :provides ["cljsjs.dat-gui"]
8 }
9 {:file "https://raw.githubusercontent.com/mrdoob/three.js/dev/examples/js/ParametricGeometries.js"
10 :provides ["cljsjs.three-parametricgeometries"]
11 :requires ["cljsjs.three"]
12 }
13
14 ]
15 }





[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-2433] HTTP server in cljs.browser.repl doesn't serve files with extensions other than specified in ext->mime-type Created: 04/Dec/17  Updated: 26/Dec/17

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

Type: Defect Priority: Minor
Reporter: Yegor Timoshenko Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-2433-fallback-to-send-static-use-host-to-determ.patch     Text File 0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch     Text File 0002-CLJ-2433-send-fils-via-send-and-close.patch    

 Description   
$ cat deps.edn
{:deps {org.clojure/clojure {:mvn/version "1.9.0-RC2"}
        org.clojure/clojurescript {:mvn/version "1.9.946"}}
$ touch hello.xml
$ clojure -m cljs.browser.repl &
$ curl localhost:9000/hello.xml
<html><body><h2>Page not found</h2>No page /hello.xml found on this server.</body></html>

Adding an entry to cljs.repl.browser/ext->mime-type superficially fixes that.

1) Currently used MIME type identification is very simplistic and doesn't return correct MIME types for most files. Instead, this should be delegated to the host:

(ns cljs.repl.browser
  (:import [java.nio.file Files Paths]))

(defn get-content-type [path]
  (Files/probeContentType (Paths/get path (into-array String []))))

2) Since both files and sockets are abstractions over bytes, cljs.repl.server/send-and-close should be agnostic to file encoding and just send whatever is in the file. Currently this is handled via cljs.repl.browser/mime-type->encoding.

I will prepare a patch by tomorrow.



 Comments   
Comment by Yegor Timoshenko [ 25/Dec/17 10:16 AM ]

I disagree with this being a minor defect. Currently most files are not served by the built-in HTTP server, which means that it's impossible to develop an application that has files other than HTML/CSS/JS.

Comment by David Nolen [ 26/Dec/17 7:03 AM ]

The builtin webserver is not an important component of ClojureScript. It exists to aid tutorials.

Comment by Yegor Timoshenko [ 26/Dec/17 7:10 AM ]

I like it much more than figwheel because it's simpler, and it seems to be feature-complete other than this bug. Also, this is the first in-browser REPL that comes to mind when using new `clojure` tooling.

Also, previous version of this patch didn't correctly count Content-Length, changed arity of send-and-close function, and didn't work for favicons. I'm attaching a new patch that this time should be complete and cause zero breakage.

New patch is called 0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch in the attachments above. Link: https://dev.clojure.org/jira/secure/attachment/17586/0001-CLJS-2433-use-JVM-to-deduce-MIME-type-send-files-via.patch

Here's a GitHub link (for code review/explanations): https://github.com/hackberryhike/clojurescript/commit/ea8336da3a779cb5982875d243dc6d40abd0d3ba

Comment by Yegor Timoshenko [ 26/Dec/17 7:18 AM ]

Also, if you don't like something about this patch, do tell me and I'll fix it right away. I'm very interested in getting this merged.





[CLJS-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-2412] Consuming npm module Created: 22/Nov/17  Updated: 22/Nov/17

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

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


 Description   

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

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

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

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

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

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

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



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

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

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





[CLJS-2410] Side-effectful assignments are wrongly eliminated by GCC under :advanced Created: 22/Nov/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: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

CLJS 1.9.946



 Description   

DEMO REPO: https://github.com/tonsky/closure-elimination-bug

Given this code:

(ns bug.core
  (:require
    [goog.object :as gobj]))


(defn get-obj []
  (js/document.querySelector "body"))


(defn ^:export aaa []
  (js/console.log "aaa start")
  (gobj/set (get-obj) "className" "+++aaa+++")
  (js/console.log "aaa end"))


(defn ^:export bbb []
  (js/console.log "bbb start")
  (set! (.-className (get-obj)) "+++bbb+++")
  (js/console.log "bbb end"))


(defn ^:export ccc []
  (js/console.log "ccc start")
  (aset (get-obj) "className" "+++ccc+++")
  (js/console.log "ccc end"))

gets compiled to

// Compiled by ClojureScript 1.9.946 {:static-fns true, :optimize-constants true}
goog.provide('bug.core');
goog.require('cljs.core');
goog.require('cljs.core.constants');
goog.require('goog.object');
bug.core.get_obj = (function bug$core$get_obj(){
return document.querySelector("body");
});
bug.core.aaa = (function bug$core$aaa(){
console.log("aaa start");

var G__4263_4266 = bug.core.get_obj();
var G__4264_4267 = "className";
var G__4265_4268 = "+++aaa+++";
goog.object.set(G__4263_4266,G__4264_4267,G__4265_4268);

return console.log("aaa end");
});
goog.exportSymbol('bug.core.aaa', bug.core.aaa);
bug.core.bbb = (function bug$core$bbb(){
console.log("bbb\u00A0start");

bug.core.get_obj().className = "+++bbb+++";

return console.log("bbb end");
});
goog.exportSymbol('bug.core.bbb', bug.core.bbb);
bug.core.ccc = (function bug$core$ccc(){
console.log("ccc start");

(bug.core.get_obj()["className"] = "+++ccc+++");

return console.log("ccc end");
});
goog.exportSymbol('bug.core.ccc', bug.core.ccc);

(fine so far). When we compile that with :optimizations :advanced, assignments are lost:

l("bug.core.aaa", function() {
  console.log("aaa start");
  return console.log("aaa end");
});
l("bug.core.bbb", function() {
  console.log("bbb start");
  return console.log("bbb end");
});
l("bug.core.ccc", function() {
  console.log("ccc start");
  return console.log("ccc end");
});

Expected: assignments are not eliminated.

Might be a GCC bug, but I can't build a minimal use-case to reproduce. Upgrading to [com.google.javascript/closure-compiler-unshaded "v20171023"] did not helped (same behaviour).



 Comments   
Comment by A. R [ 22/Nov/17 8:23 AM ]

Duplicate: CLJS-2031

TLDR: Add `:closure-warnings {:check-types :warning}` to your compiler options fixes this (which I've had on all my projects since that ticket)





[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-2406] ^:export not supported on defrecord Created: 21/Nov/17  Updated: 24/Nov/17

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

Type: Enhancement Priority: Minor
Reporter: Víctor M. Valenzuela Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

^:export doesn't appear to have an effect on defrecord - I only see Closure emitting the original symbols for def and defn.

I guess that similarly, deftype, defprotocol etc also would be reasonable targets.

In any case, it would be good to explicitly state what works and what does currently not (advanced-compilation.adoc), so one doesn't find himself blindly trying out things.



 Comments   
Comment by Mike Fikes [ 21/Nov/17 7:00 AM ]

You can export protocol methods. Proposed site change: https://github.com/clojure/clojurescript-site/pull/148

Comment by Mike Fikes [ 21/Nov/17 9:20 AM ]

^:export works on (and only on) Vars.

Protocol methods are Vars, so the above site change would help clarify how to export them.

The remaining Vars under consideration would be the map and positional factory functions and constructors that are synthetically generated for defrecord and deftype.

Perhaps a change could be accepted such that (defrecord ^:export Foo []) would export map->Foo, ->Foo, and Foo. And likewise (deftype ^:export Bar []) would export ->Bar and Bar.

Comment by Mike Fikes [ 22/Nov/17 6:10 AM ]

Workaround: Use goog/exportSymbol:

(ns my-ns.core)

(defrecord Foo [])
(goog/exportSymbol "my_ns.core.map__GT_Foo" my_ns.core.map__GT_Foo)
(goog/exportSymbol "my_ns.core.__GT_Foo" my_ns.core.__GT_Foo)
Comment by Víctor M. Valenzuela [ 23/Nov/17 3:34 PM ]

Hi Mike,

thanks so much for clarifying and taking care of this one! Appreciated.

^:export works on (and only on) Vars.

This would be an excelent piece of knowledge to include in the documentation IMO. Why not tell the whole story in a single place?

The original concern that made me open this issue was logging/debugging.

Say an application has a series of singleton defrecord instances, as part of a 'component' system (Sierra style).

Then, for logging, I might be tempted to (js/console.log (type the-instance)), so I can identify e.g. a buggy component.

But without language support, one is unable to get that (or something similar) to work without lots of boilerplate, or perhaps a hacky macro.

Would it be possible to make (type some-instance-of-a-defrecord-or-deftype) print the original name under advanced compilation?

Comment by Mike Fikes [ 24/Nov/17 9:16 AM ]

Hi Victor,

Thanks for the ticket intent clarification. ^:export is not for debugging, but is for making un-renamed symbols available to JavaScript. In fact, it doesn't change renamed and optimized JavaScript, but instead simply makes additional un-renamed aliases available which point into optimized / renamed code. (Thus, generally, debugging deals with the renamed / optimized code, even if there are un-renamed ^:exports sitting off to the side.)

Fortunately, :pseudo-names exists to help with debugging optimized code: https://clojurescript.org/reference/compiler-options#pseudo-names

For your example involving logging the type of the instance, instead of, for example [Function: ze] (under Node.js), you would get [Function: $my_ns$core$Foo$$] if :pseudo-names is enabled.

I've proposed revisions to the site: https://github.com/clojure/clojurescript-site/pull/150

If :pseudo-names satisfies the needs of this ticket, perhaps this ticket can be closed.





[CLJS-2402] Duplicate source files passed to Closure Compiler causes ModuleLoader#resolvePaths to throw "Duplicate module path after resolving" Created: 19/Nov/17  Updated: 22/Dec/17

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

Type: Defect Priority: Minor
Reporter: Corin Lawson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: modules, npm-deps

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

 Description   

I have a small (not minimal) repo here: https://github.com/au-phiware/boot-parsets

The repo includes a es6 module and a cljs file that both depend on a node module, which produces the following error.

Writing main.cljs.edn...
Compiling ClojureScript...
• main.js
                                       java.lang.Thread.run                  Thread.java:  748
         java.util.concurrent.ThreadPoolExecutor$Worker.run      ThreadPoolExecutor.java:  617
          java.util.concurrent.ThreadPoolExecutor.runWorker      ThreadPoolExecutor.java: 1142
                        java.util.concurrent.FutureTask.run              FutureTask.java:  266
                                                        ...                                   
                        clojure.core/binding-conveyor-fn/fn                     core.clj: 2022
                              adzerk.boot-cljs/compile-1/fn                boot_cljs.clj:  160
                                   adzerk.boot-cljs/compile                boot_cljs.clj:   72
                                          boot.pod/call-in*                      pod.clj:  413
                                                        ...                                   
org.projectodd.shimdandy.impl.ClojureRuntimeShimImpl.invoke  ClojureRuntimeShimImpl.java:  102
org.projectodd.shimdandy.impl.ClojureRuntimeShimImpl.invoke  ClojureRuntimeShimImpl.java:  109
                                                        ...                                   
                                          boot.pod/call-in*                      pod.clj:  410
                                      boot.pod/eval-fn-call                      pod.clj:  359
                                         clojure.core/apply                     core.clj:  657
                                                        ...                                   
                         adzerk.boot-cljs.impl/compile-cljs                     impl.clj:  151
                                       cljs.build.api/build                      api.clj:  205
                                         cljs.closure/build                  closure.clj: 2595
                             cljs.closure/handle-js-modules                  closure.clj: 2496
                            cljs.closure/process-js-modules                  closure.clj: 2389
                            cljs.closure/convert-js-modules                  closure.clj: 1680
                com.google.javascript.jscomp.Compiler.parse                Compiler.java:  995
          com.google.javascript.jscomp.Compiler.parseInputs                Compiler.java: 1731
      com.google.javascript.jscomp.deps.ModuleLoader.<init>            ModuleLoader.java:   92
com.google.javascript.jscomp.deps.ModuleLoader.resolvePaths            ModuleLoader.java:  276
java.lang.IllegalArgumentException: Duplicate module path after resolving: /home/corin/Projects/Demos/boot-parsets/node_modules/d3/d3.js
        clojure.lang.ExceptionInfo: Duplicate module path after resolving: /home/corin/Projects/Demos/boot-parsets/node_modules/d3/d3.js
    from: :boot-cljs
        clojure.lang.ExceptionInfo: Duplicate module path after resolving: /home/corin/Projects/Demos/boot-parsets/node_modules/d3/d3.js
    line: 33

Run `boot cljs` to reproduce the issue.

The patch attach removes duplicates from the set of input source files before they are preprocessed. With this patch the repo compiles correctly.



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

Hi Corin,

  1. Have you signed the CA? (Your name doesn't appear on https://clojure.org/community/contributors)
  2. Can you provide a minimal repro that doesn't employ downstream tooling? (Bug-filing details are at https://clojurescript.org/community/reporting-issues)
Comment by Corin Lawson [ 19/Nov/17 6:27 PM ]

1. I have signed the CA (I did it after filing this bug).
2. Not a problem, I'll get on to that later today. Is it okay to link to the project on GitHub, or should I upload a tarball?

Comment by Mike Fikes [ 19/Nov/17 6:48 PM ]

Hi Corin, it is not OK to link to GitHub, and any repro should not make use of any downstream tooling (Leiningen, Boot, etc.)—this means that a repro would ideally depend only on the shipping cljs.jar, executable like the examples at Quick Start https://clojurescript.org/guides/quick-start

Comment by Mike Fikes [ 19/Nov/17 6:59 PM ]

Hey Corin, you may want to submit the patch using the instructions at https://clojurescript.org/community/patches (your current patch won't apply using git am, which is what I suspect David uses in the end.

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

lein test is failing when applying patch

FAIL in (commonjs-module-processing) (module_processing_tests.clj:54)
processed modules are added to :libs
expected: (= {:foreign-libs [], :ups-foreign-libs [], :libs [(test/platform-path "out/src/test/cljs/reactJS.js") (test/platform-path "out/src/test/cljs/Circle.js")], :closure-warnings {:non-standard-jsdoc :off}} (env/with-compiler-env cenv (closure/process-js-modules {:foreign-libs [{:file "src/test/cljs/reactJS.js", :provides ["React"], :module-type :commonjs} {:file "src/test/cljs/Circle.js", :provides ["Circle"], :module-type :commonjs, :preprocess :jsx}], :closure-warnings {:non-standard-jsdoc :off}})))
  actual: (not (= {:foreign-libs [], :ups-foreign-libs [], :libs ["out/src/test/cljs/reactJS.js" "out/src/test/cljs/Circle.js"], :closure-warnings {:non-standard-jsdoc :off}} {:foreign-libs [], :closure-warnings {:non-standard-jsdoc :off}, :libs ["out/src/test/cljs/Circle.js" "out/src/test/cljs/reactJS.js"], :ups-foreign-libs []}))
Comment by Corin Lawson [ 19/Nov/17 7:48 PM ]

Thanks Mike,

I will make an effort follow all those instructions, but I do not see how I should provide the repro (no links, no attachments)... should it be inline code blocks?

Also, I shall be sure to run lein test before submitting the next patch. I note that the difference is only the order of the items in :libs vector, can you advise if order is important?

Comment by Mike Fikes [ 20/Nov/17 8:11 AM ]

Hi Corin,

Yes, inline code blocks are great. Anything minimal that doesn't depend on more than the shipping cljs.jar to demonstrate the issue (either directly in its REPL or via compiling using the build API). Here is a recent example using the build API: https://dev.clojure.org/jira/browse/CLJS-2397?focusedCommentId=47278&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-47278

There are actually other tests as well beyond lein test. See https://clojurescript.org/community/running-tests

I haven't looked into the details of this issue, so can't speak to to whether order of items is important.

Comment by Corin Lawson [ 20/Nov/17 8:47 AM ]

Steps to reproduce the problem.

Consider the following three source files:

src/distinct_inputs/core.cljs
(ns distinct-inputs.core
  (:require [d3]
            [circle :refer [circle]]))

(-> d3
    (.select "body")
    (.append "svg")
    (.attr "width" 200)
    (.attr "height" 200)
    (.call circle "steelblue"))
es6/circle.js
import * as d3 from 'd3';

export function circle(sel, color) {
  return sel
    .append("circle")
    .attr("cx", "100px")
    .attr("cy", "100px")
    .attr("r",  "100px")
    .attr("fill", color);
}
build.clj
(require 'cljs.build.api)

(cljs.build.api/build
  "src"
  {:main 'distinct-inputs.core
   :output-to "out/main.js"
   :install-deps true
   :foreign-libs [{:file "es6"
                   :module-type :es6}]
   :npm-deps     {:d3 "3.5.16"}})

Execute cljs:

java -cp cljs.jar:src clojure.main build.clj

Expected outcome

cljs should produce nothing to the standard output, exit cleanly and write the following files (approximately).

out/main.js
var CLOSURE_UNCOMPILED_DEFINES = {};
var CLOSURE_NO_DEPS = true;
if(typeof goog == "undefined") document.write('<script src="out/goog/base.js"></script>');
document.write('<script src="out/goog/deps.js"></script>');
document.write('<script src="out/cljs_deps.js"></script>');
document.write('<script>if (typeof goog == "undefined") console.warn("ClojureScript could not load :main, did you forget to specify :asset-path?");</script>');
document.write('<script>goog.require("process.env");</script>');
document.write('<script>goog.require("distinct_inputs.core");</script>');
out/distinct_inputs/core.js
goog.provide('distinct_inputs.core');
goog.require('cljs.core');
goog.require('module$distinct_inputs$node_modules$d3$d3');
goog.require('module$distinct_inputs$es6$circle');
module$distinct_inputs$node_modules$d3$d3.select("body").append("svg").attr("width",(200)).attr("height",(200)).call(module$distinct_inputs$es6$circle.circle,"steelblue");

//# sourceMappingURL=core.js.map
out/es6/circle.js
goog.provide("module$distinct_inputs$es6$circle");goog.require("module$distinct_inputs$node_modules$d3$d3");function circle$$module$distinct_inputs$es6$circle(sel,color){return sel.append("circle").attr("cx","100px").attr("cy","100px").attr("r","100px").attr("fill",color)}module$distinct_inputs$es6$circle.circle=circle$$module$distinct_inputs$es6$circle

Actual outcome.

cljs exits with exit code 1 and produces the following standard out.

stdout
Exception in thread "main" java.lang.IllegalArgumentException: Duplicate module path after resolving: /distinct_inputs/node_modules/d3/d3.js, compiling:(/distinct_inputs/build.clj:3:1)
	at clojure.lang.Compiler.load(Compiler.java:7391)
	at clojure.lang.Compiler.loadFile(Compiler.java:7317)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$script_opt.invokeStatic(main.clj:335)
	at clojure.main$script_opt.invoke(main.clj:330)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj:384)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: Duplicate module path after resolving: /distinct_inputs/node_modules/d3/d3.js
	at com.google.javascript.jscomp.deps.ModuleLoader.resolvePaths(ModuleLoader.java:276)
	at com.google.javascript.jscomp.deps.ModuleLoader.<init>(ModuleLoader.java:92)
	at com.google.javascript.jscomp.Compiler.parseInputs(Compiler.java:1731)
	at com.google.javascript.jscomp.Compiler.parse(Compiler.java:995)
	at cljs.closure$convert_js_modules.invokeStatic(closure.clj:1680)
	at cljs.closure$process_js_modules.invokeStatic(closure.clj:2371)
	at cljs.closure$handle_js_modules.invokeStatic(closure.clj:2495)
	at cljs.closure$build.invokeStatic(closure.clj:2592)
	at cljs.build.api$build.invokeStatic(api.clj:204)
	at cljs.build.api$build.invoke(api.clj:189)
	at cljs.build.api$build.invokeStatic(api.clj:192)
	at cljs.build.api$build.invoke(api.clj:189)
	at user$eval24.invokeStatic(build.clj:3)
	at user$eval24.invoke(build.clj:3)
	at clojure.lang.Compiler.eval(Compiler.java:6927)
	at clojure.lang.Compiler.load(Compiler.java:7379)
	... 11 more

None of the aforementioned expected files are produced.

Cause of the exception.

The exception emitted by ModuleLoader#resolvePaths is a result of the same input file (i.e. node_modules/d3/d3.js) having been specified more than once to Compiler#initModules. There happens to be this note in Compiler#getAllInputsFromModules:

src/com/google/javascript/jscomp/Compiler.java
// NOTE(nicksantos): If an input is in more than one module,
        // it will show up twice in the inputs list, and then we
        // will get an error down the line.

cljs.closure/process-js-modules is provided a :foreign-libs vector which contains a repeated entry for node_modules/d3/d3.js (and also it's package.json). That vector is a result of multiple invocations of cljs.closure/node-inputs; once for out/cljs$node_modules.js (which is presumably a result of the dependency in distinct_inputs/core) and again for es6/circle.js.

In short, the dependency on D3 is pulled in by both ClojureScript source files and JavaScript module source files.

Proposed solution.

In this scenario the :foreign-libs vector contains repeated entries dispite the use of distinct within cljs.closure/node-inputs. A possible solution would to remove the use of distinct within cljs.closure/node-inputs and require the caller of cljs.closure/node-inputs to use distinct.

Solution A
From 063e35080c14d35189ab7827f25f071e958ab5b4 Mon Sep 17 00:00:00 2001
From: Corin Lawson <corin@responsight.com>
Date: Tue, 21 Nov 2017 01:31:53 +1100
Subject: [PATCH] CLJS-2402: Ensure :foreign-libs vector contains distinct
 entries.

---
 src/main/clojure/cljs/closure.clj | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/main/clojure/cljs/closure.clj b/src/main/clojure/cljs/closure.clj
index a686f878..74a0cc86 100644
--- a/src/main/clojure/cljs/closure.clj
+++ b/src/main/clojure/cljs/closure.clj
@@ -2219,7 +2219,7 @@
      (when env/*compiler*
        (:options @env/*compiler*))))
   ([entries opts]
-   (into [] (distinct (mapcat #(node-module-deps % opts) entries)))))
+   (into [] (mapcat #(node-module-deps % opts) entries))))
 
 (defn index-node-modules
   ([modules]
@@ -2480,14 +2480,15 @@
         output-dir (util/output-directory opts)
         opts (update opts :foreign-libs
                (fn [libs]
-                 (into (if (= target :nodejs)
-                         []
-                         (index-node-modules node-required))
-                   (into expanded-libs
-                     (node-inputs (filter (fn [{:keys [module-type]}]
-                                            (and (some? module-type)
-                                              (not= module-type :amd)))
-                                    expanded-libs))))))
+                 (distinct
+                   (into (if (= target :nodejs)
+                           []
+                           (index-node-modules node-required))
+                         (into expanded-libs
+                               (node-inputs (filter (fn [{:keys [module-type]}]
+                                                      (and (some? module-type)
+                                                           (not= module-type :amd)))
+                                                    expanded-libs)))))))
         opts (if (some
                    (fn [ijs]
                      (let [dest (io/file output-dir (rel-output-path (assoc ijs :foreign true) opts))]
-- 
2.13.0

A more general solution is cljs.closure/process-js-modules must ensure the set of input files (i.e. js-modules) is distinct. This patch would be simpler (i.e. doesn't mess with code I don't understand) and closer to the call to Google Closure Compiler.

Solution B
From 6bf11a24b93642e118e6d29c5af8a137fa01ea94 Mon Sep 17 00:00:00 2001
From: Corin Lawson <corin@responsight.com>
Date: Sun, 19 Nov 2017 20:25:31 +1100
Subject: [PATCH] CLJS-2402: Ensure input source files are distinct.

---
 src/main/clojure/cljs/closure.clj | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/main/clojure/cljs/closure.clj b/src/main/clojure/cljs/closure.clj
index a686f878..24421bde 100644
--- a/src/main/clojure/cljs/closure.clj
+++ b/src/main/clojure/cljs/closure.clj
@@ -2364,7 +2364,8 @@
   (let [;; Modules from both :foreign-libs (compiler options) and :ups-foreign-libs (deps.cljs)
         ;; are processed together, so that files from both sources can depend on each other.
         ;; e.g. commonjs module in :foreign-libs can depend on commonjs module from :ups-foreign-libs.
-        js-modules (filter :module-type (concat (:foreign-libs opts) (:ups-foreign-libs opts)))]
+        js-modules (filter :module-type (concat (:foreign-libs opts) (:ups-foreign-libs opts)))
+        js-modules (distinct js-modules)]
     (if (seq js-modules)
       (util/measure (:compiler-stats opts)
         "Process JS modules"
-- 
2.13.0

FWIW: I prefer Solution B.

Comment by Corin Lawson [ 20/Nov/17 8:49 AM ]

Attached proposed Solution B

Comment by Corin Lawson [ 20/Nov/17 9:02 AM ]

Hi Mike,

I hope this is to your's (and BDFL's) satisfaction now; I ran lein test for both proposed solutions and I do not receive any failures. I do receive errors, however, that do not occur in assertions. I assume that the cause is something peculiar (or lack thereof) in my setup. Let me know if you require anything else from me.

Cheers,
Corin.

Comment by Mike Fikes [ 20/Nov/17 12:08 PM ]

Thanks Corin. The entire test suite passes for me with your latest patch.





[CLJS-2401] build with :optimizations is not working on Windows Created: 18/Nov/17  Updated: 22/Dec/17

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

Type: Defect Priority: Minor
Reporter: Dmitry Gusarov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

I got cljs.jar from:
https://github.com/clojure/clojurescript/releases/tag/r1.9.946


Attachments: Zip Archive prj.zip    

 Description   

Just went through Quick Start and got an error at 'Production Builds' section.
Looks like it currently unable to compile with :optimizations

Exception in thread "main" java.nio.file.InvalidPathException: Illegal char <:> at index 2: /C:/prj/out/cljs/core.js, compiling:(C:\prj\release.clj:3:1)
        at clojure.lang.Compiler.load(Compiler.java:7391)
        at clojure.lang.Compiler.loadFile(Compiler.java:7317)
        at clojure.main$load_script.invokeStatic(main.clj:275)
        at clojure.main$script_opt.invokeStatic(main.clj:335)
        at clojure.main$script_opt.invoke(main.clj:330)
        at clojure.main$main.invokeStatic(main.clj:421)
        at clojure.main$main.doInvoke(main.clj:384)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:379)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.Var.applyTo(Var.java:700)
        at clojure.main.main(main.java:37)
Caused by: java.nio.file.InvalidPathException: Illegal char <:> at index 2: /C:/prj/out/cljs/core.js
        at sun.nio.fs.WindowsPathParser.normalize(Unknown Source)
        at sun.nio.fs.WindowsPathParser.parse(Unknown Source)
        at sun.nio.fs.WindowsPathParser.parse(Unknown Source)
        at sun.nio.fs.WindowsPath.parse(Unknown Source)
        at sun.nio.fs.WindowsFileSystem.getPath(Unknown Source)
        at com.google.javascript.jscomp.SourceMapResolver.getRelativePath(SourceMapResolver.java:73)
        at com.google.javascript.jscomp.SourceMapResolver.extractSourceMap(SourceMapResolver.java:53)
        at com.google.javascript.jscomp.JsAst.parse(JsAst.java:168)
        at com.google.javascript.jscomp.JsAst.getAstRoot(JsAst.java:55)
        at com.google.javascript.jscomp.CompilerInput.getAstRoot(CompilerInput.java:122)
        at com.google.javascript.jscomp.Compiler.hoistNoCompileFiles(Compiler.java:1992)
        at com.google.javascript.jscomp.Compiler.orderInputs(Compiler.java:1890)
        at com.google.javascript.jscomp.Compiler.parseInputs(Compiler.java:1793)
        at com.google.javascript.jscomp.Compiler.parseForCompilationInternal(Compiler.java:982)
        at com.google.javascript.jscomp.Compiler.access$300(Compiler.java:102)
        at com.google.javascript.jscomp.Compiler$6.call(Compiler.java:964)
        at com.google.javascript.jscomp.Compiler$6.call(Compiler.java:961)
        at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:101)
        at java.util.concurrent.FutureTask.run(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
        at java.lang.Thread.run(Unknown Source)





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

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

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


 Description   

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

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

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


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

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

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

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

var b=g4.ki([1,2,3],1);




[CLJS-2398] cljs.core.Var shouldn't satisfy IWithMeta Created: 12/Nov/17  Updated: 19/Nov/17

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

Type: Defect Priority: Minor
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File 0001-Remove-IWithMeta-implementation-from-Var.patch    
Patch: Code

 Description   

cljs.core.Var implements IWithMeta, which differs from vars in Clojure which do not have immutable metadata. with-meta shouldn't be supported on vars, only alter-meta! should.

Unlike Clojure, ClojureScript doesn't use dynamic dispatch for alter-meta!. The implementation of cljs.core/alter-meta! will happily mutate the metadata of any object with a "meta" field, even if it's not a ref/var/whatever identity. This means that if you want to have metadata on your custom type, you have to have a "meta" field, rather than implement methods for getting/changing metadata. That's not too ideal, but doesn't present an immediate problem for me.

The reason I care about IWithMeta is that I need to distinguish metadata handling between values and reference types. Ideally, there would be an IAlterMeta protocol as well, but (and (satisfies? IMeta x) (not (satisfies? IWithMeta x))) suffices. On the JVM, the same can be done with IMeta and IObj.



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

Unit test failure:

ERROR in (test-1248) (core-advanced-test.js:64:417)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[Error Error: No protocol method IWithMeta.-with-meta defined for type object: #'cljs.core/first]




[CLJS-2395] (str (goog.date.Date. 2017 11 8)) behaves differently with no optimizations than with :simple or :advanced Created: 08/Nov/17  Updated: 19/Jan/18

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

Type: Defect Priority: Minor
Reporter: Eero Helenius Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Zip Archive cljs-goog-date-str-bug.zip    

 Description   

With :optimizations :none, (str (goog.date.Date. 2017 11 8)) returns 20171208, but with :simple or :advanced, it returns 1512684000000.

I attached a simple test case with a README.



 Comments   
Comment by A. R [ 08/Nov/17 6:41 AM ]

Closure compiler changed at some point:

$cljs$core$str$$.$cljs$core$IFn$_invoke$arity$1$ = function $Qd($x$jscomp$279$$) {
  return null == $x$jscomp$279$$ ? "" : "" + $x$jscomp$279$$;
};

It used to keep our [x].join("") code.

Workaround could be to emit a [x, ""].join("")

See CLJS-890 for details

Comment by A. R [ 19/Jan/18 1:05 AM ]

Reported it a bit ago: https://github.com/google/closure-compiler/issues/2782

They're now rewriting x.join("") to String:

https://github.com/google/closure-compiler/commit/b6973ec7b37f3890c8b0e11456afa95d79aaffab

which would also fix CLJS-1631 .





[CLJS-2393] process.env preload is added after all existing preloads Created: 31/Oct/17  Updated: 17/Nov/17

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

Type: Defect Priority: Minor
Reporter: Eugene Pakhomov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: npm-deps


 Description   

When there're other preloads and the check for process-shim returns true, `process.env` is added to the end of the vector of all preloads.
That causes problems when one of the previous preloads requires something that checks `process.env` during loading.

E.g. I have a Reagent application with `:npm-deps` set to React and a few other libraries. And I have a preload exactly like this one: https://github.com/flexsurfer/re-frisk/blob/master/src/re_frisk/preload.cljs
The issue is that `re-frisk.core` requires Reagent, which requires React, which checks `process.env` - all before `process.env` was actually created.

I think that `process.env` should go before all existing preloads.



 Comments   
Comment by Hendrik Poernama [ 02/Nov/17 8:02 AM ]

Workaround is to manually add process.env to the first of :preloads vector





[CLJS-2392] WARNING: out/inferred_externs.js: WARNING - name goog is not defined in the externs. Created: 30/Oct/17  Updated: 01/Dec/17

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

Type: Defect Priority: Minor
Reporter: Arne Brasseur Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 1.9.946 (standalone cljs.jar)



 Description   

See demo and explanation here: https://github.com/lambdaisland/npmdemo/tree/goog-undefined-minimal

  • :target :nodejs
  • :optimizations :advanced
  • :infer-externs true
  • :install-deps true

The inferred_externs.js contains goog.isArrayLike and goog.global, and the compilation complains on these lines that "goog" is not defined.

Oct 30, 2017 1:46:36 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: out/inferred_externs.js:3: WARNING - name goog is not defined in the externs.
goog.isArrayLike;
^^^^

Oct 30, 2017 1:46:36 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: out/inferred_externs.js:4: WARNING - name goog is not defined in the externs.
goog.global;


 Comments   
Comment by Mike Rodriguez [ 01/Dec/17 3:09 PM ]

I don't have a full demo project for this like the description does.

However, I figured I'd add that I see similar with a build like:

:closure-defines {goog.DEBUG false}
:infer-externs true
:optimizations :advanced
:pretty-print false

Notice that there is no :target :nodejs or :intall-deps true configured here.

I have warnings for externs, such as

WARNING: /path/to/file/inferred_externs.js:33: WARNING - name goog is not defined in the externs.
goog.DEBUG;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:34: WARNING - name goog is not defined in the externs.
goog.string;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:35: WARNING - name goog is not defined in the externs.
goog.string.StringBuffer;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:36: WARNING - name goog is not defined in the externs.
goog.string.StringBuffer.prototype.append;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:37: WARNING - name goog is not defined in the externs.
goog.isArrayLike;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:38: WARNING - name goog is not defined in the externs.
goog.net;
^^^^

Dec 01, 2017 3:54:23 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /path/to/file/inferred_externs.js:39: WARNING - name goog is not defined in the externs.
goog.net.XhrIo;
^^^^

The "inferred_externs.js" file has lines

goog.DEBUG;
goog.string;
goog.string.StringBuffer;
goog.string.StringBuffer.prototype.append;
goog.isArrayLike;
goog.net;
goog.net.XhrIo;

with goog never pre-defined. Of course it doesn't need to be of course.





[CLJS-2388] Creating :doc in attr-string? in defn throws java.lang.ClassCastException Created: 25/Oct/17  Updated: 22/Dec/17

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

Type: Defect Priority: Minor
Reporter: Vitalie Spinu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Trying to build doc on the fly:

cljs.user=> (defn tt {:doc (str "A" "B")} [])
cljs.user=> java.lang.ClassCastException: clojure.lang.PersistentList cannot be cast to java.lang.CharSequence
	at clojure.string$split.invokeStatic(string.clj:219)
	at clojure.string$split_lines.invokeStatic(string.clj:228)
	at cljs.compiler$emit_comment$print_comment_lines__3266.invoke(compiler.cljc:625)
	at cljs.compiler$emit_comment.invokeStatic(compiler.cljc:639)
	at cljs.compiler$fn__3297.invokeStatic(compiler.cljc:665)
	at cljs.compiler$fn__3297.invoke(compiler.cljc:659)
	at clojure.lang.MultiFn.invoke(MultiFn.java:229)
	at cljs.compiler$emit.invokeStatic(compiler.cljc:198)
...

Setting other attributes doesn't evaluate the code:

cljs.user=> (defn tt {:ddd (str "A" "B")} [])
cljs.user=> #'cljs.user/tt
cljs.user=> (:ddd (meta #'tt))
cljs.user=> (str "A" "B")
cljs.user=>





[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-2385] cljs.analyzer/infer-type pass infers tag with incorrect priority Created: 17/Oct/17  Updated: 17/Oct/17

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

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

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

 Description   

I tracked down an issue related to externs inference and found the culprit to be in the infer-type analyzer pass.

(defn thing [this]
  (.componentDidUpdate ^js/Thing this))

This will be picked up by :infer-externs correctly.

(defn thing [{:as this}]
  (.componentDidUpdate ^js/Thing this))

This won't be. The tag will be ignored.

I tracked this down to cljs.analyzer/get-tag. Since the binding is destructured it is already tagged as clj, found in :info. This causes it to never look at the form itself and discard the explicit tag.

(defn get-tag [e]
  (if-some [tag (-> e :tag)]
      tag
      (if-some [tag (-> e :info :tag)]
          tag
          (-> e :form meta :tag))))

I believe that any tag on the form itself should always win.

(defn get-tag [e]
  (if-some [tag (-> e :form meta :tag)]
    tag
    (if-some [tag  (-> e :tag)]
      tag
      (-> e :info :tag)
      )))

Changing the lookup order accordingly fixes the missing externs issue.

I'm not 100% confident my solution is correct here. :tag is handled quite differently in several places, some set it explicitly some let the infer-type pass find it.

No clue how to write a test for this.






[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-2382] circular dependency in node_modules prevents compilation Created: 13/Oct/17  Updated: 13/Oct/17

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

Type: Defect Priority: Minor
Reporter: Hendrik Poernama Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

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

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

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

Example of affected node_modules: apollo-client 1.9.2

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

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






[CLJS-2381] Race condition in cljs.analyzer/load-core Created: 10/Oct/17  Updated: 10/Oct/17

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

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


 Description   

There is a race condition in cljs.analyzer/load-core.

#?(:clj
   (defn load-core []
     (when (not @-cljs-macros-loaded)
       (reset! -cljs-macros-loaded true)
       (if *cljs-macros-is-classpath*
         (locking load-mutex
           (load *cljs-macros-path*))
         (locking load-mutex
           (load-file *cljs-macros-path*))))
     (intern-macros 'cljs.core)))

When 2+ threads with independent cljs.env/*compiler* bindings call load-core at the same time only one will trigger the actual load and the others will proceed directly to intern-macros and begin interning the macros into their compiler env. Since the load of the first thread may not yet be finished with loading the actual cljs.core macros the second thread will have an incomplete set of macros in its compiler env.

Related: https://dev.clojure.org/jira/browse/CLJS-1963






[CLJS-2380] deftype/defrecord with Object resolves incorrectly Created: 08/Oct/17  Updated: 08/Oct/17

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

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


 Description   

Whenever any deftype/defrecord defines methods on Object the CLJS compiler will emit an invalid type annotation which Closure complains about when running with type checking enabled.

(ns demo.type)

(deftype Foo [a b]
  Object
  (foo [this x]
    x))

produces this JS

/**
* @constructor
 * @implements {demo.type.Object}
*/
demo.type.Foo = (function (a,b){
this.a = a;
this.b = b;
});

produces the warning

Bad type annotation. Unknown type demo.type.Object

Looking at the analyzer data suggests that it indeed resolves incorrectly.

...
 :defs
 {Foo
  {:num-fields 2,
   :protocols #{demo.type/Object},
   :name demo.type/Foo,
   :file "demo/type.cljs",
   :end-column 13,
   :type true,
   :column 10,
   :line 3,
   :record false,
   :end-line 3,
   :skip-protocol-flag nil},

Not entirely sure how this bypasses checks but it might be a sign that there is an overly general check for Object which does not check the namespace.

AFAICT there are no bad effects on the CLJS analyzer/compiler. Closure does complain when type checking is enabled but otherwise seems unaffected.

Not yet sure where the issue starts, but looks like cljs.analyzer/resolve-var not correctly resolving Object without an ns, via [1].

[1] https://github.com/clojure/clojurescript/blob/998933f5090254611b46a2b86626fb17cabc994a/src/main/clojure/cljs/core.cljc#L1669-L1673






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

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

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 8
Labels: None

Attachments: Text File CLJS-2376-1.patch    

 Description   

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

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

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

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

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

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

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

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

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

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

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

The long version that already works is

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

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

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



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

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

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





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

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

Type: Defect Priority: Minor
Reporter: Corin Lawson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: npm-deps


 Description   

How to reproduce problem

  1. {{git clone https://github.com/au-phiware/cljsbuild-bootstrap4.git}}
  2. cd cljsbuild-bootstrap4
  3. make CLJS=path/to/cljs.jar
  4. open index.html
  5. Observe console messages.

Expected result

  1. Console should show the following messages:
    Error: Bootstrap dropdown require Popper.js (https://popper.js.org)
    Hello world!
  2. The Bootstrap Alert component should fade from the page resulting in a blank page.
  3. Compiler should produce:
    out/cljs_deps.js
    goog.addDependency("base.js", ['goog'], []);
    goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
    goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
    goog.addDependency("../node_modules/bootstrap/dist/js/bootstrap.js", ['bootstrap'], []); 
    goog.addDependency("../cljsbuild_bootstrap4/core.js", ['cljsbuild_bootstrap4.core'], ['cljs.core', 'bootstrap']);

Actual result

  1. Console shows
    Error: Undefined nameToPath for bootstrap
  2. The Bootstrap Alert component fails to fade from the page and remains on the page.
  3. Compiler produces:
    out/cljs_deps.js
    goog.addDependency("base.js", ['goog'], []);
    goog.addDependency("../cljs/core.js", ['cljs.core'], ['goog.string', 'goog.object', 'goog.math.Integer', 'goog.string.StringBuffer', 'goog.array', 'goog.math.Long']);
    goog.addDependency("../process/env.js", ['process.env'], ['cljs.core']);
    goog.addDependency("../cljsbuild_bootstrap4/core.js", ['cljsbuild_bootstrap4.core'], ['cljs.core', 'bootstrap']);


 Comments   
Comment by Hendrik Poernama [ 28/Sep/17 7:44 PM ]

I came across the same problem and did some tracing. It looks like 'load-foreign-library' correctly populated the 'provides' key, but 'library-graph-node' failed to get the 'provides'. I also observed that the output of library-graph-node is the one that gets passed to cljs_deps.js

load-foreign-library: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/dist/js/bootstrap.js
provides: ["bootstrap" "bootstrap/dist/js/bootstrap.js" "bootstrap/dist/js/bootstrap"]

load-foreign-library: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/package.json
provides: []

Copying file:/Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/node_modules/bootstrap/dist/js/bootstrap.js to out/node_modules/bootstrap/dist/js/bootstrap.js

load-library: out/node_modules/bootstrap/dist/js/bootstrap.js

library-graph-node: /Users/Hendrik/Projects/bugreports/cljsbuild-bootstrap4/out/node_modules/bootstrap/dist/js/bootstrap.js
{:requires [], :provides [],
:url #object[java.net.URL 0x6a0659ac "file:/<snip>/cljsbuild-bootstrap4/out/node_modules/bootstrap/dist/js/bootstrap.js"],
:closure-lib true, :lib-path "out/node_modules/bootstrap/dist/js/bootstrap.js"}

Comment by Hendrik Poernama [ 29/Sep/17 3:31 AM ]

I am also getting the same error when trying to use 'apollo-client'. 'cljs_deps.js' would be missing the 'whatwg-fetch' dependency.

I suspect this is because 'whatwg-fetch' is a polyfill that does not explicitly export anything. I was able to workaround the issue by appending the following lines to the bottom of '<projectdir>/node_modules/whatwg-fetch/fetch.js'

const whatwg_fetch = 1;
export { whatwg_fetch };

I don't know enough to say that this is the same problem, other than the error message being identical. I have not tried adding the same hack to bootstrap.js

Comment by Derek Chiang [ 08/Jan/18 2:03 AM ]

Getting the same error for `ethereumjs-abi` too.

Log from console:

```
base.js:1357 Uncaught Error: Undefined nameToPath for ethereumjs_abi
at visitNode (base.js:1357)
at visitNode (base.js:1355)
at visitNode (base.js:1355)
at Object.goog.writeScripts_ (base.js:1369)
at Object.goog.require (base.js:706)
at (index):46
```





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

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

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

n/a



 Description   

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

In Clojure,

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

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

Note that simpler forms do work correctly:

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

returns ([x]) in both environments.

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



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

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





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

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

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


 Description   

When a namespace requires itself, the compilation hangs without an error message.
It would be nice to either throw an error, or at least print a warning message to the user.



 Comments   
Comment by Mike Fikes [ 20/Nov/17 3:28 PM ]

Cannot repro with 1.9.908 nor 1.9.946:

src/itself/core.cljs:

(ns itself.core
  (:require itself.core))
$ java -cp cljs.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 54877
To quit, type: :cljs/quit
cljs.user=> (require 'itself.core)
clojure.lang.ExceptionInfo: Assert failed: Circular dependency detected, cljs.user -> itself.core -> itself.core
Comment by Frozenlock [ 04/Dec/17 11:21 AM ]

I still get the same behavior with 1.9.946.
Tried with figwheel and uberjar.
Both hang without an error message.

Could it be a problem with the toolchain, say Leiningen?

Comment by Mike Fikes [ 04/Dec/17 11:25 AM ]

Yes. With Figwheel, I'd try disabling :autoload (see https://github.com/bhauman/lein-figwheel/wiki/Configuration-Options#client)

If that allows you to isolate it to Figwheel, perhaps this JIRA can be closed.

Comment by Frozenlock [ 03/Jan/18 4:05 PM ]

This wouldn't change much in this case : it's hanging when running Figwheel or Uberjar because it can't complete the cljs compilation.

I've tested with lein-cljsbuild and I have the same issue.
I'll check with the cljsbuild team if the problem is on their side.





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

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

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


 Description   

The following code produces different results in clojure and in clojurescript

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

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



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

Ethan, I suspect this ticket is invalid. Are you defining the macro in the REPL? If you do that, you will see the consequence that macros are really just functions called by the compiler (with two extra special arguments &env and &form, which you are passing as 0 and 1 in your example code).

Macros in ClojureScript need to be defined in a separate namespace and consumed using :require-macros. See more at https://clojurescript.org/about/differences#_macros





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

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

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


 Description   

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

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

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

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






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

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

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

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


Attachments: Text File CLJS-2351-1.patch     Text File CLJS-2351.patch    
Approval: Vetted

 Description   

consider function with no parameters

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

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

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

:arglists does not get affected.



 Comments   
Comment by Hlöðver Sigurðsson [ 09/Dec/17 8:54 PM ]

I just submitted a patch, never used jira patch system before, that aside, the patch will make the compiler respect :arglists metadata when the user provdes one. Hope to get feedback and merge on this

Comment by David Nolen [ 11/Dec/17 8:53 AM ]

Thanks have you submitted your Clojure CA?

Comment by David Nolen [ 11/Dec/17 8:56 AM ]

Patch review, I don't really understand the purpose the changing arity of variadic-fn and adding this new flag. Just put the logic directly into variadic-fn no?

Comment by Hlöðver Sigurðsson [ 11/Dec/17 9:35 AM ]

Yes, I'll find a better solution to my patch, just that the `m` symbol in the let binding on line 3176

m (conj {:arglists (core/list 'quote (sigs fdecl))} m)

Merges the arglists and there's after this point no way to know if the :arglists came from a provided metadata or not. But I'll rename the let symbols and it's easy to avoid adding arity, I'll do another patch later today.

No haven't submitted my Clojure Contibutor Agreement, I'm filling it out now...

Comment by Hlöðver Sigurðsson [ 12/Dec/17 1:48 PM ]

Yesterday I signed the Clojure CA
"Clojure CA between Rich Hickey and Hlöðver Sigurðsson is Signed and Filed!"

The code:
The symbol name m (for metadata) is already not very good, but I just added a comma to differentiate between vararg-fn case and the other cases.

I found as I was testing this that multi-arity-fn arglist metadata does not work with or without specifically adding arglists. If so, I can open another jira ticket for that.





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

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

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


 Description   

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

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



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

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





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

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Unresolved Votes: 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-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-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-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-2329] REPL should not error out if quote is missing on string require Created: 18/Aug/17  Updated: 18/Aug/17

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

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


 Description   

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

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

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

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

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

----  Analysis Error  ----

Minor, solvable with:

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





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

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

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

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

 Description   

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

(mod 2.1 3) ; => 2.0999999999999996

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

Two possible solutions:

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


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

Patch welcome.

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

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

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

Great thanks, have you submitted your Clojure CA?

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

Yeah, 2h ago.

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

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

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

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

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

André, ClojureScript requires squashed patches (CLJS-2319-Fix-cljs.core-mod-handling-of-floats_INLINED.patch has two patches in it). See https://clojurescript.org/community/patches

Comment by Mike Fikes [ 19/Nov/17 9:13 PM ]

Arg... something in the patch is killing the older JavaScriptCore we run tests with in Travis. My only guess is the additional mod tests may be triggering CLJS-910, but that seems unlikely.





[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-2257] Expand dotted symbols into field accesses in the analyzer Created: 17/Jul/17  Updated: 17/Jul/17

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

Type: Enhancement Priority: Minor
Reporter: Ambrose Bonnaire-Sergeant Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently, there is a lot of implicit information in a :var node in the analyzer around dotted symbols.

The :name of a :var node can contain implicit field accesses, and this information must be manually disambiguated with every new tool that consumes the CLJS AST (like core.typed).

A solution might be to expand specific dotted :var nodes into :dot nodes containing a :var node. Locals in particular might benefit from this transformation, would others? (Global variables, js/* variables)






[CLJS-2252] Self-host: :redef-in-file doesn't trigger for bootstrapped Created: 16/Jul/17  Updated: 16/Jul/17

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

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


 Description   

If you redefine a symbol in a file and require that file using self-hosted ClojureScript, the :redef-in-file diagnostic doesn't trigger.

It is difficult to create a minimal repro for this, as it requires a setup that loads files. (Perhaps one can be made with the script/test-self-parity infrastructure).

It appears that this can be resolved by setting ana/file-defs at the right places and unsetting it at the right async completion points.






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

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

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


 Description   

Defrecord produces code that's incompatible with ECMASCRIPT3 language:

(defrecord Query [with in])
Compiling "target/main.js" from ["src"]...
Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                  ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                               ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 4 warning(s)
Successfully compiled "target/main.js" in 6.224 seconds.

Probably there should be some name escaping happening? I see that (defn with []) is compiled as cljs-test.core.with$. Should we do the same for records?



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

The problem has less to do with records than how to handle reserved names. As far as I'm concerned the Closure warnings are sufficient, but if somebody wants to devise a warning patch that warns on using reserved fields names on deftpye/record when the output language is ES3, then be my guest.

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

Related CLJS-871

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

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





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

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

Type: Defect Priority: Minor
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Defrecord produces code that's incompatible with ECMASCRIPT3 language:

(defrecord Query [with in])
Compiling "target/main.js" from ["src"]...
Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                            ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                  ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: /Users/prokopov/work/cljs-test/target/cljsbuild-compiler-0/cljs_test/core.js:117: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
return (!((other13203 == null))) && ((this13202__$1.constructor === other13203.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.with,other13203.with)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.in,other13203.in)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this13202__$1.__extmap,other13203.__extmap));
                                                                                                                                                                                                                                                               ^

Jul 14, 2017 6:58:14 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 0 error(s), 4 warning(s)
Successfully compiled "target/main.js" in 6.224 seconds.

Probably there should be some name escaping happening? I see that (defn with []) is compiled as cljs-test.core.with$. Should we do the same for records?



 Comments   
Comment by David Nolen [ 01/Aug/17 5:43 PM ]

Now that CLJS-1620 is done, we should think about this more deeply.

Comment by Stephen Sugden [ 01/Oct/17 11:19 AM ]

Ran into an interesting bug that is likely a consequence of this:

cljs.user=> (defrecord T [arguments])
cljs.user/T
cljs.user=> (= (T. [1]) (T. [0]))
true




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

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

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

Approval: Vetted

 Description   

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






[CLJS-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-2168] Properly document browser-env options Created: 04/Jul/17  Updated: 08/Jul/17

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

Type: Defect Priority: Minor
Reporter: Timothy Pote Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: browser-repl

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

 Description   

There are a number of browser-env options that work only partially or not at all:

  • :optimizations - Only :whitespace and :simple appear to work for me
  • :host - Is never read. Instead, we always bind to 0.0.0.0.
  • :serve-static - Is never read.
  • :preloaded-libs - Is never read.

These should either be properly documented, removed, or made to work.



 Comments   
Comment by Timothy Pote [ 04/Jul/17 4:55 PM ]

I think we should:

  • Properly document :optimizations and throw exception on an unsupported option
  • Make :host function properly
  • Remove :serve-static as an option
  • Remove :preloaded-libs as an option
Comment by Timothy Pote [ 04/Jul/17 4:57 PM ]

Note that, as it stands, this also addresses CLJS-1502.

Comment by Timothy Pote [ 04/Jul/17 5:51 PM ]

This does what I said in my initial comment.

Note that this commit is rather small if you exclude whitespace.

Comment by Timothy Pote [ 05/Jul/17 8:43 AM ]

After thinking about it some more, I'm not sure what the gain is for being able to specify :optimizations in the browser-env, and the cost is confusion on the part of the users. I do not think it's apparent that this is a compiler option for the child iframe JS and evaluated repl forms. This may be a case where we can just "do the right thing" and remove some burden from the user.

I'm thinking we either remove :optimizations entirely or we only use it for evaluated repl forms and use :simple for the initial payload. Considering the user can already override this in the arguments to cljs.repl/repl, I lean toward removing it altogether.

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

Related CLJS-1502





[CLJS-2167] Browser REPL leaves a socket open when it fails to connect to the browser Created: 04/Jul/17  Updated: 05/Jul/17

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

Type: Defect Priority: Minor
Reporter: Timothy Pote Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: browser-repl, repl

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

 Description   

Repro steps:
0. Via an nrepl session
1. Start a browser REPL but do not connect the browser
2. Interrupt the evaluation
3. Start another browser REPL

This will result in the following exception:

java.net.BindException: Address already in use (Bind failed)

Note that, though this is easiest to trigger via an nrepl session, the underlying problem is that the socket server is not being closed in the event of an exception during initialization. You can re-create this in a plain old clojure repl by setting up monospaced}}repl/set-break-handler!{{monospaced prior to starting the browser REPL.



 Comments   
Comment by Timothy Pote [ 04/Jul/17 3:57 PM ]

There are two parts to this patch:
1. Try/Catch the repl to make sure repl-env always gets a chance to clean-up
2. Make BrowserEnv interrupt its threads

Note that this patch is mostly whitespace from re-indenting after wrapping a from in try/catch.





[CLJS-2156] Add postamble, or some other generic way to append code to a file Created: 03/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: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: closure

Approval: Vetted

 Description   

Generally useful, but specifically for appending clj.loader/set-loaded! calls to user files.



 Comments   
Comment by Thomas Heller [ 03/Jul/17 7:08 AM ]

In shadow-cljs I have 4 attributes per :module for this.

  • :prepend treated as text, eg. licence headers. Can contain JS but won't go through optimizations.
  • :prepend-js treated as JS code that will also go through closure optimizations
  • :append-js
  • :append

I think it is valuable to have all 4 and making the distinction between "text" and "JS". shadow.loader is implemented entirely through these attributes.

Comment by David Nolen [ 03/Jul/17 7:21 AM ]

I'm having second thoughts about this one, since I don't think we need this for CLJS-2157, which is why I opened it originally.

Comment by Thomas Heller [ 03/Jul/17 10:09 AM ]

set-loaded! should only be called once per module, so it must be appended to the module itself not to individual files in the module.

By appending to individual files you will eventually call it more than once for modules that have multiple entries. AFAICT the call is not idempotent and may cause events to be emitted multiple times. At the very least it may call set-loaded! before the full module has actually been loaded. Since it immediately triggers the callbacks that may lead to bad results.

The config options are generally useful not just for the loader.

Comment by David Nolen [ 03/Jul/17 10:44 AM ]

Thomas as far as I can tell ModuleManager.setLoaded is idempotent. I haven't run into any issues locally with testing. Feel free provide a failing case if you can but I couldn't find one myself.





[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-2138] Remove redundant checks in ChunkedSeq.-rest and ChunkedSeq.-next Created: 29/Jun/17  Updated: 29/Jun/17

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

Type: Defect Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: refactoring

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

 Description   

chunked-seq always returns a ChunkedSeq object and hence the nil? checks do nothing.






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