<< Back to previous view

[CLJS-914] thrown-with-msg? is unable to get message of exception Created: 19/Dec/14  Updated: 19/Dec/14

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

Type: Defect Priority: Major
Reporter: Matthew Boston Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 914.patch    

 Description   

In JavaScript, the message of an Error is retrieved with the Error.prototype.message property but the implementation of cljs.test/thrown-with-msg? uses the method .getMessage. This is causing the following error when writing tests with thrown-with-msg?.

Test console: expected: (thrown-with-msg? ArithmeticException #"Divide by zero" (\ 1 0))
Test console: actual:
Test console: #<TypeError: 'undefined' is not a function (evaluating 'e_5184auto_.getMessage()')>



 Comments   
Comment by Matthew Boston [ 19/Dec/14 5:31 PM ]

patch file for CLJS-914





[CLJS-913] Error when trying to convert javascript object to clojure (js->clj obj) under :advanced compilation Created: 18/Dec/14  Updated: 18/Dec/14

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

Type: Defect Priority: Major
Reporter: Erik Wickstrom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript 0.0-2496

Tested on OSX 10.10 with Chrome 39



 Description   

I'm trying to convert a javascript object (parsed JSON from a web service) into a clojure data structure. It works fine if I use :simple optimization with the closure compiler, however when I switch to :advanced optimization, I get the following error:

(let my-data #js {} ; see below for JSON
(.info js/console "converted to clojure" (str (js->clj my-data))))

Uncaught Error: No protocol method IEmptyableCollection.-empty defined for type object: [object Object]

Note that this also only seems to happen with this chunk of JSON and a few other samples (though according to all the JSON linters I've tried, it is valid). I've passed other input through without issue. So it is somewhat intermittent but deepish object hierarchies seem to be a commonality.

Here is the JSON (also posted here http://pastebin.com/PLffFrFf )

[{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"16 de Septiembre","short_name":"16 de Septiembre","types":["neighborhood","political"]},{"long_name":"Miguel Hidalgo","short_name":"Miguel Hidalgo","types":["sublocality_level_1","sublocality","political"]},{"long_name":"Ciudad de México","short_name":"México D.F.","types":["locality","political"]},{"long_name":"Distrito Federal","short_name":"D.F.","types":["administrative_area_level_1","political"]},{"long_name":"Mexico","short_name":"MX","types":["country","political"]}],"formatted_address":"16 de Septiembre, Miguel Hidalgo, 11810 Ciudad de México, D.F., Mexico","geometry":{"bounds":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}},"location":{"D":-99.20755880000002,"k":19.402037},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":19.4043293,"k":19.3997335},"wa":{"j":-99.21262619999999,"k":-99.2045263}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["postal_code"]},{"long_name":"West Jakarta","short_name":"West Jakarta","types":["locality","political"]},{"long_name":"Kamal","short_name":"Kamal","types":["administrative_area_level_4","political"]},{"long_name":"Kalideres","short_name":"Kalideres","types":["administrative_area_level_3","political"]},{"long_name":"West Jakarta City","short_name":"West Jakarta City","types":["administrative_area_level_2","political"]},{"long_name":"Jakarta","short_name":"Jakarta","types":["administrative_area_level_1","political"]},{"long_name":"Indonesia","short_name":"ID","types":["country","political"]}],"formatted_address":"Kamal, Kalideres, West Jakarta City, Jakarta 11810, Indonesia","geometry":{"bounds":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}},"location":{"D":106.70282500000008,"k":-6.101219},"location_type":"APPROXIMATE","viewport":{"Ea":{"j":-6.095065399999999,"k":-6.110835},"wa":{"j":106.68747699999994,"k":106.71448510000005}}},"types":["postal_code"]},{"address_components":[{"long_name":"11810","short_name":"11810","types":["route"]},{"long_name":"Příbram District","short_name":"Příbram District","types":["administrative_area_level_2","political"]},{"long_name":"Central Bohemian Region","short_name":"Central Bohemian Region","types":["administrative_area_level_1","political"]},{"long_name":"Czech Republic","short_name":"CZ","types":["country","political"]},{"long_name":"261 01","short_name":"261 01","types":["postal_code"]}],"formatted_address":"11810, 261 01, Czech Republic","geometry":{"bounds":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}},"location":{"D":13.982032200000049,"k":49.7225575},"location_type":"GEOMETRIC_CENTER","viewport":{"Ea":{"j":49.7328257,"k":49.7102303},"wa":{"j":13.979755599999976,"k":13.986990699999978}}},"types":["route"]}]

Here is the original post to the mailing list: https://groups.google.com/forum/#!topic/clojurescript/MZ9esXbn2tg






[CLJS-907] False positives from arithmetic checks Created: 12/Dec/14  Updated: 12/Dec/14

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   
WARNING: cljs.core/>=, all arguments must be numbers, got [#{nil clj-nil} number] instead. at line 59 file:/Users/kimmoko/.m2/repository/prismatic/dommy/1.0.0/dommy-1.0.0.jar!/dommy/core.cljs

Line 59 from core.cljs is the body of when-let:

        (when-let [i (utils/class-index class-name c)]
          (>= i 0))))))
(defn class-index
  "Finds the index of class in a space-delimited class-name
    only will be used when Element::classList doesn't exist"
  [class-name class]
  (loop [start-from 0]
    (let [i (.indexOf class-name class start-from)]
      (when (>= i 0)
        (if (class-match? class-name class i)
          i
          (recur (+ i (.-length class))))))))


 Comments   
Comment by David Nolen [ 12/Dec/14 10:06 AM ]

The problem is that the inferred type of class-index doesn't include number.





[CLJS-904] timeout in start-evaluator is too short Created: 10/Dec/14  Updated: 10/Dec/14

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

Type: Defect Priority: Major
Reporter: John Chijioke Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The timeout specified in cljs.browser.repl/start-evaluator is too short.
https://github.com/clojure/clojurescript/blob/master/src/cljs/clojure/browser/repl.cljs#L87

Usually I have to update it from 50 to 500 to get my repl working any time I update to a new version. Would be nice to have this factored in as I don't suspect it has any noticeable effect on any other functionality.






[CLJS-901] In memory based builds Created: 03/Dec/14  Updated: 08/Dec/14

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

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


 Description   

Currently builds are based one files on disk. It is desirable to be able to pass arbitrary ClojureScript programs as string or seq of forms and get the fully compiled source including dependencies as a string result.



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

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

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

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





[CLJS-899] AOT cache core.cljs analysis Created: 02/Dec/14  Updated: 02/Dec/14

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

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





[CLJS-898] Cache analysis results to disk Created: 02/Dec/14  Updated: 02/Dec/14

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

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





[CLJS-897] AOT core.cljs Created: 02/Dec/14  Updated: 08/Dec/14

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

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


 Comments   
Comment by David Nolen [ 08/Dec/14 4:22 PM ]

As pointed out by Colin Fleming it will be important to cache both optimized and unoptimized ClojureScript source - specifically the static-fns build option toggled.





[CLJS-896] Investigate AOTed ClojureScript JAR Created: 02/Dec/14  Updated: 02/Dec/14

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-891] Defs in "parent" namespaces clash with "child" namespaces with the same name? Created: 28/Nov/14  Updated: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Russell Dunphy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, namespace
Environment:

Clojurescript 0.0-2371, OSX



 Description   

This has had me totally flummoxed for a few hours, but I have finally been able to create a minimal project replicating the problem based on the mies template: https://github.com/rsslldnphy/cljs-ns-issue.

The problem seems to happen when a "parent" namespace, for example `my-project.foo`, contains a def with the same name as a "child" namespace, eg if there is a namespace `my-project.foo.bar`, and a def named `bar` in `my-project.foo`, and both those namespaces are required by `my-project.core`, then calling functions in `my-project.foo.bar` ends up with an Uncaught TypeError: Cannot read property 'call' of undefined. Sometimes, depending on which ns requires which, I've also seen an Uncaught Error: Namespace "cljs_ns_issue.foo.bar" already declared.

I don't think I'm doing a particularly good job of explaining this so it might be easier to look at the code. The crux is: comment out this line and the code works, leave it in and you get an error.



 Comments   
Comment by Francis Avila [ 28/Nov/14 2:01 PM ]

Clojurescript implements namespaces with Google Closure compiler's require/provide system. Unfortunately that system does not have a hard distinction between names and namespaces like Clojure does but instead is more like a sloppy java classname. The crux of it is that vars and namespaces occupy the same tree of js objects and thus their names may not overlap.

Compare the cljs on the left with the emitted javascript (This isn't exactly what is happening, but only in essence):

(ns my-project.foo ; goog.provide('my_project.foo') // something like window.my_project = {foo: {}}
  (:require my-project.foo.bar)) //goog.require('my_project.foo.bar')

;; the "require" evaluates the other namespace, which sets
;; // window.my_project.foo = {bar:{}};
;; // my_project.foo.bar.baz = "some var in the bar ns";
;; Now window.my_project.foo.bar = {baz: "some var in the bar ns"};

(defn bar [] 1)         ; my_project.foo.bar = (function bar(){return 1;});
;; Now the js object that was the bar namespace is gone, replaced with this function.

(my-project.foo.bar/baz)
; my_project.foo.bar.baz.call() // Uncaught TypeError: Cannot read property 'call' of undefined.

;; Alternatively, if (ns my-project.foo.bar) got evaluated *after* my-project.foo namespace was
;; evaluated, then my-project.foo/bar is defined, and the emitted goog.provide('my-project.foo.bar')
;; throws "Uncaught Error: Namespace "my_project.foo.bar" already declared".

So basically this is a leaky abstraction. In Clojurescript, you cannot define a var whose ns-qualified name matches that of another namespace: the slash between name and namespace is not real.

I think the only possible things that can be done are either:

  • Warn at compile-time if a var and a namespace object-path clash. Obviously there may still be runtime-created vars.
  • Put namespace vars behind some magic name. E.g. (ns foo.bar)(def baz 1) becomes goog.provide('foo.bar.__NS__'); foo.bar.__NS__.baz = 1; This would significantly uglify cljs names exported to js (and break existing code), and the magic name could never be used as a var name.
Comment by David Nolen [ 02/Dec/14 6:07 AM ]

We actually already have some logic for this in place, we track namespace segments for this reason. This is a different manifestation of it than previously encountered. I think any further workarounds are likely more trouble than they are worth (debugging complications) - probably the best thing to do is report a warning if we detect a clash.





[CLJS-876] merged sourcemap doesn't account for output-wrapper Created: 25/Oct/14  Updated: 05/Nov/14

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

Type: Defect Priority: Major
Reporter: J. David Lowe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 876.alternative.patch     Text File 876.patch    

 Description   

When merging sourcemaps, the cljs.closure/optimize code correctly accounts for "preamble" lines that get prepended to closure compiler output. However, it fails to account for the extra lines added when :output-wrapper is true. As a result, the merged sourcemap is incorrect whenever :output-wrapper is true. (And for extra action-at-a-distance: :output-wrapper is implicitly set to true by lein-cljsbuild whenever :optimizations is :advanced.)

Too tired to make a patch tonight, but I'll go through the CA process and send a patch in a day or two, if nobody gets to it before then.



 Comments   
Comment by J. David Lowe [ 27/Oct/14 2:42 PM ]

Here are two patches, which both fix the problem.

876.patch is tiny, but I think it's probably a step in the wrong direction, because all future output prefixes will need the same fix applied to them.

876.alternative.patch is a deeper change, but I think is a better solution: it makes it much more apparent that all output prefixes should be added via make-preamble. The downside (IMO) is that the output-wrapper prefix and suffix are now separated a bit in the code.

Comment by David Nolen [ 05/Nov/14 6:46 AM ]

This patch approach looks OK but something more minimal is preferred to simplify patch reviewing. Please submit a patch which makes the most minimal set of code changes (it's preferred not to move code) use declare if necessary. Thanks!





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

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

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

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





[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 03/Oct/14

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   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.






[CLJS-867] specify with Object methods requires multi-arity style definition Created: 02/Oct/14  Updated: 02/Oct/14

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   
Object
(foo [] ...)

Throws an `nth not supported on this type: Symbol` exception.

Object
(foo ([] ...))

Is required even though this is not true for protocol implementations.






[CLJS-865] js-dependency-index won't work when running from an uberjar Created: 26/Sep/14  Updated: 03/Dec/14

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

Type: Defect Priority: Major
Reporter: Dan Johansson Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

Window7
[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2322"]
java 1.7



 Description   

I'm having problems running an uberjar with [lein-light-nrepl "0.0.18"].
I've tracked the problem down to the filter in cljs.js-deps/goog-resource.
It expects to find goog/deps.js in the google jar file but it is in the uberjar file.



 Comments   
Comment by Dan Johansson [ 29/Sep/14 2:27 AM ]

I use this workaround for now:

(defn load-nrepl []
  (require 'cljs.js-deps)
  
  (alter-var-root (find-var 'cljs.js-deps/goog-resource)
                  (fn [of]
                    (fn [path]
                      (first
                       (enumeration-seq (.getResources (.getContextClassLoader (Thread/currentThread)) path))))))
  
  (require 'lighttable.nrepl.handler))

It just removes the filter.
I guess the filter is there to make sure you get the correct goog/deps.js file? I don't know what assumptions to make about build environments to suggest a generic solution.

Comment by Dan Johansson [ 29/Sep/14 2:28 AM ]

This is the error I get:

Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
	at clojure.core$_cache_protocol_fn.invoke(core_deftype.clj:544)
	at clojure.java.io$fn__8628$G__8610__8635.invoke(io.clj:69)
	at clojure.java.io$reader.doInvoke(io.clj:102)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.js_deps$goog_dependencies_STAR_.invoke(js_deps.clj:241)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invoke(core.clj:624)
	at clojure.core$memoize$fn__5097.doInvoke(core.clj:5846)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at cljs.js_deps$js_dependency_index.invoke(js_deps.clj:265)
	at cljs.env$default_compiler_env.invoke(env.clj:45)
	at cljs.env$default_compiler_env.invoke(env.clj:42)
	at lighttable.nrepl.cljs__init.load(Unknown Source)
	at lighttable.nrepl.cljs__init.<clinit>(Unknown Source)
	... 52 more
Comment by James Cash [ 29/Sep/14 3:58 PM ]

I was able to work around this same issue by explicitly including the google-closure-library jar in the classpath (i.e. instead of `java -jar my-uber.jar`, `java -cp google-closure-library-$VERSION.jar:my-uber.jar my-uber.core`)

Comment by Taha Siddiqi [ 08/Oct/14 10:19 PM ]

This is what worked for me.

(defn goog-resource [original-fn path]
  (if-let [resource (io/resource path)]
    resource
    (original-fn path)))

(alter-var-root #'cljs.js-deps/goog-resource (fn [f] (partial goog-resource f)))
Comment by David Nolen [ 09/Oct/14 3:35 PM ]

I'd be happy to remove the hack for Google Closure if someone can demonstrate it's no longer needed. If I recall we now remove the empty files from the thirdy party JAR so this shouldn't be a problem anymore.

Comment by Joseph Moore [ 03/Dec/14 11:43 PM ]

Adding the google-closure library jar to my war file in immutant (from lein immutant war) fixed lighttable's nrepl under wildfly as well, thank you James





[CLJS-860] redundant analysis of source files in JARs Created: 18/Sep/14  Updated: 18/Sep/14

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   

Source files in JARs are analyzed once then analyzed again when copied to the output directory.






[CLJS-851] simplify :none script inclusion if :main supplied Created: 05/Sep/14  Updated: 03/Dec/14

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

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

Attachments: Text File 0001-Emit-require-for-main-namespace-do-deps-file.patch     Text File 0002-Emit-goog.base-configuration-constants.patch     Text File 0003-Emit-goog.base-into-deps-file.patch     Text File 0004-Emit-all-deps-into-a-single-file.patch     Text File 0005-Update-samples-to-new-none-opt-usage.patch    
Patch: Code

 Description   

If :main namespace supplied - under :none optimization settings :output-to file should goog.require it. This would also allow script inclusion to be unified regardless of the level of optimization supplied, i.e.:

<script type="text/javascript" src="foo.js"></script>

Instead of

<script type="text/javascript" src="out/goog/base.js"></script>
<script type="text/javascript" src="foo.js"></script>
<script type="text/javascript">goog.require("foo.core");</script>


 Comments   
Comment by David Nolen [ 05/Sep/14 5:10 PM ]

This does mean concatenating the contents of goog.base into the deps file.

Comment by Thomas Heller [ 06/Sep/14 8:31 AM ]

Just a quick Note: Not sure what your plan is for :main but it in my experience it is important to make it a collection. A larger app quickly gains multiple entry points.

Comment by David Nolen [ 06/Sep/14 9:38 AM ]

I don't see the point of making it a collection, users can simply load other entry points themselves via the "main" namespace.

Comment by Andrew Rosa [ 14/Oct/14 5:28 PM ]

Is someone already working on this issue? I will familiarize with the compiler code to try tackle this feature.

Comment by Andrew Rosa [ 15/Oct/14 10:17 PM ]

@dnolen I made the changes to compiler for auto require an specified :main namespace (but still optional). After that I started to experiment adding the goog.base code directly into the generated js, just to see what happens.

What I found is that we will need to disable the Closure default deps.js file. So we need to copy the goog.base code and after that include all dependencies, including the Closure ones. This work is need to avoid two issues:

  • We need to explicit set the base path for loading, since the auto-discovery method used by Closure searches for a <script str="base.js">.
  • By default, Closure automatically includes a <script> containing the goog/deps.js file to include itself. This causes race conditions during our require('main-ns'), since only half of our dependencies was added to dependency graph. Explicitly requiring everything will avoid this bug.

I could change the compiler to accommodate all this changes, but first like to communicate you the deep effects on our current behavior. What do you think?

Comment by Thomas Heller [ 16/Oct/14 2:24 AM ]

I have implemented this feature in shadow-build [1] and use the following strategy:

Assuming our output file is named "app.js" for an optimized build, it will look basically like this when compiling with :none.

1) imul.js fix
2) closure global settings

I found that these are required

var CLOSURE_NO_DEPS = true;
var CLOSURE_BASE_PATH = '/assets/js/src/';

The CLOSURE_BASE_PATH is required for the include scripts to find the javascripts for each namespace.

3) After the settings the goog/base.js is emitted into the file.
4) After that the contents of dependency settings are emitted (basically goog/deps.js) plus all of our namespaces. They look something like this

goog.addDependency("clojure/set.js",['clojure.set'],['cljs.core']);

When closure attempts to load a given namespace it will simply emit a <script src="CLOSURE_BASE_PATH + path/to/ns.js">, I found that using an absolute path is the best way to ensure that files are loaded correctly since the "path detection" feature of the goog/base.js file fails since we do not include a goog/base.js

5) Then finally you can just add the

goog.require('main-ns');

Hope that helps.

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

Comment by Andrew Rosa [ 16/Oct/14 7:35 AM ]

Thank you Thomas for your very nice writeup.

In my tests I found that we could leave CLOSURE_BASE_PATH alone IF we made everything relative to our output file. Do you think forcing it to some path is more reliable?

Comment by Thomas Heller [ 16/Oct/14 1:08 PM ]

Yes definitly. The location of the javascript files usually is "fixed" once the app is configured. Either you put it locally at something like "/js" or something like a CDN, so you configure it once and forget about it. But thats my own personal opinion, I have no idea how other people are handling it. I had some problems with relative paths but can't remember exactly what they were only that CLOSURE_BASE_PATH fixed it.

Comment by Andrew Rosa [ 24/Oct/14 4:52 PM ]

Just to give an update here: I already have a patch for this issue, but depends on the resolution of CLJS-874.

Comment by Andrew Rosa [ 08/Nov/14 7:59 PM ]

Here are the patches to fix this issue. I made smaller patches for each "step" to light the burden of code review.

  • 0001: this issue creates a new compiler option :main, which receives a symbol with the "main namespace" similar to Clojure. If supplied, will emit a require at the end of our custom deps file.
  • 0002: here I put the Closure configuration constants to make the loading works. I set the goog/base.js dir as a relative path so the change will be less intrusive to cljs compiler.
  • 0003: Emit whole goog.base
  • 0004: combine all deps into our single file, to avoid async dependency loading between google closure libs and our deps.
  • 0005: Update all sample documentation and remove the old "*-dev.html" files.

Thomas: thank you for the help and the examples. In the end I removed the need to know absolute paths by just considering we can aways reach the goog/base directory. This leads to less code involved to make the feature available.

Please let me know if there is any problem with those patches, so I could fix them. This is my first contribution to Clojure(Script), so I still need to grasp the workflow here.

Comment by Martin Klepsch [ 27/Nov/14 10:55 AM ]

@Andrew, I just applied your patches and got the Twitterbuzz example running.
You've done great work, lot's of people will appreciate this change.

One issue I encountered doing that:

In the Twitterbuzz Readme there's a statement as follows:

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "samples/twitterbuzz/out"})
[...]
The reason we set the `:output-dir` is because the `index.html` script tag is specifically pointing to that directory.

When compiling with these options index.html tries to load deps from "[...]clojurescript/samples/twitterbuzz/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js" which is not correct.

After that I just compile with

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "out"})

This places an "out" dir into the clojurescript project directory. Now, when opening index.html, twitterbuzz.js looks for dependencies like this: "[...]clojurescript/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js". This correctly loads the earlier compiled js and the application works. I'm not sure how this is supposed to work as I've mostly just used lein-cljsbuild to date but you probably have an idea

Some minor things

  • The referenced patch CLJS-874 doesn't apply cleanly anymore. I'm too unfamiliar working with patches to help with this unfortunately.
  • Is there any reason to specify :main in advanced compilation mode? (It has been added to the advanced mode section in the twitterbuzz example.)

Really looking forward to see this merged!

Comment by Andrew Rosa [ 29/Nov/14 9:28 PM ]

Thanks @Martin for testing the patches, I kindly appreciate that.

I need to take some time to investigate the issue with this specific example. I'm not confident with the behavior of some examples, like the hello-js one which looks like broken (even on master). Probably I will provide a patch to review and possibly update the examples in another ticket, so we can have a more understandable environment to test this patches.

About the patch CLJS-874, it was made prior the last rework I've made here and probably will not be needed anymore. I need to confess that, if I knew at the time how to do multiple patches I neither need to created it in the first place.

At the last, the :main option isn't really used in advanced compilation. I just put it there to have some sort of "consistency" across all the examples - the only thing that change between them will be the optimization level. Do you think that this generates more confusion than good?

Comment by Mike Thompson [ 30/Nov/14 6:47 PM ]

I love this idea. Ironing out these sorts of stumbling blocks is so important to achieving wider cljs adoption. When I was starting with CLJS at the beginning of the year, I accumulated a healthy set of paper cuts from this sort of stuff. WAY too much time was wasted.

But ... two things:

1. I'd be concerned that the concatenation of base.js and deps.js might slow things down. The lovely thing about `:none` currently is that it delivers sub-second compile times. That's so important to a good productive workflow. But perhaps I'm being paranoid and adding in a base.js pre-pend won't add much time.

2. You really don't need to know what :main is. You can take a sledgehammer to the process by appending the following to the generated file:

```
for(var namespace in goog.dependencies_.nameToPath)
goog.require(namespace);
```

This requires everything in. Everything. (Which is the same outcome as using :whitespace or :simple) And that turns out to be essential in a unit-testing scenario where there isn't a single root namespace. Further explanation:

https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L110-L111

Aside: I worry that this might not be backwards compatible and that it should be achieved by introducing a new `:optimization` target of `:debug` (and deprecating :none ??). Just a thought.

Comment by Thomas Heller [ 01/Dec/14 7:51 AM ]

@Mike Don't worry, prepending base+deps.js is not noticeable performance wise (less than a millisecond).

I tried defining what a "main" is for quite some time in shadow-build but the best I came up with: It's a namespace that contains functions called from outside your CLJS Code (eg. via HTML, aka API functions). It is a very valueable thing to have since its the only way to tell the CLJS compiler (and runtime) which namespaces are actually required. If you have a "main" namespace its deps (and the deps of the deps, ...) can be identified and namespaces that aren't actually needed can be skipped. The Closure dead-code elimination does this for :advanced as well, we just need a little help for :none.

IMHO every app should have at least one "main" ns, requiring everything is a hack not a solution.

As for your clojurescript.test example, that is a shortcoming of the tools.

I'm happy to provide help with shadow-build if anyone is interested, should be pretty simple to add a :test target. To be honest adding more features/options to cljs.closure/build (which lein-cljsbuild uses) is a recipe for disaster as it is already way too complex.

Comment by Andrew Rosa [ 01/Dec/14 10:01 AM ]

I'm also against introducing new options. I really want is eventually deprecate some of the unneeded ones.

I think that the :main namespace is clean, and could avoid issues like having some code evaluated in a namespace that is removed by Closure compiler in production. Not sure how common that could be in ClojureScript, but a valid buggy scenario in terms of global mutable state.

Anyway, if we follow the alternative of requiring everything, I think that could be done without dabbling with goog.base internals. We already have all app namespaces to be able to emit the deps.js

@Thomas your experience with shadow-build has shown you any need of "multiple entry-points" or only a single :main ns is enough? I have a feeling that the need to include all test namespaces is somewhat necessary because how clojurescript.test works, but I'm not familiar how the Clojure version works.

Comment by Thomas Heller [ 01/Dec/14 12:50 PM ]

@Andrew: See my first comment on this issue, in my experience an app will have more than one entry point. "clojurescript.test" would be one example why.

Comment by Martin Klepsch [ 01/Dec/14 3:14 PM ]

@thomas could you further explain the behaviour you'd expect from passing multiple namespaces to :main?

With my current understanding I don't see a need for multiple arguments to the :main option as Andre proposed it.
If you want a build that has another entry point (which also loads tests or similar) why not just make it a different build?

Comment by Thomas Heller [ 01/Dec/14 4:10 PM ]

It's always rather difficult to explain something without a good real world example, but explaining what I do in my app is too app specific. Anyways, my web app has a couple of different features. Lots of different pages and some use one or more features provided by CLJS code. It is not a single-page Application, rather an "old" web app with just some dynamic components sprinkled in.

Let me try an example:

(ns my-app.feature1)
(defn ^:export fancy-component [])

(ns my-app.feature2)
(defn ^:export fancy-component [])

(ns my-app.feature3)
(defn ^:export fancy-component [])

These would all be "main" namespaces by my definition.

I do not want a (ns my-app (:require [my-app [feature1 feature2 feature3]])) since that would couple all components together. One feature provided by shadow-build is that it is able to "split" the produced javascript into multiple files (modules) so you may load them optionally. So I can even split each feature into a seperate .js file loadable on demand, AFTER advanced compilation that is.

I do this in my web app and instead of feeding everyone a hefty 800kb javascript file, everyone gets 90kb and loads the rest when needed. This greatly improved page load times.

The problem with multiple builds is that EVERY build will contain cljs.core, which is quite heavy. You'd end up with

build1: cljs.core, my-app.feature1
build2: cljs.core, my-app.feature2
build3: cljs.core, my-app.feature3

instead of

module1: cljs.core (+ code shared by feature1,2,3)
module2: my-app.feature1
module3: my-app.feature2
module4: my-app.feature3

If you have experience with jQuery, multiple builds would be the equivalent to including jQuery with every jQuery plugin. But I'm getting off topic, thats about modules not about :main.

Hope I made some sense in regards to multiple mains, its just something I use alot but for me it is coupled with modules so YMMV.

Comment by Mike Thompson [ 01/Dec/14 4:15 PM ]

First, I'm a bit surprised. I thought there would be joy at the thought of NOT having to nominate a :main.

At its core, CLJS-851 appears to be about harmonizing `:none` with other options. The initial comment above explains how the HTML files for :none have to be different from the other options :simple etc. We don't like that. BUT the proposal to add :main into project.clj seems set to reintroduce a difference again between :none and the other options, but just in a different place. And there seems to be no need for that difference. :simple and :whitespace bring in all the code. So does :optimized except it is first tree-shaken down by Google Closure. So these 3 options do not need a main. Why should :none have a :main when the other options don't. I can see no downside to pulling in everything and making :none harmonious with the other.

@Andrew I agree that we don't need to use the internals of goog to require everything in. The necessary list of namespaces to require are present at the time of writing deps, so a list of goog.requires could be added, one for each namespace compiled.

Second, @Thomas could you explain more why you think cemerick's framework is wrong to be rootless? In my experience when unittesting you actively do not want a root. You want a directory full of test clj files, with that set of files growing and shrinking over time. Different members of the team might be adding and removing them at each step. You want your tests to be "discovered" by your "test runner" and then executed. I don't want to have to be manually maintaining a central list of tests. In my experience, tools like "Nose" under Python come with a "runner" which discovers all the tests by walking the directory of your project, but that approach isn't an option in the CLJS world. Instead, the compiler has to scoop up all the tests from the directory, compile them and then they have to be "required in", ONLY at that point can the tests be "discovered" in a CLJS world. At least that's my impression.

Proposal: if `:main` isn't there, then bring it all it as per my suggestion. If :main is there then only it gets required.

Comment by Thomas Heller [ 01/Dec/14 4:49 PM ]

@Mike: Without getting too much into the specifics of shadow-build: The feature of discovering tests and running them for you would be trivial, even without making a single change in shadow-build itself.

"... that approach isn't an option in the CLJS world" is simply not true. It is true for cljs.closure/build since its a function combining multiple very complex tasks into one simple map of options. lein-cljsbuild thereby suffers the same fate.

I haven't done much work documenting or promoting shadow-build and to be honest I'd much rather see tools like lein-cljsbuild built on top of shadow-build rather than trying to turn it into a lein plugin itself. Quite frankly to me it is perfect the way it is. I can step into every stage of the CLJS compilation/optimization and do stuff other tools simply cannot do, but that comes at the price of not being very beginner friendly. Documentation might help but I'm really bad at writing that stuff.

Anyways, I would inclue :main for EVERY build, ESPECIALLY :advanced!

I have a rather large codebase (way over 10k LOC CLJS) with several distinct builds. If I had to compile every CLJS file for every build everything would build forever. Some smaller builds just require a couple files (say 10), why feed 300+ into the Closure Compiler to eliminate what a simple :main could have done (and even skipped the CLJS->JS compilation).

I have this stuff in production for about 2 years now, but with custom tools for about as long. I cannot speak for the lein-cljsbuild world whether :main would be a good idea there or not. I don't even care since shadow-build is not affected by this change at all, but given my experience with my own app/codebase I would recommend making :main accept a collection of namespaces.

Comment by Mike Thompson [ 01/Dec/14 5:29 PM ]

@Thomas I'm looking at this through the cljsbuild point of view. I appreciate that shadow build might be able to do test discovery, but unfortunately I don't have the option of using it. So I'm explaining this from the perspective of a cljsbuild user, who is seeking a better outcome.

Comment by Andrew Rosa [ 01/Dec/14 7:33 PM ]

Nice to see this conversation going forward @Mike and @Thomas.

Anyone here can confirm that the advanced compilation behaves like a concatenation of all user defined namespaces? If so, we could change our strategy to mimic the behavior on :none - consistency is a gold IMO, and I like the idea of transparency in optimization levels. Thanks @Mike to point it out.

@Thomas, I guess that the way you suggest to modularize the builds is not possible even for :advanced compilations. I prefer to have a separate issue to discuss and address that feature (which looks interesting for use cases like you presented). And sorry to make you answer the same question again.

The conditional :main is worth the cost of added complexity? How it will affect :advanced compilation?

Comment by Thomas Heller [ 02/Dec/14 4:39 AM ]

Advanced Optimization does not behave like a concatenation, Closure handles the optimization like this (greatly simplified):

1) You provide a list of inputs (Javascript files) and externs
2) Closure parses these into AST inputs
3) basic analysis to extract goog.provide/require information
4) build dependency graph, topological sort
5) magic (if :advanced)
6) generate output

shadow-build does something similar

1) scan all source paths for "resources" (.js, .cljs)
2) extract goog.provide/require from .js
3) read first form of .cljs files (expects it to be 'ns), get provide/require from that
4) build dependency graph, topological sort
5) compile CLJS->JS (if reachable by any :main)
6) feed into Closure Compiler
7) output

Basically cljs.clojure/build does the same, except for #3 which results in CLJS compiling in a somewhat random order (eg. it compiles one 'ns and all its deps until everything is compiled). Without a :main everything will be compiled even if not used anywhere in your project. Closure Advanced Compilation can still remove dead-code since it will analyze the AST and realize that parts (or everything) of a given namespace are never used, :none can not do that without a :main.

If you have 300 source files (CLJS+JS) the load time of a web page (in dev) will be rather high (we are talking seconds here, remember worrying about a couple ms), especially with open dev tools since that will also load an additional 300 source maps. Loading files you don't need therefore is a huge waste of time especially if you reload (or even phantomjs clojurescript.test) often. I cannot stress enough how important :main is for this stuff, but it largely depends on the size of your codebase and the structure of your app. Small example: the Admin "App" of my application will do 714 requests on page load (JS+source maps) and transfer 3.3mb, page load is 3,54s on my rather beefy MacPro (in production this cuts down 13 requests, 343kb and page load in 700ms). Without the :main every page in my app would do 714 requests (or even more) on page load. Not a great development experience. Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads? REPL might provide a different experience but I found it impossible to properly build/test complex input forms when using a REPL, much rather edit/reload.

@Andrew: Agreed that the module discussion has no place here, just wanted to point out that the :main option becomes mandatory then. To be honest this whole discussion only stresses the problem of cljs.closure/build: trying to combine everyone's expectation of how a build should look into a single function is tough. So in the long run something that lets you split these parts will be easier (hint: shadow-build) but the good part is that ClojureScript itself does not even need to deal with all this stuff. A "simple" CLJS->JS compiler would do, the Closure bits are optional after all (even more so for non-browser targeted builds). But discussing what should be in CLJS and what shouldn't is another issue as well.

Comment by Martin Klepsch [ 02/Dec/14 1:21 PM ]

Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads?

No. Figwheel only reloads changed files. So the slowdown should be a lot smaller once the page is loaded.

Besides that: @thomas thanks for all your very thoughtful messages here, just reading them already taught me a ton about Clojurescript compilation!

Comment by Andrew Rosa [ 02/Dec/14 6:22 PM ]

Thank you @Thomas for all this explanation, quite insightful. You convinced me that the way to go is requiring :main anyway. If we fell the necessity, we could add the "require all" behavior later. I also like the idea of having a dependency graph to compile only required stuff, but is something for another discussion

Just a final tweak for us to think about: the :main option could receive a list of namespaces or just a single one? Conceptually, I biased for the later. A single namespace could allow us to eventually add something like a -main function, useful for node apps for example - just an idea, of course.

I just updated the patches to fix a single issue with :main not being mungled before require, which caused problems with namespaces including dashes (like foo-bar)

Comment by Mike Thompson [ 02/Dec/14 6:35 PM ]

Okay, my final observations:

  • naming
    Given :main is to be used to indicate a namespace, should its name be changed to make that clearer?
    After all this is about automating: 'goog.require(something)' but the name :main seems unrelated.
    To me :main carries connotations of main functions, not root namespaces.
    I would suggest that ":auto-require" would be a better name and I believe names (cumulatively) matter.
  • Special Marker
    No matter the name chosen, to handle my unittest use-case, would it be possible to have a
    special marker value for :main like, say, "*" which triggers the require everything approach.
    As I've indicated, that would be a big win from where I'm standing.

Thanks to everyone.

Comment by Andrew Rosa [ 02/Dec/14 6:41 PM ]

Good call @Mike. I guess :main was chosen to provide some "parity" with Clojure - that's why I suggest the auto-calling -main feature.

For the unit test case, do you think that it could be done as one enhancement on clojurescript.test? It seems like the test runner should be capable of finding the right namespaces... not sure if it does not do that for limitations of the platform.

Comment by Thomas Heller [ 02/Dec/14 7:16 PM ]

Closure calls them Entry Points, I got used to :main.

Can't really say much about clojurescript.test, only that testing generally is rather complex hiding this behind a "require all" marker will probably not be enough. I know I don't always want to run every single test, usually only those affected by the file I just changed is enough. But getting off-topic again, proper Tooling is hard.

Again for :main, my advice would be a collection but just offer a fallback and turn a single symbol into a collection of one. That covers all cases 0,1,more ... No :main just does not emit any requires which is what we have now.

:main my-app.feature1
:main [my-app.feature1]
:main [my-app.feature1 my-app.feature2]
Comment by Andrew Rosa [ 02/Dec/14 7:27 PM ]

@Thomas no defined :main will still will include base.js and deps.js, but not requiring anything. Right? At least is that what I would expect.

Yeah, tooling is very hard.

Comment by Thomas Heller [ 03/Dec/14 4:20 AM ]

No :main might as well behave like it does now, this way the feature is completely optional and will not break any existing code. Thats usually worth something, don't know how much because as I said, :main is mandatory in shadow-build.

ClojureScript itself should probably be more conservative than that. I imagine there are alot of projects that still use /out/goog/base.js + other in their HTML, that won't change over night.





[CLJS-836] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 13/Dec/14

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

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

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

 Description   

See http://dev.clojure.org/jira/browse/CLJ-1499



 Comments   
Comment by Peter Schuck [ 12/Dec/14 4:56 PM ]

This is a port of the patches / diffs at http://dev.clojure.org/jira/browse/CLJ-1499. I'm getting around a 2X perf improvement when running the benchmark quite.

Comment by David Nolen [ 13/Dec/14 8:43 AM ]

Thanks, looks great! Will check with Alex Miller about the status of CLJ-1499 and see if we can apply this one.





[CLJS-833] A named fn shadows `js/fn-name` Created: 04/Aug/14  Updated: 04/Aug/14

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

Type: Defect Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-833-Test-for-fn-name-shadowing.patch    

 Description   

Description

The function

(fn console [] js/console)

will return a reference to itself when called.
This happens because the function is transpiled to

(function console(){return console;})

Solution proposals

Mangle internal function names like let bindings

The internal name of a generated js function should be treated like a let binding, hence gensymed.
Thus the function would be transpiled to something like

(function console_1337(){return console;})

References

Brought up in https://groups.google.com/d/msg/clojure/QZmGrjNVurs/NxFtq8yDCFIJ



 Comments   
Comment by Herwig Hochleitner [ 04/Aug/14 4:10 AM ]

Attached test uses js/eval, because that's one of the shortest global identifiers, that should be available on every js runtime.

Comment by Herwig Hochleitner [ 04/Aug/14 12:43 PM ]

As detailed in the ML thread, CLJS-680 introduced :js-globals in an attempt to fix this issue. :js-globals should be removed along with a proper fix.





[CLJS-818] Externs don't get loaded when running under immutant as cljs.js-deps/find-js-classpath fails Created: 18/Jun/14  Updated: 01/Jul/14

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

Type: Defect Priority: Major
Reporter: James Cash Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Java 1.8.0_05, Clojure 1.6.0, clojurescript 0.0-2234, immutant 1.1.1


Attachments: Text File 818.patch     PNG File Screen Shot 2014-06-18 at 20.14.59 .PNG    

 Description   

When compiling clojurescript that relies on library-provided externs (e.g. Om needing React.js externs), the clojurescript is compiled without errors, but the generated javascript fails to work, due to the externs not being loaded. Externs don't get loaded, as cljs.js-deps/find-js-classpath doesn't find the javascript externs file. This occurs because it uses cljs.js-deps/all-classpath-urls, which filters out the immutant classloader, since org.immutant.core.ImmutantClassLoader is not an instance of java.net.URLClassLoader (and hence lacks a .getURLs method anyway).



 Comments   
Comment by Toby Crawley [ 19/Jun/14 9:23 AM ]

Chas: Is there a reason not to depend on dynapath here? This exact case is kinda why it exists

Comment by David Nolen [ 19/Jun/14 10:47 AM ]

Patch welcome for this.

Comment by James Cash [ 19/Jun/14 2:12 PM ]

Simply replacing cljs.js-deps/all-classpath-urls with dynapath.util/all-classpath-urls worked for me. I don't know if there are policies around adding dependencies to cljs, but the following patch is working for me. Would it be preferable to re-implement the functionality instead?

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

We are not going to take on a dependency for this. The code should be copied over, thanks.

Comment by James Cash [ 19/Jun/14 3:46 PM ]

Due to the way dynapath works, I don't think a straightforward copying of the code will work, since it relies on a protocol. Backing up a step though, would it be reasonable for externs to be loaded via io/resource, in the same way that the :preamble is?

Comment by Toby Crawley [ 19/Jun/14 3:54 PM ]

Unfortunately, the code can't be copied over. Dynapath works by providing a protocol that providers/users of funky classloaders can implement, allowing libraries that use dynapath to access the dynamic features of those classloaders without having to care about the loader's concrete type. Dynapath itself provides implementations for j.n.URLClassLoader and c.l.DynamicClassloader by default, so libraries don't have to do anything special to access the dynamic features of those classes.

java.classpath also provides a similar mechanism that the Immutant classloader implements as well. If you are more open to dependencies that are under org.clojure, using that will work as well. Ideally, I'd like to see java.classpath subsume dynapath.

Comment by James Cash [ 19/Jun/14 4:23 PM ]

Made a new patch that sidesteps the all-classpath-urls issue by just using io/resource instead of iterating over all urls

Comment by David Nolen [ 01/Jul/14 9:26 PM ]

Can people chime in whether the patch works for them, thanks.





[CLJS-813] Warn about reserved JS keyword usage in namespace names Created: 11/Jun/14  Updated: 16/Jul/14

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   

If a namespace is identified as foo.long.core it will get munged into foo.long$.core. This is unexpected and a source of confusion when goog.require("foo.long.core") fails.



 Comments   
Comment by Max Veytsman [ 15/Jul/14 4:50 PM ]

I'm starting to take a look at this.

Would it be most appropriate to add this check into compiler.cljs where the munging actually happens, or into analyzer.cljs where most of the warnings of this type live?

Comment by Mike Fikes [ 15/Jul/14 5:34 PM ]

If a solution is identified that eliminates “overly aggressive” munging for certain cases, then CLJS-689 could benefit.

Comment by Max Veytsman [ 16/Jul/14 2:44 PM ]

Currently, when munging "foo.bar.baz", we map over ["foo", "bar", "baz"] and check if each is a reserved word (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L94-L95)

According to my understanding of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Reserved_word_usage and https://es5.github.io/#x7.6 reserved words are only disallowed in Identifiers. MemberExpressions and CallExpressions (the things with "."s in them) do not ban reserved words except in the first Identifier.

For our purposes, it could be enough to check if the entire token and the first period-seperated element is reserved. I.e. long becomes long$, long.night.ahead becomes long$.night.ahead, but foo.long.core remains foo.long.core.

Mike, this unfortunately won't affect CLJS-689

Does that sound like a good approach?





[CLJS-808] Warning from `find-classpath-lib` mistakenly included in generated source Created: 02/Jun/14  Updated: 02/Jun/14

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

Type: Defect Priority: Major
Reporter: Joshua Ballanco Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojurescript 0.0-r2227
target: node


Attachments: Text File warn-with-out-str-fix.patch    
Patch: Code

 Description   

When compiling for node.js, `find-classpath-lib` generates a warning because `cljs/nodejs.js` doesn't contain a `goog.provide` declaration. However, this warning ends up being emitted into the JS files being generated (caused by a call to `with-out-str`), later causing a critical failure in the Closure compiler.

To reproduce:

  • `lein new cljs-node buggy`
  • Update clojurescript to r2227, lein-cljsbuild to 1.0.3
  • `lein cljsbuild once`

Result:

  • Multiple warnings
  • Generated "buggy.js" file contains only node.js shebang

Fix:

  • The simplest patch (attached) is to rebind `out` to `err` when emitting a warning
  • A better fix would be to write a function for error printing that wraps this idiom
  • Ideally, cljs/nodejs.js should have a `goog.provides` line added to prevent the warning in the first place


 Comments   
Comment by David Nolen [ 02/Jun/14 7:45 AM ]

A patch that adds a goog.provide to the file is preferred, thanks!





[CLJS-806] support ^:const Created: 09/May/14  Updated: 17/Dec/14

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

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

 Description   

Currently def's do not support ^:const annotations, this is useful in conjunction with case.



 Comments   
Comment by Peter Schuck [ 17/Dec/14 4:53 PM ]

def's marked with ^:const are now treated as constants, they can't be redefined or set to a different value. Currently only case takes advantage of this. Additionally the emitted var is annotated as a constant for the closure compiler.





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

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

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

r2173



 Description   

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

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



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

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

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

Bump, this enhancements sound simple & fine.

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

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

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





[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 02/Dec/14

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

Type: Defect Priority: Major
Reporter: Kevin Marolt Assignee: David Nolen
Resolution: Unresolved Votes: 0
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-713] optimized case Created: 04/Dec/13  Updated: 23/Jun/14

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

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

Attachments: Text File 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch     Text File 0001-CLJS-713-first-cut-at-compiling-case-to-switch.patch    

 Description   

With the advent of asm.js many engines will like compile switch statements over integers into jump tables. We should provide a real `case*` ast node that compiles to JS `switch` when possible - i.e. numbers, strings, keywords etc.



 Comments   
Comment by Michał Marczyk [ 18/Feb/14 5:56 PM ]

First cut impl also available here:

https://github.com/michalmarczyk/clojurescript/tree/713-compile-case-to-switch

With this patch applied, case expressions are compiled to switch + some extra bits when all tests are numbers or strings, otherwise old logic is used.

For example, {{(fn [] (let [x 1] (case x 1 :foo (2 3) :bar :quux)))}} gets compiled to

function () {
    var x = 1;
    var G__6469 = x;
    var caseval__6470;
    switch (G__6469) {
      case 1:
        caseval__6470 = new cljs.core.Keyword(null, "foo", "foo", 1014005816);
        break;
      case 2:
      case 3:
        caseval__6470 = new cljs.core.Keyword(null, "bar", "bar", 1014001541);
        break;
      default:
        caseval__6470 = new cljs.core.Keyword(null, "quux", "quux", 1017386809);
    }
    return caseval__6470;
}

The existing test suite passes, but I suppose it wouldn't hurt to add some tests with case in all possible contexts.

Comment by Michał Marczyk [ 18/Feb/14 6:05 PM ]

As a next step, I was planning to arrange things so that numbers/strings are fished out from among the tests and compiled to switch always, with any leftover tests put in an old-style nested-ifs-based case under default:. Does this sound good?

It seems to me that to deal with symbols and keywords in a similar manner we'd have to do one of two things:

1. check symbol? and keyword? at the top, then compile separate switches (the one for keywords would extract the name from the given keyword and use strings in the switch);

2. use hashes for dispatch.

Which one sounds better? Or is there a third way?

Comment by Michał Marczyk [ 18/Feb/14 6:11 PM ]

Of course we'd need to compute hashes statically to go with 2. I'd kind of like it if it were impossible (randomized seed / universal hashing), but currently it isn't.

Comment by Francis Avila [ 19/Feb/14 12:22 AM ]

At least on v8, there are surprisingly few cases where a switch statement will be optimized to a jump table. Basically the type of the switched-over value must always (across calls) match the type of every case, and there must be fewer than 128 cases, and integer cases must be 31-bit ints (v8's smi type). So mixing string and number cases in the same switch guarantees the statement will never be compiled. In many cases an equivalent if-else will end up being significantly faster on v8 just because the optimizing jit recognizes them better. There's an oldish bug filed against v8 switch performance. Looking at the many jsperfs of switch statements, it doesn't seem that v8 has improved. Relevant jsperf

Firefox is much better at optimizing switch statements (maybe because of their asm.js/emscripten work) but I don't know what conditions trigger (de)optimization.

I suspect the best approach is probably going to be your option one: if-else dispatch on type if any case is not a number, and then a switch statement covering the values for each of the keyword/string/symbol types present (no nested switch statements, and outlining the nested switches might be necessary). Even with a good hash, to guarantee v8 optimizing-compilation you would need to truncate the hashes into an smi (signed-left-shift once?) inside the case*.

Comment by David Nolen [ 19/Feb/14 12:50 AM ]

There's no need for invention here. We should follow the strategy that Clojure adopts - compile time hash calculation.

Comment by Francis Avila [ 19/Feb/14 3:09 PM ]

The problem, as Michal alluded to, is that the hash functions in cljs's runtime environment are not available at compile-time (unlike in Clojure). This might be a good opportunity to clean up that situation or even use identical hash values across Clojure and Clojurescript (i.e. CLJS-754), but that's a much bigger project. Especially considering it will probably not bring much of a speedup over an if-else-if implementation except in very narrow circumstances.

Comment by David Nolen [ 19/Feb/14 4:38 PM ]

Francis Avila I would make no such assumptions about performance without benchmarks. One of the critical uses for case is over keywords. Keyword hashes are computed at compile time, so that's one function call and a jump on some JavaScript engines. This is particularly useful for the performance of records where you want to lookup a field via keyword before checking the extension map.

This ticket should probably wait for CLJS-754 before proceeding.

Comment by Francis Avila [ 22/Feb/14 4:44 AM ]

Record field lookup is a good narrow use case to test. I put together a jsperf to compare if-else (current) vs switch with string cases vs switch with int cases (i.e., hash-compares, assuming perfect hashing).

Comment by David Nolen [ 10/May/14 3:59 PM ]

I've merged the case* analyzer and emitter bits by hand into master.

Comment by David Nolen [ 10/May/14 4:42 PM ]

I'v merged the rest of the patch into master. If any further optimizations are done it will be around dispatching on hash code a la Clojure.

Comment by Francis Avila [ 11/May/14 12:53 AM ]

Your keyword-test optimization has a bug: https://github.com/clojure/clojurescript/commit/9872788b3caa86f639633ff14dc0db49f16d3e2a

Test case:

(let [x "a"] (case x :a 1 "a"))
;=> 1
;;; Should be "a"

Github comment suggests two possible fixes.

Comment by David Nolen [ 11/May/14 10:50 AM ]

Thanks fix in master.

Comment by Christoffer Sawicki [ 23/Jun/14 3:41 PM ]

case over "chars" is currently not being optimized to switch (in other words: (case c (\a) :a :other) uses if instead of switch).

Given that ClojureScript chars are just strings of length 1, could this perhaps simply be fixed by tweaking https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/core.clj#L1187 ?

Comment by Christoffer Sawicki [ 23/Jun/14 4:11 PM ]

OK, I couldn't resist trying and it seems to be that easy. Would be great if somebody more knowledgeable could look at it and say if it has any side-effects. (See patch with name 0001-CLJS-713-Allow-test-expressions-for-case-to-be-chars.patch.)

Comment by David Nolen [ 23/Jun/14 4:15 PM ]

The patch looks good I would have applied it if I hadn't already gone and done it master myself just now

Comment by Christoffer Sawicki [ 23/Jun/14 4:22 PM ]

Hehe. Thanks! Don't forget to update the "case* tests must be numbers or strings" message on line 496 too.

Comment by David Nolen [ 23/Jun/14 4:48 PM ]

The existing docstring is inaccurate - case supports all compile time literals.





[CLJS-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 03/Dec/13

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   

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-710] port clojure.pprint Created: 02/Dec/13  Updated: 27/Oct/14

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


 Comments   
Comment by Shaun LeBron [ 15/Aug/14 1:29 PM ]

working on this here: https://github.com/shaunlebron/cljs-pprint

Comment by Shaun LeBron [ 05/Oct/14 12:40 PM ]

I'm ceasing development on the port.

fipp should be ported in place of pprint in cljs. pprint is slow and complex, whereas fipp is fast, simple, and very customizable.

Brandon Bloom is awaiting a go-ahead from the cljs community on whether the fipp port should be completed:
https://github.com/brandonbloom/fipp/issues/7

Comment by David Nolen [ 05/Oct/14 12:55 PM ]

fipp only covers a very small (though important part) of clojure.pprint's functionality. I think it's fine that fipp is a standalone library that user's can adopt in their own projects if they desire. This doesn't change the desire for a full port of clojure.pprint.

Comment by Shaun LeBron [ 27/Oct/14 1:13 PM ]

k, resuming.





[CLJS-705] locals clearing Created: 27/Nov/13  Updated: 29/Nov/13

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   

Without locals clearing ClojureScript is likely to suffer from the same cases as Clojure did for common uses of lazy sequences.



 Comments   
Comment by David Nolen [ 29/Nov/13 3:03 PM ]

For this we'll need to introduce some special private way to set a local to nil, i.e. (_clear_local sym)





[CLJS-689] js/-Infinity munges to _Infinity Created: 20/Nov/13  Updated: 02/Dec/14

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   

A dumb special-casing of js/ munging should fix.






[CLJS-667] validate extend-type and extend-protocol shape Created: 07/Nov/13  Updated: 02/Dec/14

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   

deftype and defrecord are sugar over the extend-type grouping, this is a source of confusion we should signal an error/warning if extend-type / extend-protocol malformed.






[CLJS-666] :require-macros should throw a sensible error if no macro file exists Created: 06/Nov/13  Updated: 25/Apr/14

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


 Comments   
Comment by Jonas Enlund [ 25/Apr/14 1:29 PM ]

What would you consider to be a sensible error? The resulting stacktrace after a failed (:require [cljs.reader :include-macros true]) looks like this:

Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:test/cljs/test_runner.cljs {:file #<File test/cljs/test_runner.cljs>}
at clojure.core$ex_info.invoke(core.clj:4327)
at cljs.compiler$compile_file.invoke(compiler.clj:1006)
at cljs.compiler$compile_root.invoke(compiler.clj:1059)
at cljs.closure$compile_dir.invoke(closure.clj:337)
at cljs.closure$eval2820$fn__2821.invoke(closure.clj:377)
at cljs.closure$eval2757$fn_2758$G2748_2765.invoke(closure.clj:288)
at cljs.closure$eval2807$fn__2808.invoke(closure.clj:391)
at cljs.closure$eval2757$fn_2758$G2748_2765.invoke(closure.clj:288)
at cljs.closure$build.invoke(closure.clj:940)
at cljs.closure$build.invoke(closure.clj:909)
at user$eval2998.invoke(cljsc.clj:21)
at clojure.lang.Compiler.eval(Compiler.java:6619)
at clojure.lang.Compiler.load(Compiler.java:7064)
at clojure.lang.Compiler.loadFile(Compiler.java:7020)
at clojure.main$load_script.invoke(main.clj:294)
at clojure.main$script_opt.invoke(main.clj:356)
at clojure.main$main.doInvoke(main.clj:440)
at clojure.lang.RestFn.invoke(RestFn.java:436)
at clojure.lang.Var.invoke(Var.java:423)
at clojure.lang.AFn.applyToHelper(AFn.java:167)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: Could not locate cljs/reader__init.class or cljs/reader.clj on classpath: at line 1 /home/jonas/dev/clojure/clojurescript/test/cljs/foo/ns_shadow_test.cljs {:tag :cljs/analysis-error, :file "/home/jonas/dev/clojure/clojurescript/test/cljs/foo/ns_shadow_test.cljs", :line 1, :column 1}
at clojure.core$ex_info.invoke(core.clj:4327)
at cljs.analyzer$error.invoke(analyzer.clj:267)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1415)
at cljs.analyzer$analyze$fn__1517.invoke(analyzer.clj:1506)
at cljs.analyzer$analyze.invoke(analyzer.clj:1499)
at cljs.analyzer$analyze.invoke(analyzer.clj:1494)
at cljs.analyzer$analyze_file$fn__1534.invoke(analyzer.clj:1575)
at cljs.analyzer$analyze_file.invoke(analyzer.clj:1569)
at cljs.analyzer$analyze_deps.invoke(analyzer.clj:963)
at cljs.analyzer$eval1306$fn__1308.invoke(analyzer.clj:1131)
at clojure.lang.MultiFn.invoke(MultiFn.java:241)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1417)
at cljs.analyzer$analyze$fn__1517.invoke(analyzer.clj:1506)
at cljs.analyzer$analyze.invoke(analyzer.clj:1499)
at cljs.analyzer$analyze.invoke(analyzer.clj:1494)
at cljs.compiler$compile_file_STAR_.invoke(compiler.clj:885)
at cljs.compiler$compile_file.invoke(compiler.clj:999)
... 20 more
Caused by: java.io.FileNotFoundException: Could not locate cljs/reader__init.class or cljs/reader.clj on classpath:
at clojure.lang.RT.load(RT.java:443)
at clojure.lang.RT.load(RT.java:411)
at clojure.core$load$fn__5018.invoke(core.clj:5530)
at clojure.core$load.doInvoke(core.clj:5529)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.core$load_one.invoke(core.clj:5336)
at clojure.core$load_lib$fn__4967.invoke(core.clj:5375)
at clojure.core$load_lib.doInvoke(core.clj:5374)
at clojure.lang.RestFn.applyTo(RestFn.java:142)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$load_libs.doInvoke(core.clj:5413)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$require.doInvoke(core.clj:5496)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at cljs.analyzer$eval1306$fn__1308.invoke(analyzer.clj:1137)
at clojure.lang.MultiFn.invoke(MultiFn.java:241)
at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1417)
... 34 more





[CLJS-575] cljsc.bat emit FileNotFoundException when compile samples in windows Created: 25/Aug/13  Updated: 19/Jun/14

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

Type: Defect Priority: Major
Reporter: Park Sang Kyu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: Compiler, bug, patch
Environment:

in windows 7


Attachments: File cljsc.bat.diff     File cljsc-path.bat    
Patch: Code

 Description   

cljsc.bat emit FileNotFoundException when it compile samples of the ClojureScript project in windows like below.

------------------------------------------------
Exception in thread "main" java.io.FileNotFoundException: Could not locate cljs/closure__init.class
or cljs/closure.clj on classpath:
------------------------------------------------

It is caused by lack of a backslash in the end of path of the system environment variable, %CLOJURESCRIPT_HOME% set by a user.
In the case CLASSPATH is set to "C:\\clojure\clojurescriptsrc\clj;C:\\clojure\clojurescriptsrc\cljs" and this make it impossible for javac to find cljs/clojure.clj file.

So it can be solved by adding a backslash to the path of %CLOJURESCRIPT_HOME%.

I attached the patched file, "cljsc-path.bat"



 Comments   
Comment by David Nolen [ 04/Sep/13 11:04 PM ]

Can we please get a proper git diff thanks (and please send in your CA)! Also would be nice to get Windows users to check this out.

Comment by Park Sang Kyu [ 15/Sep/13 3:16 AM ]

git diff

Comment by David Nolen [ 05/Oct/13 11:55 AM ]

Thank you! Have you sent in your CA? http://clojure.org/contributing

Comment by Park Sang Kyu [ 19/Jun/14 10:24 AM ]

Yes i have sent my CA.

Comment by David Nolen [ 19/Jun/14 10:27 AM ]

Excellent, the patch is not correctly formatted. Can we get a new patch that conforms to http://github.com/clojure/clojurescript/wiki/Patches





[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-497] Constant literal optimization Created: 25/Apr/13  Updated: 27/Aug/13

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

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


 Description   

We should optimize constant literals, in particular keywords. This optimization means that we will have to decide whether to make identical? slower by testing for keywords (this means it's probably a bad idea to continue to inline it) or to provide a special keyword-identical? that does the right thing.



 Comments   
Comment by Sean Grove [ 26/Aug/13 11:03 PM ]

This is related to the reified keywords in cljs, see http://dev.clojure.org/jira/browse/CLJS-576

Comment by Sean Grove [ 27/Aug/13 4:24 PM ]

There's another interesting twist while using piggieback + brepl that relates to a missing constants_table.js. Not sure what causes it (haven't found a way to repro), but only happens in a few circumstances, so the repl still mainly works.

The runtime part continues to works fine however.





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

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

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





[CLJS-365] apply needs to put all args after the 20th 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-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-109] Compiler errors/warnings should be displayed when cljs namespace 'package' names start with an unacceptable javascript symbol. Created: 29/Nov/11  Updated: 31/Aug/12

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

Type: Defect Priority: Major
Reporter: Benjamin Conlan Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None
Environment:

OSX 10.7



 Description   

Clojurescript namespaces are extremely flexible. The generated javascript symbol names are not. ie:

cljs:
[code]
(ns canvas.context.2d)

(defn ^:export create-image-data
([canvas image-data] (.createImageData image-data))
([canvas sw sh] (.createImageData canvas sw sh)))
[/code]

generated js:
[code]
goog.provide('canvas.context.2d');
goog.require('cljs.core');
canvas.context.2d.create_image_data = (function() {
var create_image_data = null;
var create_image_data__2432 = (function (canvas,image_data){ return image_data.createImageData(); });
var create_image_data__2433 = (function (canvas,sw,sh){ return canvas.createImageData(sw,sh); });
create_image_data = function(canvas,sw,sh){
switch(arguments.length){ case 2 : return create_image_data__2432.call(this,canvas,sw); case 3 : return create_image_data__2433.call(this,canvas,sw,sh); }
throw('Invalid arity: ' + arguments.length);
};
return create_image_data;
})()
;
goog.exportSymbol('canvas.context.2d.create_image_data', canvas.context.2d.create_image_data);[/code]

Note the symbol name "canvas.context.2d.create_image_data". Because of the "2d" namespace name, the javascript generated is invalid.

This can simply be resolved by renaming the namespace but some notification to the developer during the compilation stage should help avoid unnecessary problems.



 Comments   
Comment by Jozef Wagner [ 09/Jan/12 2:59 PM ]

I had a similar problem when my namespace contained a word class, e.g. in namespace:

(ns foo.bar.class)

Advanced closure compiler produced an error treating class as a JS keyword.





[CLJS-27] Conditional compilation (or reading) Created: 22/Jul/11  Updated: 07/Nov/14

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

Type: Enhancement Priority: Major
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Unresolved Votes: 12
Labels: None

Attachments: File cljs-27.diff     File cljs-27-v2.diff     File cljs-27-v3.diff     File conditional-compilation-clojure.diff     File conditional-compilation-clojurescript.diff    
Patch: Code and Test
Approval: Vetted

 Description   

As people start trying to write libs that are portable between Clojure and ClojureScript they might need to have a bit of branching on target. N.B. supporting this means a change to Clojure, although it has general utility there as well.

Consider CL #+ #- reader macros - http://www.lispworks.com/documentation/lw50/CLHS/Body/02_dhq.htm

Patch: cljs-27-v3.diff

Related: CLJ-1424, TRDR-14



 Comments   
Comment by Roman Scherer [ 19/Jul/12 8:52 AM ]

The following patches include an implementation of Common Lisp's #+
and #- reader macros to allow conditional compilation/reading for
Clojure and ClojureScript.

The patches add a dynamic variable called features to the
clojure.core and cljs.core namespaces, that should contain the
supported features of the platform in question as keywords.

Unlike in Common Lisp, the variable is a Clojure set and not a list.
In Clojure the set contains at the moment the :clojure keyword, and in
ClojureScript the :clojurescript keyword.

I would like to get feedback on the names that are added to this
variable. Are those ok? Is :jvm for Clojure and :js for ClojureScript
better? Should ClojureScript add something like :rhino, :v8 or
:browser as well?

To run the ClojureScript tests, drop a JAR named "clojure.jar" that
has the Clojure patch applied into ClojureScript's lib directory.

Comment by David Nolen [ 19/Jul/12 12:18 PM ]

This is an enhancement so it probably requires a design page and extended discussion before it will go anywhere. Until that happens I'm marking this is as low priority.

Comment by Roman Scherer [ 19/Jul/12 1:45 PM ]

Ok. If someone could give me write access to the Clojure Dev Wiki I would be happy to start such a design page.

Comment by David Nolen [ 19/Jul/12 5:50 PM ]

If you've sent in your CA request permissions on the clojure-dev mailing list.

Comment by Roman Scherer [ 21/Jul/12 5:45 AM ]

I started a design page for this ticket in the Clojure Dev wiki:
http://dev.clojure.org/display/design/Feature+Expressions

Comment by Stuart Halloway [ 27/Jul/12 1:48 PM ]

Posting my comments over on the design page...

Comment by Alex Miller [ 06/Aug/14 7:42 AM ]

Latest patch updates into current ClojureScript and use of tools.reader/tools.analyzer etc. The reader changes are all in the accompanying tools.reader patch in TRDR-14. This patch adds support to allow a new option "features" which is expected to be a set of keywords. build will inject :cljs into this set. The feature set is maintained in clojure.tools.reader/*features*. set! is enhanced to special-case an update to *features* in the code (presumably a rarely-used feature).

Because tools.reader needs the supporting patch, I left in several changes that pull in a new version of tools.reader - I don't actually expect those to be the correct versions but they are there as a reminder to update all of the proper places.

Comment by Alex Miller [ 11/Sep/14 9:04 AM ]

cljs-27-v2.diff adds ability to load .clj files as well as .cljs files when compiling.

Comment by Alex Miller [ 07/Nov/14 10:55 AM ]

Fresh patch, switch to take .cljc files.





Generated at Fri Dec 19 18:13:04 CST 2014 using JIRA 4.4#649-r158309.