<< Back to previous view

[CLJS-1072] Calling .hasOwnProperty("source") in Clojurescript's string/replace will break with ES6 Created: 01/Mar/15  Updated: 02/Mar/15  Resolved: 02/Mar/15

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

Type: Defect Priority: Major
Reporter: Matthew Davidson Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: bug, cljs, es6, replace, string
Environment:

Firefox Developer Edition
Mac OS X 10.10.1
Clojurescript 0.0-2913 (but I checked, and it's still present in current master branch)



 Description   

The "replace" function in string.cljs identifies RegExp objects by calling .hasOwnProperty to check for the "source" property. If true, then it's a regex. However, in ES6, the "source" property will become part of the RegExp prototype and not the actual object, so .hasOwnProperty("source") will return false instead.

Documentation on the RegExp change is at http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/source under the "Specifications" section. The change hit Firefox two weeks ago, according to this bug report, https://bugzilla.mozilla.org/show_bug.cgi?id=1120169, so it will still be 2-3 months before it appears in the main release channel. But given that ES6 is aiming for a June release date, we should expect other browsers to show this error soon enough.

To fix the issue, the Javascript "in" operator would work, if there's a way to compile cljs to use it. Otherwise, the next best thing might be to check to see if the "source" property is non-nil. Basically, the test:

(.hasOwnProperty match "source")

could become:

(.-source match)

It's not a major code change at all, but I'm happy to provide a patch and verify tests if you prefer.



 Comments   
Comment by Elliot Block [ 02/Mar/15 12:54 AM ]

Just to slightly summarize/reiterate, this bug means that `string/replace` is always broken if you pass it a RegExp as the `match` (under the new ES6 rules, already live in FF Dev Ed.)

This further breaks plenty of applications, like it immediately breaks the `secretary` router found in many Om apps.

Comment by David Nolen [ 02/Mar/15 7:46 AM ]

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





[CLJS-1071] Convert symbol keys in :closure-defines to munged strings Created: 01/Mar/15  Updated: 01/Mar/15  Resolved: 01/Mar/15

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

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


 Comments   
Comment by David Nolen [ 01/Mar/15 9:54 AM ]

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





[CLJS-1070] top-level boolean inference does not work Created: 28/Feb/15  Updated: 28/Feb/15

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

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


 Description   

Problem for using boolean Closure defines






[CLJS-1069] Generic :jsdoc support Created: 28/Feb/15  Updated: 28/Feb/15  Resolved: 28/Feb/15

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

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


 Description   

Currently :jsdoc support is not generalized for use. This means users cannot easily leverage Google Closure defines.



 Comments   
Comment by David Nolen [ 28/Feb/15 12:14 PM ]

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





[CLJS-1068] CLJS_NODE_TARGET define Created: 27/Feb/15  Updated: 28/Feb/15  Resolved: 28/Feb/15

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

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


 Description   

While we have our own means of tracking in the analyzer/compiler this it would be nice to expose a user-land closure define so that users can customize their own code bases now that this is out of scope for conditional reading.



 Comments   
Comment by David Nolen [ 28/Feb/15 12:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/801861cc03d5dc3c783b18a375eec37725e6671b





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

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

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


 Description   

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-1066] Rhino REPL regression Created: 26/Feb/15  Updated: 26/Feb/15  Resolved: 26/Feb/15

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

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


 Description   

Uncovered by CLJS-1065



 Comments   
Comment by David Nolen [ 26/Feb/15 8:52 AM ]

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





[CLJS-1065] Fix inconsistencies in Quick Start Guide Created: 24/Feb/15  Updated: 26/Feb/15

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   

Good feedback from Micah Martin:

  • Compiler in REPL seems broken: https://gist.github.com/slagyr/7d52ba4ba98e96172456
  • Not interested in Node.js so I skipped all that
  • Development Mode: Okay... this is where I'm confused.
  • The wiki says include 'hello.js'.
  • hello-dev.html explicitely includes goog.base.js.
  • hello-dev.html also requires hello.core.
  • The generated hello.js loads goog.base if it isn't already.
  • out/cljs_deps.js looks like cljsbuild's :output-to file
  • cljbuild doesn't generate anything like sample/hello.js
  • maybe I'll learn about all these difference later in the guide
  • It seems that "Development Mode" requires a modest but significant change in the way the js is included in the page. This is not obvious in the guide. It would be nice if no change was needed.
  • At the end of the Quick Start page... where do I go from here? Would be nice if there was a recomendation.
  • The 'lein cljsbuild' page doesn't address the confusion mentioned above
  • "Source maps" mentions :source-map-path... Am I supposed to add this to [:cljsbuild :builds :name :compiler]? Doesn't seem to do anything.

In the end, I have better picture of the ClojureScript landscape. I see where my assumptions failed me, and huzzah, got source maps working.



 Comments   
Comment by David Nolen [ 26/Feb/15 8:55 AM ]

The Rhino REPL issue has been addressed in master.





[CLJS-1064] ex-info is not printable Created: 24/Feb/15  Updated: 01/Mar/15  Resolved: 01/Mar/15

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

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


 Description   
(str (ex-info "foo" {}))

Fails under master.



 Comments   
Comment by David Nolen [ 01/Mar/15 12:26 PM ]

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





[CLJS-1063] Regression for Rhino REPL part of Quick Start tutorial Created: 24/Feb/15  Updated: 28/Feb/15  Resolved: 28/Feb/15

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

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


 Description   

Curiously bin/repljs appears to work. When trying the tutorial we get a bunch of "already declared" errors which seems like a REPL bootstrapping issue.



 Comments   
Comment by David Nolen [ 28/Feb/15 10:24 AM ]

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





[CLJS-1062] Incorrect deftype/defrecord definition leads to complex error messages Created: 24/Feb/15  Updated: 24/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Julien Eluard Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File deftype-fields.diff    

 Description   

Every once in a while I forget to define fields in deftype/defrecord. In this case compilation fails with a low level error message hard to understand.
Clojure compilation prints a better error message in this case: "No fields vector given."

I propose to backport Clojure error message.






[CLJS-1061] Expose disable-console-print! Created: 23/Feb/15  Updated: 24/Feb/15

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

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


 Description   

enable-console-print! is convenient from some developers on a team who like println to print to the console. However, for others, like myself, it is more desirable to print directly to the REPL (e.g. w/in Emacs) instead of the browser console. On occasion enable-console-print! get's checked and can cause errors that result from calling println before the *print-fn* is set. It then becomes a hassle to either comment out the println or the enable-console-print!. Having a disable-console-print! which could restore the previous *print-fn*, whatever that may have been, would be nice to have.

As it currently stands, one must do this manually, which frankly is kind of awkward and not immediately obvious. It is much like having a light you can turn on but not off unless you cut the main power.



 Comments   
Comment by Thomas Heller [ 24/Feb/15 6:01 AM ]

I think the whole print-fn handling is not optimal. The problem is that each namespace can set it and will override the previous setting and the last one wins. While this is most likely one of "my" namespaces and "my" print-fn it may be someone else's while initializing (loading dependant namespaces). So you might run into issues where some log output goes to console and some goes to util.print or whichever other method the library author decided to use. Some libraries already call (enable-console-print!) as the very first thing their namespace.

What I recently added to shadow-build is an option to make this a compiler option (eg. {:print-fn :console}). Code can just use prn, println and never worry about that *print-fn* might not be set. I think this is a very convenient option that can default to :console and one less hurdle new users may stumble over. I don't need to initialize println in Clojure either.

Maybe this option should make it into core.





[CLJS-1060] simplify nREPL integration Created: 23/Feb/15  Updated: 23/Feb/15

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   

-setup and -teardown need to be elidable. If cljs.env/*compiler* already bound use that. The eval of (ns cljs.user ...) form for auto-import of cljs.repl helpers needs to respect *cljs-ns* and restore it afterwards.






[CLJS-1059] Simple interface wanted to convert cljs forms to js Created: 22/Feb/15  Updated: 23/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Stuart Mitchell Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: analyzer, compiler

Patch: Code

 Description   

In our project (a clojurescript debugger) we want to convert cljs forms or a sequence of forms into javascript so that they can be executed in the javascript console.

We would like something similar to closure/compile-form-seq (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/closure.clj#L308)

However, we need to supply, the namespace requires and locals in an env like this

{:ns {:name "test.core" :requires {(quote gstring) (quote goog.string)}} :locals {}}

This code seems to do what we want.

(defn compile-form-seq
    \"Compile a sequence of forms to a JavaScript source string.\"
    [forms env]
    (env/ensure
    (compiler/with-core-cljs nil
      (fn []
        (with-out-str
            (doseq [form forms]
              (compiler/emit (analyzer/analyze env form))))))))

I am not sure why I need env/ensure.

Would you be able to patch compile-form-seq to provide the needed interface, or suggest what we should be doing.

Thanks
Stu



 Comments   
Comment by Mike Thompson [ 22/Feb/15 10:09 PM ]

Just to be clear:
1. when our debugger is at a breakpoint,
2. the user can type in an expression at the repl
3. in response, our debugger has to compile the user-typed-in expression to javascript (and then execute it, showing a result)
4. taking into account any local bindings. <---- this is the key bit.

To satisfy point 4, our tool extracts all the 'locals' from the current call-frame, and then supplies all these local bindings in env/locals, so the compiler doesn't stick a namespace on the front of them.

For example, if there was a local binding for 'x' in the callstack, and the user's repl-entered-expression involves 'x', then we want the compiler to leave the symbol 'x' alone and to not put some namespace on the front of it. In the final javascript, it must still be 'x', not 'some.namespace.x'

Our method to achieve this is to put 'x' into env/locals when compiling – and it all works. Except, with the recent changes this has become more of a challenge. Hence this ticket asking for a way to pass in env.

Comment by Thomas Heller [ 23/Feb/15 3:19 AM ]

You could wrap the user expression in an fn, that would allow you to skip messing with the locals. The REPL basically does the same trick for *1,*2,...

(fn [x]
  ~user-expression-here)




[CLJS-1058] Arities listed in doc output are wrapped in (quote ...) Created: 22/Feb/15  Updated: 23/Feb/15  Resolved: 22/Feb/15

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

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


 Description   

The arities listed in the doc output have an extra (quote ...) wrapping them.

Example:

ClojureScript:cljs.user> (doc +)
-------------------------
cljs.core/+
(quote ([] [x] [x y] [x y & more]))
  Returns the sum of nums. (+) returns 0.
nil


 Comments   
Comment by Mike Fikes [ 22/Feb/15 4:38 PM ]

This is a regression that occurred with the commit for CLJS-1055.

Comment by David Nolen [ 22/Feb/15 11:36 PM ]

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

Comment by Mike Fikes [ 23/Feb/15 6:37 AM ]

Confirmed fixed.





[CLJS-1057] Nashorn REPL should not use EDN rep for errors Created: 22/Feb/15  Updated: 22/Feb/15

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   

Nashorn REPL should just convert the JVM stacktrace into a string form that mirrors JavaScript stack property and implement IParseStacktrace. This seems like pointless work but it simplifies cljs.repl/pst which needs to grab the error value out of Nashorn runtime anyway. The current strategy will not work work cljs.repl/pst.






[CLJS-1056] declaring an already-defined var clobbers its metadata Created: 22/Feb/15  Updated: 22/Feb/15

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

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


 Description   

For example, in om.core, path is defined at https://github.com/omcljs/om/blob/master/src/om/core.cljs#L106 and then declared later in the file at https://github.com/omcljs/om/blob/master/src/om/core.cljs#L152. Examining its meta at the REPL using

(meta #'om.core/path)

yields:

{:arglists (),
 :test nil,
 :name path,
 :column 1,
 :line 152,
 :file "resources/public/js/out/om/core.cljs",
 :doc nil,
 :ns om.core}

Note that {:arglists} is empty and {:line} points to the line of the declare.

Examining the compiler env using e.g.

(get-in @cljs.env/*compiler* [:cljs.analyzer/namespaces 'om.core :defs 'path])

yields:

{:file "file:/Users/griffithsm/.m2/repository/om/om/0.6.2/om-0.6.2.jar!/om/core.cljs",
 :line 113,
 :column 1,
 :end-line 113,
 :end-column 23,
 :declared true,
 :test true,
 :name om.core/path}





[CLJS-1055] cljs.repl/doc should support namespaces and special forms Created: 22/Feb/15  Updated: 22/Feb/15  Resolved: 22/Feb/15

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

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


 Comments   
Comment by David Nolen [ 22/Feb/15 10:11 AM ]

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





[CLJS-1054] add clojure.repl/source functionality to cljs.repl Created: 22/Feb/15  Updated: 22/Feb/15  Resolved: 22/Feb/15

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

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


 Comments   
Comment by David Nolen [ 22/Feb/15 9:09 AM ]

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





[CLJS-1053] REPLs need import special fn Created: 22/Feb/15  Updated: 22/Feb/15  Resolved: 22/Feb/15

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

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


 Comments   
Comment by David Nolen [ 22/Feb/15 8:21 AM ]

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





[CLJS-1052] Cannot require ns from within the ns at the REPL for reloading purposes Created: 21/Feb/15  Updated: 21/Feb/15  Resolved: 21/Feb/15

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

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


 Comments   
Comment by David Nolen [ 21/Feb/15 9:28 AM ]

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





[CLJS-1051] :modules validation, :output-to should be unique Created: 20/Feb/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

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


 Comments   
Comment by David Nolen [ 20/Feb/15 8:26 PM ]

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





[CLJS-1050] Warn on code that will obviously prevent dead code elimination or cross module code motion Created: 20/Feb/15  Updated: 20/Feb/15

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

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


 Description   

Might make sense to support a module level flag like *warn-on-reflection*, perhaps *warn-on-deoptimization*? Simple cases of this are top-level data structures and and top-level side effects.






[CLJS-1049] Include macro information in var resolution functions Created: 20/Feb/15  Updated: 22/Feb/15

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


 Description   

The fundamental issue around CLJS-964. Macro information doesn't appear in any of the analysis helpers.



 Comments   
Comment by David Nolen [ 22/Feb/15 12:48 PM ]

This is an important problem. Because this information is missing macros are not of part of the various reflection helpers that are now a big part of ClojureScript REPLs.





[CLJS-1048] support function values in static vars compile time metadata Created: 20/Feb/15  Updated: 20/Feb/15

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

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


 Description   

Function values are currently only supported for :test metadata key as a special case.






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

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

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


 Description   

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



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

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

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

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





[CLJS-1046] static vars do not respect user compile time metadata Created: 18/Feb/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

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

Attachments: Text File 1046.patch     Text File 1046-upd0.patch    

 Description   

We should probably generalize the :test function value support.



 Comments   
Comment by Ivan Mikushin [ 19/Feb/15 7:12 AM ]

I've submitted a patch.

Comment by David Nolen [ 19/Feb/15 7:35 AM ]

Thanks! Have you submitted your CA?

The patch looks good, it doesn't address the case where a user may accidentally include function values but this can be addressed separately in an enhancement ticket.

Comment by Ivan Mikushin [ 19/Feb/15 7:47 AM ]

I've signed the CA before uploading the patch.

Comment by Ivan Mikushin [ 19/Feb/15 8:20 AM ]

I'll add the ticket later today. Can you give me a pointer where to look for the current :test function value support?

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

It will need to be handled in def analyzer case as that's the environment function values must be interpreted in. See https://github.com/clojure/clojurescript/blob/6019e4908b6643cf1e9dd2c403f9bfdf15ab7495/src/clj/cljs/analyzer.clj#L764

Comment by Ivan Mikushin [ 20/Feb/15 6:07 AM ]

CLJS-1048

Comment by Ivan Mikushin [ 20/Feb/15 7:25 AM ]

rebase the patch and rm some noise

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

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

Ivan, I did not use the supplied patch as is because it leaked internal analyzer and compiler metadata into user space. I did copy over the supplied tests.

Let me know if this works for you thanks.

Comment by Ivan Mikushin [ 20/Feb/15 1:21 PM ]

Awesome, thanks! It works great.

Comment by David Nolen [ 20/Feb/15 1:37 PM ]

Ivan glad to hear it! Thanks for the confirmation.





[CLJS-1045] cljs.repl/repl should mirror the expectations and behavior of the standard Clojure REPL as closely as possible Created: 17/Feb/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

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


 Description   

All the differences simply creates trouble and needless indirection for other tooling.



 Comments   
Comment by David Nolen [ 17/Feb/15 4:20 PM ]

Worth investigating if we can't implement cljs REPLs on top the Clojure REPL as suggested by Chas Emerick.

Comment by David Nolen [ 17/Feb/15 8:31 PM ]

Not going to reuse clojure.main/repl it's more trouble than it's worth and we need more options and other contextual things. However the option set that clojure.main/repl accepts is simple, clean and well considered. Happy to make this set the meeting place for the default behavior and customizations needed by tools like nREPL.

Comment by David Nolen [ 20/Feb/15 4:36 PM ]

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





[CLJS-1044] Investigate Closure CommonJS Support Created: 16/Feb/15  Updated: 16/Feb/15

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

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


 Description   

It appears Google Closure has basic support for CommonJS modules. Apparently mixing CommonJS and Closure style libraries seems somewhat supported but this is not a necessary goal. For our purposes it may be satisfactory to simply allow CommonJS libs to be a part of the build, likely as a separate pass, in which require is compiled away.






[CLJS-1043] top-level locals issues Created: 16/Feb/15  Updated: 16/Feb/15

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

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


 Description   
(let [x #js {"Foo" #js {"Bar" (fn [])}}
      y (new x.Foo.Bar.)])

x will get renamed and not considered a local. This seems like a fundamental problem with whatever top level let special casing we may have in place.






[CLJS-1042] Google Closure Modules :source-map support Created: 15/Feb/15  Updated: 18/Feb/15  Resolved: 18/Feb/15

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

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


 Description   

Should be pretty straightforward but means we probably want to make cljs.closure/emit-optimized-source-map a bit more generic so that it may be reused for this ticket.



 Comments   
Comment by David Nolen [ 18/Feb/15 7:02 PM ]

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





[CLJS-1041] Google Closure Modules :foreign-libs support Created: 15/Feb/15  Updated: 16/Feb/15  Resolved: 16/Feb/15

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

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


 Description   

We should probably just move :foreign-libs into :cljs-base. If a user wants to lock a foreign library to a module they can add it to that module's :entries. If multiple modules need to share some subset of :foreign-libs these should be lifted manually into their own module.



 Comments   
Comment by David Nolen [ 16/Feb/15 9:37 PM ]

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





[CLJS-1040] Source-mapped script stack frames for the Nashorn repl Created: 15/Feb/15  Updated: 15/Feb/15  Resolved: 15/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Pieter van Prooijen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

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

 Description   

Add the new source map support for stack frames to the Nashorn repl. It now also reports the actual javascript stackframes (when applicable) to exceptions which happen during repl evaluation.



 Comments   
Comment by David Nolen [ 15/Feb/15 9:31 AM ]

fixed https://github.com/clojure/clojurescript/commit/664c8d54599d4e93a598423777bcb7f579099430





[CLJS-1039] Under Emacs source directory watching triggers spurious recompilation Created: 14/Feb/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

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


 Description   

Emacs is noisy when it comes to the writing disk. We currently filter out only JS and CLJS files but we probably need to do a little more.



 Comments   
Comment by David Nolen [ 20/Feb/15 4:58 PM ]

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





[CLJS-1038] Namespace aliasing does not appear to work in the built in browser REPL Created: 14/Feb/15  Updated: 21/Feb/15  Resolved: 21/Feb/15

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

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


 Description   

When creating a simple "Hello world" style project, requiring and aliasing the main namespace does not appear to work. Warnings are emitted about the namespace not existing. Sneaking suspicion that this is a code path issue for browser REPL - it's the oldest REPL outside of Rhino and has received the fewest updates.



 Comments   
Comment by David Nolen [ 21/Feb/15 10:11 AM ]

Cannot replicate this on master





[CLJS-1037] cls.analyzer/ns-dependents fails for common cases Created: 13/Feb/15  Updated: 13/Feb/15  Resolved: 13/Feb/15

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

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

CLJS 2843


Attachments: Text File CLJS_1037.patch     Text File CLJS_1037-tests-and-fix.patch    

 Description   

Because of the order of execution in ns-dependents namespaces can be eliminated and not included in the final return set.



 Comments   
Comment by Bruce Hauman [ 13/Feb/15 2:52 PM ]

I attached a patch with the failing test. There are lots of fixes for this I'm going to submit one in just a few.

Comment by Bruce Hauman [ 13/Feb/15 3:19 PM ]

Attached a path with a fix, but would prefer to rewrite the function.

Comment by David Nolen [ 13/Feb/15 3:25 PM ]

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





[CLJS-1036] cljs.closure/build does not find upstream dependencies when called from worker thread Created: 13/Feb/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

Type: Defect Priority: Major
Reporter: Michael Griffiths Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: cljs, closure, compiler

Attachments: Text File CLJS-1036--001.patch    

 Description   

Example stacktrace: https://www.refheap.com/97198 (context is using figwheel in a clojure REPL from CIDER)

Because cljs.closure/build calls the 0-arity form of get-upstream-deps, it is implicitly using the current thread's classloader to find the deps.cljs resources. It is then assoc'ing the result into opts, so the caller has no way of gathering the dependencies themselves and passing them in.



 Comments   
Comment by Michael Griffiths [ 13/Feb/15 1:40 PM ]

Patch CLJS-1036--001.patch, attached, uses merge instead of assoc to allow passing of the upstream dependency data via opts.

Comment by Michael Griffiths [ 13/Feb/15 1:45 PM ]

We could also allow passing of a :resources-classloader opt so callers don't have to gather the dependencies themselves, if you'd prefer a patch that does that.

Comment by Bruce Hauman [ 15/Feb/15 2:17 PM ]

This is a problem with calling cljs.closure/build inside nREPL in general when upstream deps are involved.

Comment by David Nolen [ 15/Feb/15 2:34 PM ]

I would like Chas Emerick to chime in on this thread before considering it.

Comment by Chas Emerick [ 16/Feb/15 9:02 AM ]

Could someone point me to a simple (sample?) project that uses upstream dependencies? I haven't used them yet, and don't have such a thing handy.

The proposed patch is confusing to me; if the problem is in resolving dependencies due to classloader state/configuration, how does it help to give opts the opportunity to clobber what get-upstream-deps returns? That is, doesn't this just move the problem downstream to who/whatever is calling build?

Comment by Michael Griffiths [ 16/Feb/15 10:10 AM ]

Chas: yes, the proposed patch simply moves the responsibility to the caller. I was under the assumption that changing how get-upstream-deps/build resolves the classloader by default would be too-breaking of a change, and had not even considered the fact that this might be a broader issue with nREPL itself.

Example usage of get-upstream-deps in the wild (that even mentions classloader issues): https://github.com/immutant/immutant-release/blob/ea538feb548bde86ebce20ec679da7a19b639259/integration-tests/apps/ring/basic-ring/src/basic_ring/core.clj#L51

Note that Weasel (at least) is currently gathering the upstream deps itself too: https://github.com/tomjakubowski/weasel/commit/5cd7009f584100f0d89fc7f00079458bb1f2c016

I'll set up an example project tonight if you like, but I think all you need is to add [cljsjs/react "0.12.2-5"] as a dependency to an existing cljs project and then require [cljsjs.react] in one of its namespaces.

Comment by Bruce Hauman [ 16/Feb/15 1:04 PM ]

Chas, David here is a very quick example:

If you do a

lein new om-mies hello-world
cd hello-world
lein repl
user> (require '[cljs.closure :as cc])
user> (cc/build "src" {})

Assuming lein repl started an nREPL repl. This will produce the error mentioned above. The problem is that om is utilizing the new deps.js facility to autmatically import cljsjs.react. get-upstream-deps depends on being run on the main thread.

https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/closure.clj#L1116

A solution that fixes this in get-upstream-deps* would be preferable.

Is this simple solution to use the system classloader when not on the main thread:

(if (not= (.getName (Thread/currentThread)) "main")
  (java.lang.ClassLoader/getSystemClassLoader)
  (. (Thread/currentThread) (getContextClassLoader)))

To naive? My experience with Java threads and classloaders is minimal.

Or should we always use the (java.lang.ClassLoader/getSystemClassLoader) in this case?

Comment by Chas Emerick [ 17/Feb/15 3:35 PM ]

Using the system classloader isn't necessary. get-upstream-deps is currently using .findResources, not .getResources; the difference is that the former looks only for matching resources in the target classloader, while the latter first delegates to the parent classloader, and only looks locally if nothing is found there. So, just using .getResources instead will make all current usage work as expected, I think. e.g.

;; `lein repl` nREPL here
user=> (enumeration-seq (.getResources (. (Thread/currentThread) (getContextClassLoader))  "deps.cljs"))
(#<URL jar:file:/home/chas/.m2/repository/cljsjs/react/0.12.2-5/react-0.12.2-5.jar!/deps.cljs>)
user=> (enumeration-seq (.findResources (. (Thread/currentThread) (getContextClassLoader))  "deps.cljs"))
nil

There are some classloader edge cases that might motivate making get-upstream-deps perform a more comprehensive search, explicitly pinging all classloaders for matching resources. This is sometimes necessary for these sorts of scanning scenarios if e.g. a deps.cljs file is available via a parent classloader, which will therefore "shadow" other deps.cljs files in child classloaders. In that case, you need something like cemerick.pomegranate/resources.

Hope that helps.

Comment by David Nolen [ 17/Feb/15 3:46 PM ]

Chas's suggested fix has been committed to master. It works on the simple tests I've run but I'd like to hear more confirmation. Thanks everyone.

https://github.com/clojure/clojurescript/commit/79208f5bf15825a2ba2d0ce95aae6d2b71966494

Comment by David Nolen [ 20/Feb/15 4:36 PM ]

Closing this one. Chas's solution is the correct one.





[CLJS-1035] Built in REPLs and 3rd party REPLs should support :watch option Created: 13/Feb/15  Updated: 13/Feb/15  Resolved: 13/Feb/15

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

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


 Description   

We should try running cljs.closure/watch in a future and see what problems arise. How much coordination do we need if we are auto building and a user can (require ... :reload) at the same time?



 Comments   
Comment by Mike Fikes [ 13/Feb/15 9:59 AM ]

There would probably be interesting interactions of this feature with CLJS-901.

Comment by David Nolen [ 13/Feb/15 2:51 PM ]

fixed https://github.com/clojure/clojurescript/commit/606ccce775706b9f9f6f40ecdcc40d6947226824

Comment by Mike Fikes [ 13/Feb/15 3:44 PM ]

Confirmed fixed.





[CLJS-1034] Support REPL-defined functions in stacktrace infrastructure Created: 12/Feb/15  Updated: 22/Feb/15

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


 Description   

Some new machinery has recently been added to ClojureScript facilitating dealing with exception stack traces and their source-mapping with the REPL-author-facing documentation here.

As an enhancement, support REPL-defined functions which have namespaces / functions, but no source to refer to. Clojure REPLs accomplish this via NO_SOURCE_FILE; this enhancement ticket asks for the same in ClojureScript.

To accomplish this probably involves expanding the definition of the canonical stactrace format (allowing :file key to me missing or with a nil value, or some such, along with similar ideas for :line and :column), along with some special conditional handling in the underlying source-mapping machinery to conditionally handle REPL-defined functions as a special case.



 Comments   
Comment by Mike Fikes [ 12/Feb/15 8:19 PM ]

A motivating example for this enhancement is detailed in a ticket for the Ambly REPL.





[CLJS-1033] take and drop accept nil as n argument Created: 12/Feb/15  Updated: 12/Feb/15

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

Type: Defect Priority: Minor
Reporter: Sean Grove Assignee: Sean Grove
Resolution: Unresolved Votes: 0
Labels: None
Environment:

ClojureScript


Attachments: Text File require-integer-for-take-and-drop.patch    
Patch: Code and Test

 Description   

(take nil [1 2 3]) in Clojure raises an error
In ClojureScript, it's the same as (take 0 [1 2 3])

This patch checks that take and drop both check that n is an integer, so that it doesn't silently behave differently.






[CLJS-1032] Node.js target should support :main Created: 12/Feb/15  Updated: 12/Feb/15  Resolved: 12/Feb/15

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

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


 Comments   
Comment by David Nolen [ 12/Feb/15 7:48 AM ]

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





[CLJS-1031] Download Closure over https: in ./script/bootstrap Created: 12/Feb/15  Updated: 12/Feb/15  Resolved: 12/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Chris Cowan Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Line 69 of ./script/bootstrap downloads Google's Closure Compiler from http://dl.google.com/closure-compiler/compiler-$CLOSURE_RELEASE.zip, though the URL may be accessed securely over HTTPS too. Could the script be changed to do so?



 Comments   
Comment by David Nolen [ 12/Feb/15 5:58 AM ]

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





[CLJS-1030] Add cljs.repl/pst for printing error values Created: 11/Feb/15  Updated: 22/Feb/15  Resolved: 22/Feb/15

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

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

Attachments: Text File CLJS-1030-2.patch     Text File CLJS-1030-3.patch     Text File CLJS-1030.patch    

 Description   

This would mirror clojure.repl/pst and leverage the new stacktrace mapping support.



 Comments   
Comment by Mike Fikes [ 11/Feb/15 9:04 PM ]

We now have *e and cljs.repl/IParseStacktrace (as well as optional cljs.repl/IPrintStacktrace) support so the pieces needed to do this appear to exist.

Comment by Mike Fikes [ 12/Feb/15 12:33 AM ]

I investigated: Since the values of interest (*e and others) are in the JavaScript engine, but the REPL has the stack trace parsing/mapping/printing support in the Clojure implementation, one approach is to define a new REPL special function, pst.

I was interested in feedback on whether this strategy makes sense, and if so, I'd be happy to pursue fleshing out a proper patch that supports this, mirroring clojure.repl/pst as much as possible.

Defining cljs.repl/pst as:

(defn pst
  ([repl-env env args] (pst repl-env env args nil))
  ([repl-env env args opts]
    (let [desc-js (comp/emit-str (ana/no-warn (ana/analyze env '(pr-str *e) opts)))
          desc-ret  (-evaluate repl-env "filename" 1 desc-js)
          rst-js (comp/emit-str (ana/no-warn (ana/analyze env '(.-stack *e) opts)))
          rst-ret (-evaluate repl-env "filename" 1 rst-js)
          ret {:value (:value desc-ret) :stacktrace (:value rst-ret)}]
      (display-error repl-env ret nil opts))))

and hooking in an appropriate 'pst in the cljs.repl/default-special-fns implementation yields promising behavior that could be polished:

ClojureScript:cljs.user> (ffirst 1)
Error: 1 is not ISeqable
	 cljs.core/seq (.ambly_jsc_repl/cljs/core.cljs:727:13)
	 cljs.core/first (.ambly_jsc_repl/cljs/core.cljs:736:7)
	 cljs.core/ffirst (.ambly_jsc_repl/cljs/core.cljs:1155:3)
nil
ClojureScript:cljs.user> *e
#<Error: 1 is not ISeqable>
ClojureScript:cljs.user> (pst)
#<Error: 1 is not ISeqable>
	 cljs.core/seq (.ambly_jsc_repl/cljs/core.cljs:727:13)
	 cljs.core/first (.ambly_jsc_repl/cljs/core.cljs:736:7)
	 cljs.core/ffirst (.ambly_jsc_repl/cljs/core.cljs:1155:3)

Another approach I tried, but failed with, is to mimic the way doc works; this fails because it doesn't allow you access to the REPL environment.

Comment by David Nolen [ 12/Feb/15 5:53 AM ]

Yeah I don't see how it can easily be done outside of a special function. Happy to see a patch along these lines.

Comment by Mike Fikes [ 14/Feb/15 9:50 PM ]

Patch attached.

Here is a sample run in the Ambly REPL (long paths elided for readability) illustrating exercising various arities:

ClojureScript:cljs.user> (ffirst 1)
Error: 1 is not ISeqable
	 cljs.core/seq (.../cljs/core.cljs:727:13)
	 cljs.core/first (.../cljs/core.cljs:736:7)
	 cljs.core/ffirst (.../cljs/core.cljs:1155:3)
nil
ClojureScript:cljs.user> (pst)
Error: 1 is not ISeqable
	 cljs.core/seq (.../cljs/core.cljs:727:13)
	 cljs.core/first (.../cljs/core.cljs:736:7)
	 cljs.core/ffirst (.../cljs/core.cljs:1155:3)

ClojureScript:cljs.user> (pst 2)
Error: 1 is not ISeqable
	 cljs.core/seq (.../cljs/core.cljs:727:13)
	 cljs.core/first (.../cljs/core.cljs:736:7)

ClojureScript:cljs.user> (def foo *e)
#<Error: 1 is not ISeqable>
ClojureScript:cljs.user> (pst foo)
Error: 1 is not ISeqable
	 cljs.core/seq (.../cljs/core.cljs:727:13)
	 cljs.core/first (.../cljs/core.cljs:736:7)
	 cljs.core/ffirst (.../cljs/core.cljs:1155:3)

ClojureScript:cljs.user> (pst foo 1)
Error: 1 is not ISeqable
	 cljs.core/seq (.../cljs/core.cljs:727:13)
Comment by Mike Fikes [ 15/Feb/15 8:41 AM ]

I'm going to submit a simpler patch that employs (.-message e) eliminating the additional function that strips the unreadable bits off.

Comment by Mike Fikes [ 15/Feb/15 9:00 AM ]

Simpler patch that uses (.-message e) and additionally strips description off front of stacktrace if it is already there (otherwise it appears twice in Node REPL).

Comment by Mike Fikes [ 15/Feb/15 9:22 AM ]

I upgraded to Node v0.12.0 and the patches in this ticket are somewhat thwarted by the same kind of stuff described at the bottom of CLJS-1018.

Comment by David Nolen [ 15/Feb/15 9:27 AM ]

Mostly looks good but even with the second patch I still see the error twice at the top. I tested using rlwrap ./scripts/noderepljs in the project repo.

Comment by Mike Fikes [ 15/Feb/15 9:34 AM ]

Right. The code to eliminate the double message worked under the previous version of node. Under Node v0.12.0 the (.-message e) is no longer strictly the first part of what is in the stack trace text.

For this aspect, it makes me feel that, just like parsing of the stack trace into a canonical vector form is delegated to REPLs, the same is perhaps the right approach here for the message part. Dunno yet, but it is feeling like a bit of a mess with the Node v0.12.0 error indicator appearing in parts of the message and stack trace making naive processing fragile.

I have no problem holding off on this patch if your opinion is that this needs to be better addressed. (We just need to sort out where to address it.)

Comment by Mike Fikes [ 15/Feb/15 11:58 AM ]

Third patch eliminates the doubling in Node.js REPL. The way it accomplishes this is to synthesize the first part of what a Node.js stack looks like and to remove it if it finds a match.

I've tested this on Node v0.12.0 and v0.10.25, as well as Ambly, and it works.

The negative is that this essentially has a little of Node v0.12.0 aspects in the base ClojureScript real code.

Comment by David Nolen [ 22/Feb/15 12:45 PM ]

fixed https://github.com/clojure/clojurescript/commit/5218a4f4d88db4483eefea80da66acfac20cc907





[CLJS-1029] Investigate how ns aliasing can be supported in ClojureScript macro files Created: 11/Feb/15  Updated: 11/Feb/15

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

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


 Description   

Currently we just require the file. Perhaps possible to control compilation of the macro file such that ClojureScript ns aliases are established. This may not bear fruit but worth thinking about.



 Comments   
Comment by David Nolen [ 11/Feb/15 4:05 PM ]

Any design ideas along this path needs to keep .cljc files in mind.





[CLJS-1028] Roll back "single-segmented namespace" warning Created: 10/Feb/15  Updated: 10/Feb/15  Resolved: 10/Feb/15

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

Type: Defect Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

This is about changed merged from CLJS-1004

I don't see why single-segment namespace is bad. Something like (ns rum) can have collision with <div id=rum>, but either can (ns rum.id) or (ns rum.lang) or ns rum.style.display).

If our goal is really robust semantic for namespaces, maybe it will be better to provide some warning when (require) instruction references non-ns object (I assume we can identify at runtime if JS object is ns or not?).

Anyway, I think CLJS-1004 patch should be rolled back



 Comments   
Comment by David Nolen [ 10/Feb/15 12:47 PM ]

We're not rolling this back. Single segment namespaces have many potential problems.

Comment by Nikita Prokopov [ 10/Feb/15 1:18 PM ]

Not for the sake of argue, but can you name a few? I really want to understand the subject

Comment by David Nolen [ 10/Feb/15 1:26 PM ]

Clashes with global objects & classpath problems. By using two or more segments you are exponentially decreasing the likelihood of either surfacing in your program.

This will not be rolled back. Disable the warning with the compiler flag if you want to suppress the warning. For third party libraries it's simply too bad. This anti-pattern has never been encouraged in Clojure ever.





[CLJS-1027] server side source map helper for mapping canonicalized stack traces under :none & concatenating builds Created: 10/Feb/15  Updated: 10/Feb/15

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

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





[CLJS-1026] CLJS Repl doesn't respond well to clients refreshing. Created: 10/Feb/15  Updated: 10/Feb/15  Resolved: 10/Feb/15

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

Type: Defect Priority: Minor
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

CLJS 2815



 Description   

It's not uncommon for CLJS browser clients to be refreshed. This blows away the cljs.user data on the client side. And this results in the REPL erroring whenever the repl user tries to define something in the cljs.user ns in the REPL.

I solved this in figwheel repl client code by ensuring that there is at least an empty #js {} in cljs.user before evaling code sent from the REPL.

I also require cljs.repl in my figwheel repl client code to ensure its existence through reload.



 Comments   
Comment by David Nolen [ 10/Feb/15 9:36 AM ]

Is there any reason you can't resend the init client state form, (ns cljs.user ...), when the client connects again? The existing REPL infrastructure sends it once for you but having to resend on reconnect is kind of a issue specific to certain kinds of REPLs semantics.

Comment by Bruce Hauman [ 10/Feb/15 10:44 AM ]

Yeah, Say I'm in IJavaScript/-evaluate because a connection was lost and I'm waiting for a new one, after connect I'll have to call cljs.repl/evaluate-form? It just doesn't seem like the best place to do it. It seems like strange behavior could result in certain circumstances.

Does my solution above not work?

Comment by David Nolen [ 10/Feb/15 11:34 AM ]

This one is out of scope.





[CLJS-1025] Source mapping could support clients more generally. Created: 10/Feb/15  Updated: 11/Feb/15  Resolved: 11/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently print-mapped-stacktrace takes a path from the root of the project.

(print-mapped-stacktrace
  [{:file "samples/hello/out/hello/core.js" <-- path from project root
    :function "first"
    :line 2
    :column 1}]))

Some clients are getting their code from a server and thus don't have project root relative paths.

In fact, the client side paths can be arbitrary in a way that is difficult and non trivial
for repl server code like IJavaScriptEnv's to deduce the correct project root path from.

For instance if a browser (figwheel) client is getting its code from localhost/arbtirary/path/to/example/core.js
but :output-dir is "out", how is the IJavaScriptEnv supposed to reliably calculate the
path from project root. :asset-path can help here but asset path is not a required build option.

Because all clients can easily calculate the output-dir relative path with js/goog.basePath:
output-dir_relative_path = path - ( js/goog.basePath - #"goog/$")

I think it may be more general to take an output-dir relative path from base like so:

(print-mapped-stacktrace
  [{:file "hello/core.js" <-- path from output dir
    :function "first"
    :line 2
    :column 1}]))

I'm solving this problem in figwheel by calculating the output-dir_relative_path client side
then having the IjavaScriptEnv prepend the :output-dir.

Since the web is our most common eval environment it would be nice not to force clients to process stack lines on both sides of the divide.



 Comments   
Comment by David Nolen [ 10/Feb/15 9:06 AM ]

Thanks I'm starting to see the problem and will look into it. Will definitely get this sorted out before the next release as I would like everyone to just reuse this work.

Comment by Bruce Hauman [ 10/Feb/15 10:55 AM ]

I'm happy to contribute a default stackline parser.

One more thing, it may be nice to allow for stack line parse failures, in order to allow for incredible pace at which these environments are evolving. Either have print-mapped-stacktrace accept a vector of maps(successful stack line parses) and strings(unsuccessful parses). Or accept an extra key in the stack-line map like :orig-stack-line.

print-mapped-stacktrace can then just print out the unsuccessful parses as they are.

Comment by David Nolen [ 10/Feb/15 11:34 AM ]

We're not going to do default stackline parser. Will look into the path stuff for sure.

Comment by David Nolen [ 10/Feb/15 11:41 PM ]

Ok so I looked into this some more, one problem is that IJavaScriptEnv methods beyond -setup do not take compiler opts. This makes it impossible for REPLs to correctly generate the :output-dir relative paths at the time the exception is detected without introducing some state to store opts. I think we should introduce a new protocol IMapStacktrace which REPLs may optionally implement. This protocol will consist of a single method, it will receive the original stacktrace string and compiler opts. It's implicit that REPLs do not return canonical stacktraces under any other conditions - only strings. Feedback welcome.

Comment by Bruce Hauman [ 11/Feb/15 8:14 AM ]

It seems to me this focuses on the difficulty of getting output-dir, but exacerbates the path parsing problem for browser based IJavaScriptEnvs'

This is a choice that would prevent the client side(browser) from parsing canonical traces and foist it all on IJavaScriptEnv. Why make that decision for implementors?

IJavaScriptEnv doesn't have js/goog.basePath and :asset-path isn't a required build option and it really shouldn't be for folks to get mapped stacktraces. To me this increases the difficulty of going from an arbitrary client-side path to a project root or output-dir relative path because we are forcing the computation to be made in an environment that doesn't have an very important piece of information.

In my proposal I was thinking that leveraging the client side knowledge of -basePath is absolutely key.

I much much prefer the code the way it is now. At least implementors have the flexibility to decide how and where to parse stack traces for given environments.

As far as making the :output-dir available to IJavaScriptEnv. I'm providing to the the FigwheelEnv record constructor. An IJavaScript can only be associated with one build anyway.

David, thanks for looking into this.

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

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

Comment by Bruce Hauman [ 11/Feb/15 8:18 AM ]

I really didn't want that to sound combative. But reading it back it does, I'm sorry about that. I'm not really feeling that way. It's just early for me.

Comment by Mike Fikes [ 11/Feb/15 10:58 AM ]

Confirmed working with Node REPL, Nashhorn REPL (not yet participating, but still works), and the Ambly REPL.





[CLJS-1024] Double analysis warnings for dependencies in JARs Created: 10/Feb/15  Updated: 10/Feb/15

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 there is a problem with a CLJS source in a JAR, warnings will be emitted twice - once when analyzing the source in the JAR and again when the file is compiled to the output directory. Because the path has changed we don't realize we have already seen it. This probably just needs minor tweaks so that we drop warnings from the second pass.






[CLJS-1023] macro-autoload-ns? and ns-dependents need to throw on cyclic dependencies Created: 09/Feb/15  Updated: 09/Feb/15  Resolved: 09/Feb/15

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

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


 Description   

Currently both will result in stack overflow as they run independently of the existing cyclic check in the parse ns portion of the analyzer.



 Comments   
Comment by David Nolen [ 09/Feb/15 6:29 PM ]

fixed https://github.com/clojure/clojurescript/commit/014524d9f04f6262dce7af13cb47c0c29db07908





[CLJS-1022] Concatenate foreign dependencies safely Created: 08/Feb/15  Updated: 08/Feb/15  Resolved: 08/Feb/15

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

Type: Defect Priority: Major
Reporter: Shaun LeBron Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 1022.patch    

 Description   

If there is a comment on the last line of a foreign JS dependency, it will result in the first line of the concatenated code to be commented out.

Including a patch to join foreign JS dependencies with newlines between them.



 Comments   
Comment by David Nolen [ 08/Feb/15 9:45 AM ]

fixed https://github.com/clojure/clojurescript/commit/0657b0a3ab2a606bb114620d0f66265158a7bc39





[CLJS-1021] REPL source map support needs to treat JS files correctly Created: 07/Feb/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

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

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


 Comments   
Comment by David Nolen [ 07/Feb/15 8:17 PM ]

fixed https://github.com/clojure/clojurescript/commit/57277cff5ccde7cac6fe3d2784db2275ab409542





[CLJS-1020] REPL source map support off by one Created: 07/Feb/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

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

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


 Comments   
Comment by David Nolen [ 07/Feb/15 8:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/57277cff5ccde7cac6fe3d2784db2275ab409542





[CLJS-1019] REPL source map caching support Created: 07/Feb/15  Updated: 07/Feb/15

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   

Need smarter caching for better performance. Probably something based on last modified times.






[CLJS-1018] Add support for cljs.core/*e Created: 07/Feb/15  Updated: 11/Feb/15  Resolved: 07/Feb/15

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

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

Attachments: Text File CLJS-1018-better-format.patch     Text File CLJS-1018.patch    

 Description   

ClojureScript supports *1, *2, *3 in the REPL just like Clojure REPLs. But, it is lacking support for *e, the last exception caught.



 Comments   
Comment by Mike Fikes [ 07/Feb/15 1:05 PM ]

In the Clojure REPL, here is an example showing that the exception is stored and printed as a non-readable:

user=> (first 1)

IllegalArgumentException Don't know how to create ISeq from: java.lang.Long  clojure.lang.RT.seqFrom (RT.java:505)
user=> *e
#<IllegalArgumentException java.lang.IllegalArgumentException: Don't know how to create ISeq from: java.lang.Long>

I experimented in the Ambly ClojureScript REPL with a hypothetical cljs.user/*e (wrong namespace) and found that a non-readable could be captured if the JS Error is placed in this dynamic var:

ClojureScript:cljs.user> (def ^:dynamic *e nil)
nil
ClojureScript:cljs.user> (first 1)
Error: 1 is not ISeqable
(... trace omitted)
nil
ClojureScript:cljs.user> *e
#<Error: 1 is not ISeqable>

I did this by revising the internals of the REPL impl near here to stash the exception in the var, with a bit of code.

self.jsContext[@"cljs"][@"user"][@"_STAR_e"] = self.jsContext.exception;

Of course, this isn't the right place at all to implement something like this, but it experimentally shows that such a thing would be feasible.

There is code that imperatively shifts down *1, *2, *3, in the JavaScript that is sent to the execution environment when a form is evaluated. Perhaps that code is a place that could be revised to catch exceptions, stash them in *e, and re-throw. (I don't know enough about JavaScript to know if that is feasible or performant.)

I suppose any solution that would work would simply need access to the actual JavaScript exception object so that it can assign it to *e.

Comment by Mike Fikes [ 07/Feb/15 3:00 PM ]

Adding a try-catch-throw is relatively simple and the attached patch works:

ClojureScript:cljs.user> (doc *e)
-------------------------
cljs.core/*e
()
  bound in a repl thread to the most recent exception caught by the repl
nil
ClojureScript:cljs.user> (first 1)
Error: 1 is not ISeqable
cljs.core/seq (.ambly_jsc_repl/cljs/core.cljs:707:4)
cljs.core/first (.ambly_jsc_repl/cljs/core.cljs:731:6)
nil
ClojureScript:cljs.user> *e
#<Error: 1 is not ISeqable>

This causes a try-catch-throw to be sent to the JavaScriptEngine:

cljs.core.pr_str.call(null,(function (){try{var ret__3518__auto__ = cljs.core.first.call(null,(1));
cljs.core._STAR_3 = cljs.core._STAR_2;

cljs.core._STAR_2 = cljs.core._STAR_1;

cljs.core._STAR_1 = ret__3518__auto__;

return ret__3518__auto__;
}catch (e4844){var e__3519__auto__ = e4844;
cljs.core._STAR_e = e__3519__auto__;

throw e__3519__auto__;
}})())
Comment by Mike Fikes [ 07/Feb/15 3:14 PM ]

The last attachment had poorly formatted code in it. This new attachment fixes that.

Comment by David Nolen [ 07/Feb/15 8:22 PM ]

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

Comment by Mike Fikes [ 11/Feb/15 11:24 AM ]

This "catch and rethrow" strategy is not completely transparent in the Node.js REPL (using Node v0.12.0). In particular, an indicator of the throw site appears in the description:

ClojureScript:cljs.user> (ffirst 1)
repl:12
throw e__3553__auto__;
      ^
Error: 1 is not ISeqable
    at Object.seq (.../cljs/core.cljs:727:20)
    at Object.first (.../cljs/core.cljs:736:16)
    at ffirst (.../cljs/core.cljs:1155:11)

I don't recall seeing this with the previous Node version.

The stackframes themselves are fortunately not modified, which this SO claims is the proper behavior; the problem described in this comment is limited only to the description.

Comment by David Nolen [ 11/Feb/15 12:16 PM ]

I think we can live this for now





[CLJS-1017] Support :main for :advanced builds Created: 06/Feb/15  Updated: 06/Feb/15

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   

This would use the supplied entry point to compute the build instead of looking at everything in the source directory.






[CLJS-1016] Make more marker during printing configurable Created: 06/Feb/15  Updated: 10/Feb/15  Resolved: 10/Feb/15

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

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

Attachments: Text File cljs_1016_more_effective.patch     Text File cljs_1016_new_attempt.patch     Text File cljs_1016_no_unicode.patch     Text File cljs_1016.patch     Text File minimal_breaking_change.patch    
Patch: Code and Test

 Description   

print-length controls how many items will be printed by sequential printer. If there are more items available it is communicated by printing "..." the last item of a sequence. For example:

(binding [*print-length* 2] (str [1 2 3 4 5 6 7 8 9 0]))
=> "[1 2 ...]"

I want this "..." string (more-marker) to be configurable via printing opts. This is useful for cljs-devtools. For complex structures I call clojurescript printer. I can set more-marker to some less likely unicode value. And then use it for detection if all values were actually printed or not. This way I don't have to understand internal structure. I simply observe presence of my marker. And can render disclosure triangle for user to dig deeper into the structure.



 Comments   
Comment by David Nolen [ 06/Feb/15 6:09 PM ]

When I try to apply this patch many tests fail. The failures are pretty strange as they don't seem to have anything to do with printing.

Comment by Antonin Hildebrand [ 06/Feb/15 6:13 PM ]

Tests passed on my machine, but I have only spidermonkey engine available.

Maybe unicode characters in the patch could be an issue? My tests contain … which is a unicode char.

Comment by Antonin Hildebrand [ 06/Feb/15 6:14 PM ]

Also now when I look at that let binding second time, maybe I should just inline the :more-marker lookup in places where it is used. Now the lookup is done every time even when entering the method.

Comment by Antonin Hildebrand [ 06/Feb/15 6:17 PM ]

consider this one instead, lookup is done only when needed

Comment by David Nolen [ 06/Feb/15 6:19 PM ]

The new patch does not apply to master.

Comment by David Nolen [ 07/Feb/15 8:24 PM ]

Patch still does not apply. Let's drop the unicode.

Comment by Antonin Hildebrand [ 08/Feb/15 9:13 AM ]

FYI: I did a clean checkout of clojurescript repo. Followed https://github.com/clojure/clojurescript/wiki/Running-the-tests.

./scripts/test does not work for me. All tests pass or it spills random errors/failures (even without this patch applied). When I apply this patch. And try to run ./scripts/test the generated javascript in builds/out-adv does not contain my changes. Of course, I was looking for unique strings not function names.

./scripts/test-simple works as expected and reflect my code changes (for example when I break tests)

Comment by David Nolen [ 08/Feb/15 9:41 AM ]

I just tried a clean checkout of the repository and I cannot replicate the test failures.

Comment by Antonin Hildebrand [ 08/Feb/15 9:56 AM ]

FYI: with CLJS-1010 I have two clojurescript repos checked out. Because I was trying to do your workflow of applying my patches with "git am" just to test them. And I observed that even script/test-simple does not work in one of them. I deliberately broke the tests and script/test-simple didn't reflect my change. In other repo the same change worked as expected.

The broken repo is not totally clean. It was my original one where I was editing files with unicode characters. Maybe there is something bad there. But I wonder what, because since then I did "git reset --hard your-master".

Anyways. I will try to download it from scratch and test it again.

Comment by David Nolen [ 09/Feb/15 7:34 AM ]

The latest patch applies but I still get test failures:

Testing cljs.core-test
builds/out-adv/core-advanced-test.js:605: Error: No matching clause: :fn
],0)));default:throw Error([mb("No matching clause: "),mb(Nr(D,F))].join(""));
                                                                    ^
Error: No matching clause: :fn
    at Error (native)
    at builds/out-adv/core-advanced-test.js:605:194
    at builds/out-adv/core-advanced-test.js:605:206
    at Er (builds/out-adv/core-advanced-test.js:596:78)
    at builds/out-adv/core-advanced-test.js:3739:1
    at builds/out-adv/core-advanced-test.js:3927:3
Comment by Antonin Hildebrand [ 09/Feb/15 10:17 AM ]

I tried to find minimal case to trigger the problems with core-advanced-test.js. The change to core.cljs can be as small as replacing the first one of those "..." in pr-sequential-writer with '(get opts :x "...")' (no changes to test files).

When trying different versions by rewriting my code, errors reported by script/test were changing. I tried to make some sense of them by debugging them in chrome devtools, but I didn't find any common traits. They seem to me to be at random places. Advanced compilation unfortunately prevents me to reason about them effectively.

I've got one new observation: I added :static-fns true in scripts/test and problems went away. I didn't find any documentation about static-fns tough.

Comment by David Nolen [ 09/Feb/15 5:25 PM ]

For debugging advanced builds enable :pseudonames true, :pretty-print true compiler options. This should clarify where things are going wrong.

Comment by Antonin Hildebrand [ 10/Feb/15 2:27 PM ]

This test suite failures are not related to my patch. This is some issue in the script/test or closure compiler itself and I was unlucky to trigger it.

Also I assume there is some caching issue, because my results are not stable. Clean checkout without code
modifications script/test passes without errors. Then I change core.cljs with a trivial change. And script/test spills bunch of unrelated errors. I revert the code back. Run script/test and I get the same errors. This is really puzzling.

When I set :static-fns true in script/test all problems go away. I'm afraid this is not something I could solve on my side.

Comment by Antonin Hildebrand [ 10/Feb/15 2:58 PM ]

Tested with current tip 2711d029:
https://github.com/clojure/clojurescript/commit/2711d02948c103e33ab4fd1431881edcd65095ef

Full protocol with clean checkout:
https://gist.github.com/darwin/8e9d75a4e810d6f2d9e3

btw. In this case I was unable to reproduce unstable results. When I revert back. The script/test reflect it and pass all tests.

Comment by David Nolen [ 10/Feb/15 3:44 PM ]

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





[CLJS-1015] Closure libraries using custom annotation generate warnings during compilation Created: 06/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Julien Eluard Assignee: Julien Eluard
Resolution: Completed Votes: 0
Labels: None

Attachments: File CLJS-1015.diff    

 Description   

Closure libraries defining their own JSDoc annotation generate warnings during compilation. This forces library consumers to rely on {:closure-warnings {:non-standard-jsdoc :off}} to hide them.

I can see 2 options to improve this:

See more context here.

What do you think?



 Comments   
Comment by David Nolen [ 06/Feb/15 9:57 AM ]

Patch for option 1 welcome.

Comment by Julien Eluard [ 06/Feb/15 4:57 PM ]

I just attached a patch for option #1.

Comment by David Nolen [ 06/Feb/15 5:06 PM ]

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





[CLJS-1014] Support Closure Defines under :none Created: 06/Feb/15  Updated: 01/Mar/15  Resolved: 01/Mar/15

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

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


 Description   

Because Google Closure never runs under :none Closure Defines do not work.



 Comments   
Comment by David Nolen [ 06/Feb/15 9:51 AM ]

This is easy to make work with :main. Can probably be made to work without as well?

Comment by David Nolen [ 01/Mar/15 9:24 AM ]

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





[CLJS-1013] track goog closure defines Created: 06/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

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


 Description   

Currently if you write a conditional a closure define it will be wrapped in cljs.core.truth_ call. This is unintuitive. The compiler should track all defines and emit the correct code.



 Comments   
Comment by David Nolen [ 06/Feb/15 9:45 AM ]

closure defines need not be boolean





[CLJS-1012] When *print-length* set to 0 the behaviour is inconsistent with Clojure Created: 06/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

Type: Defect Priority: Minor
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print

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

 Description   

https://github.com/darwin/clojurescript/commit/fc7c91ca009528ac54cd020509b49acea4eecca9

I don't care that much about Clojure. I need this to work for cljs-devtools:
https://github.com/binaryage/cljs-devtools/commit/dcbfe1ba1aef78f300ef8fbf58c8fd1e77dd2450



 Comments   
Comment by David Nolen [ 06/Feb/15 8:48 AM ]

Patch welcome for this.

Comment by Antonin Hildebrand [ 06/Feb/15 8:53 AM ]

The patch is here:
https://github.com/darwin/clojurescript/commit/fc7c91ca009528ac54cd020509b49acea4eecca9

based on CLJS-1010 because it uses :more-text from opts. Do you want me to sign the CLA?

Comment by Antonin Hildebrand [ 06/Feb/15 8:56 AM ]

But I can fork current tip and make it independent. And then rebase CLJS-1010 on top of this.

Comment by David Nolen [ 06/Feb/15 9:07 AM ]

Patches need to be added to tickets. And yes re: CLA.

Comment by Antonin Hildebrand [ 06/Feb/15 9:44 AM ]

Got it. Patch rebased on top of master and added. I will submit :more-text change in a separate CLJS.

Comment by David Nolen [ 06/Feb/15 5:08 PM ]

fixed https://github.com/clojure/clojurescript/commit/20778cd528167910031bffcc77624628c02c9e8f





[CLJS-1011] Unified REPL stacktrace reporting Created: 05/Feb/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

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

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


 Description   

Every REPL should take the original error stack string from the JavaScript target and parse into a canonical vector representation containing function name, file, and line, column. Then the core REPL architecture can provide source mapping functionality universally.



 Comments   
Comment by David Nolen [ 07/Feb/15 10:06 PM ]

fixed in master https://github.com/clojure/clojurescript/commit/ae62c6cfca71832e9ec829f073503ff892a6edd8





[CLJS-1010] Printing hook for cljs-devtools Created: 05/Feb/15  Updated: 10/Feb/15  Resolved: 10/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Antonin Hildebrand Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: print

Attachments: Text File cljs_1010_new_hope.patch     Text File cljs_1010.patch    
Patch: Code

 Description   

cljs-devtools[1] should provide pretty-printing support for ClojureScript values in javascript console under blink-based browsers.

We could just print dumb strings using pr-str, but we want to be smarter. Basically we need a way how hook into pr-writer to re-use ClojureScript printing machinery (including calls to IPrintWithWriter) when doing our pretty-printing. The main goal is to get this functionality "for free" and to be future-proof. Any future code which will call pr-writer, pr-sequential-writer, print-map or similar should be supported by cljs-devtools automatically. Any future or user code implementing IPrintWithWriter protocol should be supported as well, at least as string tokens. Please read the description in the patch[2].

I have a working implementation[3] of cljs-devtools which depends on this patch and results look good. My approach was to augment calls pr-writer:
1) catch calls for simple values I know how to render myself and fall back to original pr-writer for more complex ones
2) (optionally) stop recursion by printing some placeholder or abbreviated value
3) (optionally) post-process results returned from original pr-writer and wrap them in references to live objects (devtools will render disclosure triangles for further user-initiated expansion)

[1] https://github.com/binaryage/cljs-devtools
[2] https://github.com/darwin/clojurescript/commit/a3c96832df6d8893f2066afb4441fa9e9ee629ad
[3] https://github.com/binaryage/cljs-devtools/blob/6dd12324938d9efcb0a7245f5ff0e2db35d1bf56/src/devtools/format.cljs#L132-L147



 Comments   
Comment by Antonin Hildebrand [ 06/Feb/15 11:02 AM ]

Apply after CLJS-1016

Comment by Antonin Hildebrand [ 08/Feb/15 9:48 AM ]

rebased on top of latest CLJS-1016 patch

Comment by David Nolen [ 10/Feb/15 3:46 PM ]

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





[CLJS-1009] Allow deps.cljs to declare a foreign lib as remote Created: 05/Feb/15  Updated: 21/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When using things like the Google Maps JS API [1] the Javascript that is required can't be bundled inside a jar as it depends on the used API key.

To be able to provide externs for those kind of libraries there should be a way to declare them as "remote" in deps.cljs.

[1] https://developers.google.com/maps/documentation/javascript/tutorial






[CLJS-1008] Browser repl doesn't find upstream dependencies. Created: 05/Feb/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

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

Type: Defect Priority: Minor
Reporter: Brian Jenkins Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl
Environment:

MacOS 10.9.5
Running a repl through cider & emacs.

project.clj attached.


Attachments: Text File fix-upstream-deps-from-repl.patch     File project.clj    
Patch: Code

 Description   

I found that when running piggieback from the repl like this:

```clojure
(defn piggieback-repl []
(cemerick.piggieback/cljs-repl :repl-env (cljs.repl.browser/repl-env :port 9000)) )
```

get-upstream-deps doesn't find anything.

This prevents me from being able to use piggieback with Om's new cljsjs dependency on React.

Comment on get-upstream-deps says

```
Should be run in the main thread. If
not, pass (java.lang.ClassLoader/getSystemClassLoader) to use the
system classloader.
```

I guess the repl isn't the main thread?



 Comments   
Comment by David Nolen [ 07/Feb/15 10:05 PM ]

fix https://github.com/clojure/clojurescript/commit/f311ebd121bf2385efcfba6f7bfb07251cf812f1





[CLJS-1007] Add :recompile-dependents knob Created: 05/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

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


 Description   

This is to support disabling the default. This is useful in live coding scenarios.



 Comments   
Comment by David Nolen [ 06/Feb/15 9:08 AM ]

fixed https://github.com/clojure/clojurescript/commit/65a8973e2db8a9bd48aac55d310e536ebc0dcd75





[CLJS-1006] Implicit dependency of clojure.browser.repl on cljs.repl Created: 05/Feb/15  Updated: 05/Feb/15  Resolved: 05/Feb/15

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

Type: Defect Priority: Minor
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File 1006.patch    

 Description   

The clojure.browser.repl/connect function monkey patches goog.require with :optimizations :none. The substituted version uses goog.basePath-relative URLs to retrieve assets, one of which is cljs.repl (the repl-connection callback returns "goog.require('cljs.repl')"). Therefore clojure.browser.repl has an implicit dependency on cljs.repl, which will not be able to be located if the :output-dir of application compilation (eg cljsbuild) and that passed to cljs.repl/repl* are not one in the same.



 Comments   
Comment by David Nolen [ 05/Feb/15 7:13 AM ]

fixed https://github.com/clojure/clojurescript/commit/576fd1c70933fc91180c769cb639c262b7c79071





[CLJS-1005] Browser REPL creates "out" directory no matter what Created: 05/Feb/15  Updated: 05/Feb/15  Resolved: 05/Feb/15

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

Type: Defect Priority: Minor
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File 1005.patch    

 Description   

Steps to reproduce:

$ lein new mies test-brepl
$ cd test-brepl
$ sed -i 's/:output-dir "out"/:output-dir "not-out"/' scripts/brepl.clj
$ ./scripts/brepl # <Ctrl-c> <Ctrl-c>
$ ls -la

Expected: Only "not-out" and ".repl-0.0-XXXX" directories should be created for compiled javascript.

Actual: "out" directory is also created.



 Comments   
Comment by David Nolen [ 05/Feb/15 7:13 AM ]

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





[CLJS-1004] Single segment namespaces are not properly loaded Created: 04/Feb/15  Updated: 05/Feb/15  Resolved: 05/Feb/15

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

Type: Defect Priority: Minor
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File 1004-single-segment-ns-warning.diff    

 Description   

When having a namespace app and supplying it to the compilers :main option it is not properly loaded by the resulting Javascript file (output-to).

A test case is in this repository: https://github.com/martinklepsch/single-segment-cljs-ns

I'm not sure what the error here is exactly. The files that get loaded look fine and I don't see an issue with how the dependency is registered:

{{goog.addDependency("../app.js", ['app'], ['cljs.core', 'om.dom', 'om.core']);}}

Still when later goog.require("app"); is called it returns undefined and has no effect on the DOM.

Happy to look into this if someone can provide me with some pointers.



 Comments   
Comment by Martin Klepsch [ 05/Feb/15 5:46 AM ]
goog.isProvided_ = function(name) {
    return !goog.implicitNamespaces_[name] &&
        goog.isDefAndNotNull(goog.getObjectByName(name));
  };

When it's checked if a namespace is already provided goog.getObjectByName is called.
When there is a DOM node with the same id that is passed to goog.getObjectByName
it returns the DOM node and assumes everything that has been required is present.
Therefore the dependency resolution process stops early.

Comment by Martin Klepsch [ 05/Feb/15 5:49 AM ]

This patch adds a warning when using namespaces with only a single segment (i.e. no dot).

WARNING: Using single segment namespaces (app) is discouraged. at line 1 ssrc/app.cljs

Comment by David Nolen [ 05/Feb/15 7:08 AM ]

Didn't notice the patch! Fixed in master.

https://github.com/clojure/clojurescript/commit/dc47f1fa1483871641c34f765034efedb7238f28





[CLJS-1003] Cannot pass custom env to run-tests Created: 04/Feb/15  Updated: 05/Feb/15  Resolved: 04/Feb/15

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

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


 Description   

Calling (cljs.test/run-tests (cljs.test/empty-env :my-reporter)) does not actually pass the environment with a custom reporter key onwards. I think the problem lies here: https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/test.clj#L257

We should pass ~env-or-ns argument to cljs.test/test-ns call, because now the single-arg version internally creates a new empty environment (see: https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/test.clj#L309).



 Comments   
Comment by Kimmo Koskinen [ 04/Feb/15 9:01 AM ]

Seems that adding ~env-or-ns to cljs.test/test-ns call in test-ns isn't enough. Summary is still reported with the :cljs.test/default reporter.

Comment by David Nolen [ 04/Feb/15 2:35 PM ]

Kimmo do you have a minimal example of something that you think should work so that we can use it as a test? Thanks.

Comment by Kimmo Koskinen [ 04/Feb/15 2:49 PM ]

Could this be of help? https://gist.github.com/viesti/bba5c88abbb2f6ecb914

The call to (test/run-tests (test/empty-env :karma)) should print a message "Testing with Karma myapp.test" instead of "Testing myapp.test".

Comment by David Nolen [ 04/Feb/15 6:38 PM ]

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

Comment by Kimmo Koskinen [ 05/Feb/15 2:01 AM ]

Thanks for that fast reply! I tried the patch quickly, and report method is called on the :begin-test-ns phase correctly. However, on the :summary phase, my custom report method isn't called.

I updated the gist: https://gist.github.com/viesti/bba5c88abbb2f6ecb914. It should print "All done" at end of the test, instead of "Ran 1 tests containing 1 assertions.
0 failures, 0 errors.".

Comment by David Nolen [ 05/Feb/15 6:53 AM ]

fixed in master https://github.com/clojure/clojurescript/commit/647f775f01fad563a9c7b481a7e15273a8cff884

Comment by Kimmo Koskinen [ 05/Feb/15 7:22 AM ]

Thanks!





[CLJS-1002] `cljs.closure/get-upstream-deps`: Allow caller to specify classloader to use Created: 03/Feb/15  Updated: 03/Feb/15  Resolved: 03/Feb/15

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

Type: Enhancement Priority: Minor
Reporter: Moritz Ulrich Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: patch
Environment:

Clojurescript, commit d124f71


Attachments: Text File 0001-cljs.closure-get-upstream-deps-Add-optional-CLASSLOA.patch     Text File 0001-cljs.closure-get-upstream-deps-Add-optional-CLASSLOA.patch     Text File 0002-cljs.closure-get-upstream-deps-Correct-arity-order-r.patch    
Patch: Code

 Description   

Some users of cljs.closure/get-upstream-deps don't run it from the main thread. Because it uses the current thread's classloder, it just returns an empty list.

This patch allows callers to specify the classloader to use, for example (java.lang.ClassLoader/getSystemClassLoader) (which fixes the function for weasel)



 Comments   
Comment by Moritz Ulrich [ 03/Feb/15 4:54 PM ]

Combined patch, should apply to d124f71b4fa7dd3ceecb064eaf5a2f7392e738e1

Comment by David Nolen [ 03/Feb/15 4:58 PM ]

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





[CLJS-1001] metadata leakage Created: 03/Feb/15  Updated: 10/Feb/15  Resolved: 10/Feb/15

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

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


 Description   

Using this template https://github.com/hellofunk/hellofunk-lein-template/blob/master/resources/leiningen/new/hellofunk/project.clj results in producing metadata leakage on maps at a minimum.



 Comments   
Comment by Andrew S [ 03/Feb/15 1:39 PM ]

Not sure this is relevant but if the :source-map line is removed, the issue remains.

Older versions of clojurescript do not have this "leakage" of file paths in the compiled JS.

Here is an example of the leakage from that template:

return (new weirdpathissue.core.t11112(owner,app,main,new cljs.core.PersistentArrayMap(null, 5, [new cljs.core.Keyword(null,"end-column","end-column",1425389514),33,new cljs.core.Keyword(null,"end-line","end-line",1837326455),14,new cljs.core.Keyword(null,"column","column",2078222095),7,new cljs.core.Keyword(null,"line","line",212345235),11,new cljs.core.Keyword(null,"file","file",-1269645878),"/Users/andrew/REPOS/THROWAWAYS/weirdpathissue/src/cljs/weirdpathissue/core.cljs"], null)));

Comment by Immo Heikkinen [ 04/Feb/15 1:18 AM ]

Seems to be related to reify. Here's minimal repro case:

(defprotocol Foo (foo [_]))

(defn new-foo []
  (reify Foo (foo [_])))

(new-foo)
Comment by David Nolen [ 10/Feb/15 6:30 AM ]

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





[CLJS-1000] Source mapping helpers Created: 03/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

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


 Description   

In some debugging scenarios it would be nice to provide a Clojure REPL api for mapping a JS line to the original ClojureScript line and vice versa.



 Comments   
Comment by David Nolen [ 06/Feb/15 5:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/751a3eb3d5c8fcf90fe97ae6395105640cb585a3





[CLJS-999] JS file concatanation in preamble should place a newline between each file Created: 03/Feb/15  Updated: 06/Feb/15  Resolved: 06/Feb/15

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

Type: Defect Priority: Minor
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

Some JavaScript files one might wish to include in a preamble are not well formed, in the sense that they end without a semicolon and without a newline.

These files work fine as long as they are in their own <script/> tag, but will cause invalid JavaScript when concatenated with any other file, as they are in a ClojureScript :preamble.

It should be trivially possible for ClojureScript to be defensive about this type of error, by inserting a newline character between concatenated files, in the process of building the preamble.



 Comments   
Comment by David Nolen [ 03/Feb/15 10:35 AM ]

We already do this.

Comment by David Nolen [ 06/Feb/15 5:38 PM ]

We already do this. Need an example of something that doesn't work though I suspect this is JS parsing issue.





[CLJS-998] Nashorn REPL does not support require special fn Created: 02/Feb/15  Updated: 21/Feb/15  Resolved: 21/Feb/15

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

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


 Description   

Need to monkey patch goog.require as we have done elsewhere.



 Comments   
Comment by David Nolen [ 21/Feb/15 9:42 AM ]

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





[CLJS-997] Nashorn repl bindings Created: 02/Feb/15  Updated: 02/Feb/15  Resolved: 02/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Pieter van Prooijen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Jdk 8 or higher


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

 Description   

Hello,

I've created a repl binding to the Nashorn script engine, similar to the Rhino and node.js repls. Although Nashorn is slow loading javascript code, it still has a reasonable startup time (about 10s with fast trampolines, cache-analysis enabled and AOT compiled libraries on a four year old laptop).

The code could be useful for people wanting to minimize the number of external dependencies in their project and could also offer a starting point when trying out with server-side Clojurescript using Nashorn.
The file contains comments on how to setup the repl under leiningen and nrepl / piggieback.

Kind regards,

Pieter van Prooijen

(Patch follows shortly)



 Comments   
Comment by David Nolen [ 02/Feb/15 12:50 PM ]

Pieter, thanks. In order for the patch to be applied you must sign the Contributor Agreement - http://clojure.org/contributing.

Comment by Pieter van Prooijen [ 02/Feb/15 12:56 PM ]

Patch against 2761

Comment by Pieter van Prooijen [ 02/Feb/15 1:03 PM ]

David, I've signed the agreement.

Comment by David Nolen [ 02/Feb/15 2:57 PM ]

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





[CLJS-996] .indexOf method(in javascript) returns wrong result when called with keyword array Created: 02/Feb/15  Updated: 02/Feb/15  Resolved: 02/Feb/15

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

Type: Defect Priority: Major
Reporter: sorrow17 Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, cljs
Environment:

Windows, clojurescript-2755



 Description   

The following expression should return "1":

(.indexOf (to-array [:a :b :c]) :b)

but actually the result is -1.
Note that this method call behaves normally with other type of array:

>(.indexOf (to-array ["a" "b" "c"]) "b")
>1

So I guess there might something wrong with the type Keyword.
Anyone have a look at this?



 Comments   
Comment by Francis Avila [ 02/Feb/15 8:18 AM ]

Array.prototype.indexOf uses strict equality (js === operator) to compare search item with array items. Keywords are Objects, and the same keyword-equal object may not be the same identical object. Non identical objects never compare strictly equal in js and there is nothing that can be done to change this.

Use something other than indexOf to find your item. You need to compare items with the cljs = function or with keyword-equal?

Comment by David Nolen [ 02/Feb/15 9:40 AM ]

This is not a bug.





[CLJS-995] deps.js contains references to js files not found in out/ Created: 01/Feb/15  Updated: 04/Feb/15  Resolved: 04/Feb/15

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

Type: Defect Priority: Minor
Reporter: Mike Thompson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None
Environment:

Version 2665 was okay. Version 2719 shows the problem.



 Description   

When using ":optimisations :none" ...

In 2665 and prior, the list of goog.Dependency entries in deps.js appears to have been filtered/trimmed in some way, and it matched the js contents of out/. In effect, deps.js did not contain references to any js file which didn't exist in out/

As of 2719 release, I'm seeing a really large deps.js file which contains goog.Dependency entries referencing js files which are not present in out/



 Comments   
Comment by Mike Thompson [ 01/Feb/15 8:04 PM ]

< I can't seem to edit my ticket description. Puzzling>

I should stress that this will NOT cause a problem generally. If all you do is goog.require() a root namespace, then the dependencies are still correct, and everything works.

I only noticed this issue because, as you know, I am doing this:

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

And, whereas that worked fine in 2665 (and prior), in 2719 (and later) it results in a lot of js files being imported which do not exist (in out/). The non-existence of js files referenced in <script>s causes copious console errors, which is not a showstopper, but it is untidy.

So, my "hack" still works, albeit now with copious warnings, but it just doesn't seem right that deps.js contains code that references non-existent js files.

Comment by David Nolen [ 01/Feb/15 10:47 PM ]

The previous behavior caused other problems - deps.js would not get updated consistently. I would take a patch that is guaranteed to keep deps.js minimal and up to date.

Comment by Thomas Heller [ 02/Feb/15 3:12 AM ]

@Mike: Instead of doing a require ALL you could just requires the namespaces that require cljs.test (or the cemerick variant).

{{goog.dependencies_.nameToPath["cljs.test"]}} gives you the filename {{goog.dependencies_.requires[name]}} gives you everything that requires it. If you goog.require those it will bring in all other dependencies. That might work arround the problem with deps.js.

Comment by Mike Thompson [ 02/Feb/15 3:15 AM ]

I've gone through all the checkins between the two releases, and it looks as if this is the commit which changed the behaviour:
https://github.com/clojure/clojurescript/commit/e726c22fcc77bfd38cc17a2f0d8349c43c83add5
made when addressing CLJS-963

I've had a look at http://dev.clojure.org/jira/browse/CLJS-963 but details about the problem to be avoided are scant.

I'll keep looking.

Comment by Mike Thompson [ 02/Feb/15 3:16 AM ]

Thanks Thomas! Will experiment.

Comment by Stuart Mitchell [ 04/Feb/15 5:16 PM ]

Replace the hack

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

with this

//find out what requires cljs.core
 for(var key in goog.dependencies_.requires) {
     if (goog.dependencies_.requires.hasOwnProperty(key)) {
         if (goog.dependencies_.requires[key]["cljs.core"]) {
             //as key is a path find its namespace
             for (var namespace in goog.dependencies_.pathToNames[key])
                 goog.require(namespace);
         }
     }
 }
Comment by David Nolen [ 04/Feb/15 6:06 PM ]

Not changing this for now.

Comment by Mike Thompson [ 04/Feb/15 8:30 PM ]

Hack in use has been modified as per Thomas' and Stu's suggestion. Thanks!





[CLJS-994] print a warning when :externs file paths can't be found. Created: 30/Jan/15  Updated: 01/Feb/15

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

Type: Enhancement Priority: Trivial
Reporter: Crispin Wellington Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs, enhancement, errormsgs, patch,
Environment:

Linux 64bit

java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)


Attachments: Text File clojurescript-extern-missing-warning.patch    
Patch: Code

 Description   

clojurescript silently ignores missing externs files possibly leading a developer to chase their tail.

Presently it can be very confusing using advanced compilation if you have made a mistake in the path name of one of your :externs files. This patch makes the compiler print a warning on stderr so you can quickly determine the cause of the broken advanced compilation output.

As a side effect, when doing a basic lein-cljsbuild a warning is always printed:

```
WARNING: js resource path closure-js/externs does not exist
```

This is because lein-cljsbuild quietly adds this extra path to your :externs listing without you knowing.



 Comments   
Comment by David Nolen [ 31/Jan/15 1:59 PM ]

You need to bind *out* to *err*, or just print to it directly a la cljs.util/debug-prn.

Comment by Crispin Wellington [ 31/Jan/15 7:30 PM ]

I did bind out to err. Check the patch.

Comment by David Nolen [ 01/Feb/15 12:30 PM ]

Crispin, oops sorry you are correct. Thanks.





[CLJS-993] binding macro returns non-nil with empty body Created: 30/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

Type: Defect Priority: Minor
Reporter: Stefano Pugnetti Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: binding, test
Environment:

Ubuntu Linux 12.04
Clojurescript 0.0-2740



 Description   

In Clojure the binding macro returns nil when called with an empty body. In Clojurescript it returns a non empty object.

In this repository
https://github.com/stepugnetti/binding-issue
I've put a simple project in which the two behaviors are compared. The same code

(defn foo []
  (if (= nil (binding [*test-var* 1]))
    (.log js/console "Ok!")
    (.log js/console "Too bad!")))

can be run with
1) Clojure: lein run
2) Clojurescript: lein cljsbuild once main && node target/binding-issue.js

I was expecting the same result (nil)...

See discussion at https://groups.google.com/forum/#!topic/clojurescript/anbDq9pjvEs



 Comments   
Comment by David Nolen [ 30/Jan/15 5:47 PM ]

fixed https://github.com/clojure/clojurescript/commit/996f33e5250712072eaefb5eff13bb9372d5e1b6





[CLJS-992] satisfies? does not work with locally bound symbols Created: 28/Jan/15  Updated: 30/Jan/15

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

Type: Defect Priority: Minor
Reporter: Richard Davies Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2727"]



 Description   
(ns sample)
(defprotocol ICustomer (doit [_]))
(defrecord Customer [fname])
(extend-type Customer
  ICustomer
    (doit [_] nil))
(let [t ICustomer]
  (js/alert (satisfies? t (Customer. nil))))

;satisfies? returns false but similar code in returns true.





[CLJS-991] Wrong inference - inconsistent behavior? Created: 28/Jan/15  Updated: 31/Jan/15  Resolved: 31/Jan/15

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

Type: Defect Priority: Minor
Reporter: Christian Fortin Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

The following function will cause a warning to appear when compiling:

(defn test-fn [a]
  (let [b (when a (aget js/document "documentElement" "clientHeight"))]
    (- 10 b 2)))

WARNING: cljs.core/-, all arguments must be numbers, got [number #{nil clj-nil}] instead. at line 12 src/hello_world/core.cljs

This one will compile without any problem:

(defn test-fn2 []
  (let [b (rand-nth [nil (aget js/document "documentElement" "clientHeight")])]

(- 10 b 2)))



 Comments   
Comment by David Nolen [ 31/Jan/15 2:04 PM ]

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





[CLJS-990] Clojurescript records do not have same equality semantics as Clojure records Created: 27/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

Type: Defect Priority: Major
Reporter: Richard Davies Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2665"]



 Description   

(defrecord Pot [a])

(= (Pot. 1) (Pot. 1)) ; returns false

This arose when I was trying to get some code to cross-compile between Clojure and ClojureScript and the Clojure code was using records as map keys.

Workaround:

(extend-type Pot
IEquiv
(-equiv [this that] (and (instance? Pot that) (= (into {} this) (into {} that)))))

Can this behaviour be baked into ClojureScript records by default?



 Comments   
Comment by Richard Davies [ 27/Jan/15 11:27 PM ]

I tried with the latest version of ClojureScript and this works (in isolation). However, when I compile it along with the rest of my code, the equality test still fails without extend-type. I will try to isolate the root case.

Comment by David Nolen [ 30/Jan/15 3:07 PM ]

Cannot reproduce. Please do not reopen this ticket until a minimal case can be supplied thanks.





[CLJS-989] ClojureScript REPL loops on EOF signal Created: 27/Jan/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None
Environment:

ClojureScript 2740


Attachments: Text File CLJS_989.patch    

 Description   

If you run the cljs.repl outside of clojure.main and you CTRL+D, the REPL will loop infinitely



 Comments   
Comment by Bruce Hauman [ 27/Jan/15 3:45 PM ]

Here's a simple patch that fixes it.

Comment by David Nolen [ 20/Feb/15 3:02 PM ]

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





[CLJS-988] Support async testing in cljs.test Created: 27/Jan/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 2
Labels: None

Attachments: File cljs-988.diff     File cljs-988.diff    

 Description   

Async tests should take the form of:

(deftest foo
  (cljs.test/async done
    ...))

This should desugar into:

(deftest foo
  (reify
    cljs.test/IAsyncTest
    IFn
    (-invoke [_ done]
      ...)))

If test running code encounters an async test it should invoke the async test with the next step as the done parameter - it's simply a thunk for the continuation of testing.



 Comments   
Comment by Thomas Heller [ 28/Jan/15 4:40 AM ]

What are the benefits of using an empty marker protocol combined with IFn?

(defprotocol IAsyncTest
  (start-test [_ done]))

Seems like it would do the trick just fine since we will probably never have another arity?

Comment by David Nolen [ 28/Jan/15 6:08 AM ]

It really doesn't matter that much but it does mean the test runner code can be written in the usual functional continuation passing style.

(let [ret (test ...)]
  (if (async? ret)
    (ret k)
    ...))

vs.

(let [ret (test ...)]
  (if (async? ret)
    (start-test k)
    ...))

The former permits simple further functional combinations. The later does not. In this light it may be better to use function metadata to detect async test functions instead of a protocol.

(deftest foo
  (with-meta
    (fn [done]
      ...)
    {:async-test true}))
Comment by Leon Grapenthin [ 03/Feb/15 3:15 AM ]

I don't see the benefit of letting the test return the async test? Why not have a (defasynctest done ...) that associates the IAsyncTest obj directly into :test?
Also, wouldn't the chaining of test after test blow the call stack at some time? If I skimmed over Chas clojurescript.test code correctly, it uses setInterval or setTimeout to detect completion of asynchronous tests in mutable state, assumedly for that reason.

Comment by Leon Grapenthin [ 05/Feb/15 2:22 PM ]

Hi David,
I began working on a patch yesterday. Unless someone else is working on this, I'd like to finish it. Several difficulties and options presented themselves - it would be great if you could comment.

1. Asynchronous execution

To avoid blowing the stack, it seems that the canonical way is to use (js/setTimeout f 0). This is what I came up with so far.

(defn- schedule-async-fns
"Schedules invocation of first fn, passing it a callback and
optionally args. callback must be invoked to schedule the second fn,
again optionally with args..."
[fns & args]
(when-first [f fns]
(js/setTimeout #(apply f
(partial schedule-async-fns (rest fns))
args)
0)))

An alternative would obviously be to use core.async - js/setTimeout doesn't seem present in most browserless execution environments - is there an alternative way or workaround?

2. API of runner

By allowing tests to take over runner execution via callback, the runner API needs to change.
a. Most macros could have added arities to invoke a final callback with what they currently return. However, API compatibility can't always be preserved. Here is a list:

  • test-ns: Can have a new arity, but won't return the result anymore
  • run-tests: Can't have a new arity due to variadic overload, won't return result anymore.
    Unless API compatibility with clj is a non-goal, this certainly isn't the best idea...
    b. The current API could skip async tests, so that it can return synchronously with a meaningful result. The final test-env could include a function which could be invoked with a callback function to trigger the async-tests and receive the final test-env.
    c. As an alternative to the idea in b., it could expect a function to be present in the test-env which it would call with the final result.
    d. As an alternative to b and c, there could be an async test runner. In contrast to b and c, this test runner would run all tests in declared order.
    e. ...

It would be great if you could point in some direction.

3. API of async tests

Should tests really be able to return async tests? The only advantage I can see is that tests can run both as sync and async test. But why? Unless this is a desired usecase, I'd propose a defasynctest which would associate the IAsnycTest obj directly into test. Even simpler, it could just flag the var with :async? true. The latter would certainly make the test-runner code more straight forward - What do you think?

Comment by David Nolen [ 05/Feb/15 2:45 PM ]

Leon

1. if the tests are actually async there's no need to worry about blowing the stack.

2. people use cljs.test to run a side-effect - test reporting. That you can't return values in async isn't a real problem.

3. there will be no changes to the design I've outlined above.

Comment by Leon Grapenthin [ 06/Feb/15 6:32 AM ]

Thanks David, I think I can work with this and will get back to it.

A point I forgot to mention is fixtures. Tear down code as described in the ns docstring would have to work asynchronously as well:

The straightforward way seems to optionally call the passed function with a tear-down-fn callback.

Comment by David Nolen [ 06/Feb/15 5:15 PM ]

Yes fixtures optionally be a map of :before and :after. If we have an async test and the fixture we have in hand isn't a map I think we should abort testing and throw.

Comment by Leon Grapenthin [ 07/Feb/15 9:40 PM ]

Patch notes:

New:

  • fixtures can be specified as maps - see docstring of cljs.test
  • asnyc macro as described in CLJS-988
  • for all runner fns there are -block versions which return composable blocks for users who want to pick their own execution paths, e.g. in a test-ns-hook. Also see run-block, block.

Minor API changes:

  • all runner fns and macros return before any asynchronous testing is finished.
  • test-ns doesn't return the result anymore
  • run-test doesn't return the result anymore
Comment by Leon Grapenthin [ 07/Feb/15 9:46 PM ]

minor fix

Comment by David Nolen [ 07/Feb/15 10:03 PM ]

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





[CLJS-987] Introduce Special :main marker value to help with unit testing. Created: 26/Jan/15  Updated: 31/Jan/15  Resolved: 31/Jan/15

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

Type: Enhancement Priority: Minor
Reporter: Mike Thompson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

This is a follow on enhancement building on http://dev.clojure.org/jira/browse/CLJS-851 (simplify :none script inclusion if :main supplied)

I'd like to improve the unit test story for :optimizations :none

Explanation: https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L69-L93

I propose that if ":main" has a value of "*" then instead of single require, this loop would be put in:
https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L110-L111



 Comments   
Comment by David Nolen [ 27/Jan/15 12:20 PM ]

We're not going to do this. I would be up for allowing a sequence of namespaces.

Comment by Mike Thompson [ 27/Jan/15 3:23 PM ]

Instead of a sequence, how about namespace wildcarding? Eg: :main "some-ns.test.*"

Why? Well, I really, really don't want to manually maintain a sequence. It will be a guaranteed pit of failure. If I have 3 developers working on a project, each adding and deleting unit-test "cljs" files in a certain directory, manual maintenance of that sequence will be error prone, verbose and horrible. tests will be accidentally omitted, some might, wastefully be included twice, etc. Uggh.

I guess I'm saying that manual maintenance of a sequence does not scale up, in a unittest context.

All the good unit test frameworks I've used over the years (eg Nose under python) have a "a runner" ... you point it at a directory, and it recursively "finds" all the unittests in that directory. You never have to list the tests. Instead there is a set of rules around identifying tests:

  • must be in a (sub) directory called "test"
  • must start with "test_" ;; equivalent to wildcarding etc

These "rules" are pretty much just wildcards (which is what I'm proposing) ... discovery by pattern.

Comment by David Nolen [ 28/Jan/15 8:13 AM ]

Mike, we're just not going to do it sorry. Sugar like this is best handled by third party tools like lein-cljsbuild, figwheel, etc.

Also perhaps you're not aware of cljs.test/run-all-tests? It takes an optional regex.

Comment by Mike Thompson [ 28/Jan/15 7:36 PM ]

David, that you are suggesting "run-all-tests" as a solution means I've failed to explain the problem.

RUNNING all the tests is not hard in any of the unittest runners. That is not the issue.

The problem is that of LOADING (goog.require) the namespaces of the unittests WHEN you use `optimisations :none`. In that particular case, there is no root namespace you can `goog.require` which triggers all the unittests cljs into the browser (or Nodejs).

Note: if you use `:optimisation :simple` the problem disappears because all the code comes in one big blob. All the unittests are concatenated into the one file. Easy then to run them, when they are all loaded.

It is `:optimisations :none` which causes the problem, and only when combined with an unknown, flat set of unit-test namespaces (a directory of unittests in cljs files)

I can't do better than this explanation: https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L69-L93

And lein-cljsbuild can't help. Anyway, I've got a feeling you've given your answer on this. Maybe you'll revisit this subject when you get around to getting cljs.test to work with `:optimisations :none`. In the meantime, we'll continue to use the hack.

Comment by David Nolen [ 30/Jan/15 10:10 AM ]

Mike how would you accomplish the same thing in Clojure?

cljsbuild or third party testing tools can easily help. It's simple for them to find all ClojureScript files that match and generate a test runner namespace require the matched namespaces and emit the needed run-all-tests expression.

Comment by Mike Thompson [ 30/Jan/15 9:44 PM ]

I can achieve anything in clojure, or 8-bit assembler, given sufficient time and motivation

My comment about lein-cljsbuild not being able to help was more to do with this: it is not high enough in the tool chain to provide a general solution.

Instead, Boot will have to do as you suggest, and shadow-build will do it, and then cljsbuild could do the same as well. The fact that each of them has to repeat essentially the same thing, lead me to think that it should be factored "up the tool chain" to a level which is already generating requires. And this does appear to be a generalization of the new ":main" feature to me.

But, anyway, like I said above, we should close this ticket. Your time is too important to cljs and I don't want to waste another drop of it.

I'll continue to use the hack. cemeric has asked for it to be put ported into cemerick/clojurescript.test so there is a an `:optimisations :none` story. I guess I wondered if there was a chance of a more elegant fix. Thank you for considering it.

Comment by David Nolen [ 31/Jan/15 1:57 PM ]

No doing this for now. In future we may revisit permitting multiple entry points if people find the need for cases outside of just testing.





[CLJS-986] Add :target to the list of build options that should trigger recompilation Created: 21/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

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


 Comments   
Comment by David Nolen [ 30/Jan/15 3:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/1611bdc7fa7ef21ed2e8543afaefd81b516bacec





[CLJS-985] ex-info loses stack information Created: 20/Jan/15  Updated: 23/Jan/15  Resolved: 23/Jan/15

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

Type: Defect Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File cljs-985-ex-info-stack.patch    

 Description   

Native js Error keeps stacktrace:

(js/console.log (.-stack (js/Error. "message")))

Error: message
    at eval (/Users/prokopov/Dropbox/ws/datascript/test/test/datascript.cljs[eval5]:10:14)
    at eval (native)
    at SocketNamespace.<anonymous> (http://localhost:65000/socket.io/lighttable/ws.js:118:26)
    at SocketNamespace.EventEmitter.emit [as $emit] (http://localhost:65000/socket.io/socket.io.js:633:15)
    at SocketNamespace.onPacket (http://localhost:65000/socket.io/socket.io.js:2248:20)
    at Socket.onPacket (http://localhost:65000/socket.io/socket.io.js:1930:30)
    at Transport.onPacket (http://localhost:65000/socket.io/socket.io.js:1332:17)
    at Transport.onData (http://localhost:65000/socket.io/socket.io.js:1303:16)
    at WebSocket.websocket.onmessage (http://localhost:65000/socket.io/socket.io.js:2378:12)

But ex-info does not:

(js/console.log (.-stack (ex-info "message")))

Error
    at file:///Users/prokopov/Dropbox/ws/datascript/web/target-cljs/cljs/core.js:32066:38

Problem is that ex-info inherits stack property from prototype which is instantiated at script load time here:

(deftype ExceptionInfo [message data cause])

(set! (.-prototype ExceptionInfo) (js/Error.))
(set! (.. ExceptionInfo -prototype -constructor) ExceptionInfo)

The possible solution is to create new instance of js/Error at (ex-info) and manually copy stack property to ExceptionInfo object. Related SO: http://stackoverflow.com/questions/783818/how-do-i-create-a-custom-error-in-javascript

Problem is that Chrome has setter on stack property, and it only allows for this property to be set inside a constructor functions.

Proposed fix creates new Error each time ex-info is called and sets ExceptionInfo.prototype to newly created error. This way new ExceptionInfo instance will inherit stack from newly created Error with correct stack.

This patch has been tested in Chrome 39 Mac, Safari 8 Mac, Firefox 35 Mac and IE 10 Win. Here's test code I used:

(defn -ex-info
  ([msg data]
    (set! (.-prototype ExceptionInfo) (js/Error msg))
    (set! (.. ExceptionInfo -prototype -name) "ExceptionInfo")
    (set! (.. ExceptionInfo -prototype -constructor) ExceptionInfo)
    (ExceptionInfo. msg data nil))
  ([msg data cause]
    (set! (.-prototype ExceptionInfo) (js/Error msg))
    (set! (.. ExceptionInfo -prototype -name) "ExceptionInfo")
    (set! (.. ExceptionInfo -prototype -constructor) ExceptionInfo)
    (ExceptionInfo. msg data cause)))

(try
  (throw (ex-info "[ -- Current ex-info message -- ]" 123))
  (catch ExceptionInfo e
    (js/console.log "Current ex-info::" (.-stack e))))

(try
  (throw (js/Error "[ -- Native message -- ]"))
  (catch js/Error e
    (js/console.log "Native error::" (.-stack e))))

(try
  (throw (-ex-info "[ -- Patched ex-info message -- ]" 123))
  (catch ExceptionInfo e
    (js/console.log "Patched ex-info::" (.-stack e))))

Test results:

Chrome, Firefox, IE, Safari

Note that current implementation reports line number and overall stacktrace from cljs.core file where Error prototype is created in current implementation.
Note that patched version reports correct line number (it should be close to native error stack), stack, message and exception name.
Also note that IE is fine even without patch — that's because in IE stack is capturead at throw place, not at new Error() call site.



 Comments   
Comment by Nikita Prokopov [ 20/Jan/15 2:48 PM ]

Ok, this is crazy, but this seems to solve the issue:

(defn ex-info [msg data cause]
  (set! (.-prototype ExceptionInfo) (js/Error msg))
  (set! (.. ExceptionInfo -prototype -name) "cljs.core.ExceptionInfo")
  (set! (.. ExceptionInfo -prototype -constructor) ExceptionInfo)
  (ExceptionInfo. msg data cause))

Basically we change prototype before creating each object.

(taken from http://stackoverflow.com/questions/783818/how-do-i-create-a-custom-error-in-javascript#answer-12030032)

I guess high performance is not needed from ex-info, so this solution is somewhat okay-ish? Should I make a patch from it?

Comment by David Nolen [ 20/Jan/15 2:55 PM ]

It would be nice to get confirmation from others that this works under the major browser - Safari, Firefox, Chrome, and modern IE.

Comment by Nikita Prokopov [ 20/Jan/15 3:12 PM ]

I can confirm Firefox 34, Firefox 35, Safari 8.0.2 and Chrome 39 (all Mac) for now

Comment by Nikita Prokopov [ 22/Jan/15 3:50 AM ]

David, I updated issue, added patch, test code and test results (including IE). There’s no unit test on this because stack traces are very engine-specific. Please take a look

Comment by David Nolen [ 22/Jan/15 2:24 PM ]

Nikita, thanks for the update will check it out.

Comment by David Nolen [ 23/Jan/15 6:13 PM ]

fixed https://github.com/clojure/clojurescript/commit/93dce672e1af8f698cfc2a61e293cb48aeeddc2c





[CLJS-984] Update Node.js REPL support to use public API Created: 20/Jan/15  Updated: 20/Jan/15  Resolved: 20/Jan/15

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

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


 Description   

https://github.com/google/closure-library/commit/d909e5aa4b71923d72c15305f70d01a976c9947f



 Comments   
Comment by David Nolen [ 20/Jan/15 2:56 PM ]

fixed https://github.com/clojure/clojurescript/commit/62d898ae30eb58397628b45b3c0c95d3e899a274





[CLJS-983] Make ExceptionInfo printable Created: 20/Jan/15  Updated: 24/Jan/15  Resolved: 24/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-983-printable-ex-info-2.patch    

 Description   

Pretty simple enhancement — so we can see what's inside



 Comments   
Comment by David Nolen [ 23/Jan/15 6:21 PM ]

This patch needs a rebased to master.

Comment by Nikita Prokopov [ 24/Jan/15 2:56 AM ]

Things got a little messier after CLJS-985 and prototype manipulations

Comment by Nikita Prokopov [ 24/Jan/15 2:57 AM ]

Uploaded updated patch, rebased to master

Comment by David Nolen [ 24/Jan/15 10:29 AM ]

fixed https://github.com/clojure/clojurescript/commit/7f6f8fd2745a49ac87d3307bdb6e15e91abc26e2





[CLJS-982] Var derefing should respect Clojure semantics Created: 18/Jan/15  Updated: 18/Jan/15  Resolved: 18/Jan/15

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

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


 Description   

In every compilation mode other than advanced possible to preserve via a thunk



 Comments   
Comment by David Nolen [ 18/Jan/15 10:00 PM ]

fixed https://github.com/clojure/clojurescript/commit/522fbdc04e5e35171e6818ce80b3db4811cc3284





[CLJS-981] Better benchmarking infrastructure Created: 17/Jan/15  Updated: 17/Jan/15

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   

We should use ProcessBuilder to run the various benchmark scripts and control which benchmarks we test and which engines we run. Benchmarks should produce EDN data that can be written to a file, loaded into Incanter, etc.






[CLJS-980] ClojureScript REPL stacktraces overrun prompt in many cases Created: 17/Jan/15  Updated: 17/Jan/15  Resolved: 17/Jan/15

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

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript 2687


Attachments: Text File CLJS-980.patch    

 Description   

ClojureScript repl stacktraces overrun prompt in many cases

If you do
(doc 'clojure.string/join)
The failure stacktrace hides the prompt, creating the illusion the REPL has hung.



 Comments   
Comment by Bruce Hauman [ 17/Jan/15 11:10 AM ]

Provided patch that fixes this.

Comment by David Nolen [ 17/Jan/15 11:21 AM ]

fixed https://github.com/clojure/clojurescript/commit/359ea4eec7aad5902abbaeee319df44d78183469





[CLJS-979] ClojureScript REPL needs error handling for the special functions Created: 16/Jan/15  Updated: 16/Jan/15  Resolved: 16/Jan/15

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

Type: Defect Priority: Major
Reporter: Bruce Hauman Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript 2688


Attachments: Text File CLJS-979.patch    

 Description   

Currently the the special functions are not run int a try catch block. This creates a brittle repl that will fail is someone trys to require a namespace that doesn't exist or execute any of the special functions with error causing arguments.

The following fail and terminate the repl loop.
(require 'ex) where ex doesn't exist
(in-ns '5) causes an infinite loop.



 Comments   
Comment by Bruce Hauman [ 16/Jan/15 4:32 PM ]

Here is a patch that fixes this. A general handler ensures that injected special-fns are handled as well.

Comment by David Nolen [ 16/Jan/15 5:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/7e098b2ea2196e2643ceea62bde943f674b0d362





[CLJS-978] Analysis caching doesn't account for constants table Created: 15/Jan/15  Updated: 16/Jan/15

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 means advanced optimizations cannot leverage analysis caching.



 Comments   
Comment by Thomas Heller [ 16/Jan/15 4:48 AM ]

Currently the constants use a generated id, we could instead use the constant itself as the key.

:test/keyword creates cljs.core.constant$keyword$123 where 123 is the sequential id which makes it unusable between compiles. We could instead turn it into cljs.core.constant$keyword$_COLON_test_SLASH_keyword (to reuse current munge). We would then just need to keep track of which namespace uses which constant and emit this as well in the analysis cache.

Might be missing something, will investigate a bit later.

Comment by David Nolen [ 16/Jan/15 5:59 AM ]

The constants table needs to work for all compile time constants - vectors, sets, maps, etc.





[CLJS-977] Subvec missing IKVReduce implementation Created: 15/Jan/15  Updated: 15/Jan/15

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





[CLJS-976] Node REPL breaks from uncaught exceptions Created: 14/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

Type: Defect Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-976.patch    

 Description   

Certain errors raised by Node will cause a ClojureScript Node REPL to fail.

Steps to reproduce:

cljs.user> (require '[cljs.nodejs :as node])
cljs.user> (def fs (node/require "fs"))
cljs.user> (.createReadStream fs "fail")
;; => #<[object Object]>
cljs.user> (+ 1 1)
;; => java.lang.IllegalArgumentException: Value out of range for char: -1

A vanilla Node REPL will also fail under the same condition and terminate the session.

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: ENOENT, open 'fail'

The CLJS REPL should probably be a bit more durable and, instead of terminating, catch and report the error.

A gist of the above example can be found here: https://gist.github.com/noprompt/3d0e5cbfca7883727fcf.



 Comments   
Comment by Mike Fikes [ 29/Jan/15 9:59 PM ]

Hey Joel, this appears to be rooted in the Node process itself dying.

I reproduced using your initial steps and confirmed that my Node process disappears. You can produce the same java.lang.IllegalArgumentException: Value out of range for char: -1 if you simply kill the node process out from underneath the REPL, or do something else that will cause it to give up the ghost (I ran into it having it evaluate (set (range 20000000)))

If you try to issue a subsequent command you will then get a java.net.SocketException: Broken pipe when the attempt is made to write the JavaScript out to the Node process on the (defunct) socket.

The IllegalArgumentException associated with the failure to handle -1 being returned from the socket read could be converted to an IOException, perhaps giving the user more of a clue:

diff --git a/src/clj/cljs/repl/node.clj b/src/clj/cljs/repl/node.clj
index 563d63a..9ea81db 100644
--- a/src/clj/cljs/repl/node.clj
+++ b/src/clj/cljs/repl/node.clj
@@ -16,7 +16,7 @@
             [clojure.data.json :as json])
   (:import java.net.Socket
            java.lang.StringBuilder
-           [java.io File BufferedReader BufferedWriter]
+           [java.io File BufferedReader BufferedWriter EOFException]
            [java.lang ProcessBuilder ProcessBuilder$Redirect]))
 
 (defn socket [host port]
@@ -39,6 +39,7 @@
   (let [sb (StringBuilder.)]
     (loop [sb sb c (.read in)]
       (cond
+       (= c -1) (throw (EOFException.))
        (= c 1) (let [ret (str sb)]
                  (print ret)
                  (recur (StringBuilder.) (.read in)))

But perhaps an even better solution would be to, in addition, add a try-catch in node-eval to handle all IOException s encountered when attempting to communicate with the Node process and to simply print a suitable meaningful error to the user stating such. (I suppose you can't necessarily conclude that the Node process is actually gone; it may be a timeout that leads to an IOException). But this would arguably be more robust than derailing trying to cast -1 to a char.

Comment by Mike Fikes [ 30/Jan/15 8:42 AM ]

The attached patch adds robustness by handling errors reading and writing to the underlying Node process and by additionally detecting if Node has died.

For Joel's example, it doesn't fail fast, but when you attempt a subsequent evaluation, it does provide diagnostics:

ClojureScript:cljs.user> (+ 1 1)
IOException communicating with Node process: Connection closed
Node process has exited with value: 1
nil

and with the example I encountered (in a fresh REPL):

ClojureScript:cljs.user> (set (range 20000000))
FATAL ERROR: CALL_AND_RETRY_2 Allocation failed - process out of memory
IOException communicating with Node process: Connection closed
Node process has exited with value: 134
nil

(This last example is on OS X. Note that 134 - 128 = 6, which is SIGABRT.)

Comment by David Nolen [ 30/Jan/15 3:01 PM ]

The correct solution is to prevent the Node.js process from ever dying from uncaught exceptions.

Fixed in master https://github.com/clojure/clojurescript/commit/2d81bde0a7e1fbcf1f4883cb144bf2b10cea393a

Comment by Mike Fikes [ 30/Jan/15 3:17 PM ]

Confirmed fixed for Joel's example, and for mine, the outcome is also still reasonable given that the out-of-memory condition is displayed:

ClojureScript:cljs.user> (set (range 20000000))
FATAL ERROR: CALL_AND_RETRY_2 Allocation failed - process out of memory
java.lang.IllegalArgumentException: Value out of range for char: -1
	at clojure.lang.RT.charCast(RT.java:962)
	at cljs.repl.node$read_response.invoke(node.clj:47)
	at cljs.repl.node$node_eval.invoke(node.clj:57)
	at cljs.repl.node.NodeEnv._evaluate(node.clj:164)
	at cljs.repl$evaluate_form.invoke(repl.clj:206)
	at cljs.repl$evaluate_form.invoke(repl.clj:168)
	at cljs.repl$eval_and_print.invoke(repl.clj:263)
	at cljs.repl$repl_STAR_.invoke(repl.clj:427)
	at user$eval3562.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:6703)
	at clojure.lang.Compiler.eval(Compiler.java:6666)
	at clojure.core$eval.invoke(core.clj:2927)
	at clojure.main$eval_opt.invoke(main.clj:288)
	at clojure.main$initialize.invoke(main.clj:307)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.doInvoke(main.clj:420)
	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)
java.lang.IllegalArgumentException: Value out of range for char: -1
nil




[CLJS-975] preserve :reload & :reload-all in ns macro sugar Created: 13/Jan/15  Updated: 21/Feb/15  Resolved: 21/Feb/15

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

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


 Description   

:include-macros true and :refer-macros [...] are both sugar forms supported in :use and :require, they expand into :use-macros or :require-macros specs. We should make sure to preserve :reload and :reload-all when desugaring.



 Comments   
Comment by Thomas Heller [ 13/Jan/15 10:48 AM ]

Oops, didn't see there was an issue for this.

Sorry for the rant but please note my comment on:
https://github.com/clojure/clojurescript/commit/e408305344ef7b3658118377e530c78c2eb93cf3

I do not think REPL related "options" have anything to do in the parse function of the namespace. I outlined a different approach which I hope you'll consider. Of course all of this is motivated by shadow-build, so YMMV.

Comment by David Nolen [ 13/Jan/15 11:16 AM ]

While shadow-build is an interesting piece of technology it's not relevant for tickets like this. Please keep threads clear of non-issues.

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

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





[CLJS-974] Clean compile/build fails to resolve namespaces for cljs.test Created: 12/Jan/15  Updated: 13/Jan/15  Resolved: 13/Jan/15

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

Type: Defect Priority: Major
Reporter: Denis Johnson Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: cljs, test
Environment:

Linux Ubuntu 14.04
Java 1.7.0_17 Java HotSpot(TM) 64-Bit Server VM
leiningen 2.5.1
Intellij 13.1.6 with latest cursive 0.1.43 plugin

:dependencies
[[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2665"]
[reagent "0.5.0-alpha"]
[alandipert/storage-atom "1.2.3"]
[com.andrewmcveigh/cljs-time "0.3.0"]]
:plugins [[lein-cljsbuild "1.0.3"]]


Attachments: File test_suite.cljs    

 Description   

Clean compile gives the following error for a valid namespace for runner/suite (see attachment).
Same namespace works fine using cemerick/clojurescript.test
NOTE 1: if compiled using "cljsbuild auto test" after giving failure, if you delete the test/test-suite.cljs the compile completes successfully, if then you revert the file it also recompiles successfully and tests run as expected.
NOTE 2: Happens all the time on my machine but other devs on Windows platform don't seem to get this issue. The only difference I can see is I use leiningen checkouts feature to point at our other local repos and these are project dependencies. I have tested with removing the checkouts folder with change in behaviour so I doubt it is related.

Deleting files generated by lein-cljsbuild.
Compiling ClojureScript.
Compiling "compiled/test.js" from ["src" "test"]...
Compiling "compiled/test.js" failed.
clojure.lang.ExceptionInfo: failed compiling file:test/test_suite.cljs
core.clj:4403 clojure.core/ex-info
compiler.clj:1039 cljs.compiler/compile-file
compiler.clj:1069 cljs.compiler/compile-root
closure.clj:358 cljs.closure/compile-dir
closure.clj:398 cljs.closure/eval3160[fn]
closure.clj:306 cljs.closure/eval3096[fn]
closure.clj:412 cljs.closure/eval3147[fn]
closure.clj:306 cljs.closure/eval3096[fn]
compiler.clj:44 cljsbuild.compiler.SourcePaths/fn
core.clj:2557 clojure.core/map[fn]
LazySeq.java:40 clojure.lang.LazySeq.sval
LazySeq.java:49 clojure.lang.LazySeq.seq
RT.java:484 clojure.lang.RT.seq
core.clj:133 clojure.core/seq
core.clj:624 clojure.core/apply
core.clj:2586 clojure.core/mapcat
RestFn.java:423 clojure.lang.RestFn.invoke
compiler.clj:44 cljsbuild.compiler/cljsbuild.compiler.SourcePaths
closure.clj:1018 cljs.closure/build
closure.clj:972 cljs.closure/build
compiler.clj:58 cljsbuild.compiler/compile-cljs[fn]
compiler.clj:57 cljsbuild.compiler/compile-cljs
compiler.clj:159 cljsbuild.compiler/run-compiler
form-init1706317005734214457.clj:1 user/eval3512[fn]
form-init1706317005734214457.clj:1 user/eval3512[fn]
LazySeq.java:40 clojure.lang.LazySeq.sval
LazySeq.java:49 clojure.lang.LazySeq.seq
RT.java:484 clojure.lang.RT.seq
core.clj:133 clojure.core/seq
core.clj:2855 clojure.core/dorun
core.clj:2871 clojure.core/doall
form-init1706317005734214457.clj:1 user/eval3512
Compiler.java:6703 clojure.lang.Compiler.eval
Compiler.java:6693 clojure.lang.Compiler.eval
Compiler.java:7130 clojure.lang.Compiler.load
Compiler.java:7086 clojure.lang.Compiler.loadFile
main.clj:274 clojure.main/load-script
main.clj:279 clojure.main/init-opt
main.clj:307 clojure.main/initialize
main.clj:342 clojure.main/null-opt
main.clj:420 clojure.main/main
RestFn.java:421 clojure.lang.RestFn.invoke
Var.java:383 clojure.lang.Var.invoke
AFn.java:156 clojure.lang.AFn.applyToHelper
Var.java:700 clojure.lang.Var.applyTo
main.java:37 clojure.main.main
Caused by: clojure.lang.ExceptionInfo: No such namespace: test.model.dimension-glossary-test at line 1 test/test_suite.cljs
core.clj:4403 clojure.core/ex-info
analyzer.clj:297 cljs.analyzer/error
analyzer.clj:294 cljs.analyzer/error
analyzer.clj:1095 cljs.analyzer/analyze-deps
analyzer.clj:1280 cljs.analyzer/eval1561[fn]
MultiFn.java:249 clojure.lang.MultiFn.invoke
analyzer.clj:1609 cljs.analyzer/analyze-seq
analyzer.clj:1696 cljs.analyzer/analyze[fn]
analyzer.clj:1689 cljs.analyzer/analyze
compiler.clj:948 cljs.compiler/compile-file*[fn]
compiler.clj:906 cljs.compiler/with-core-cljs
compiler.clj:927 cljs.compiler/compile-file*
compiler.clj:1033 cljs.compiler/compile-file



 Comments   
Comment by David Nolen [ 13/Jan/15 7:31 AM ]

Please report cljsbuild bugs with the cljsbuild project thanks.





[CLJS-973] cljs.test ignores some deftest names Created: 12/Jan/15  Updated: 15/Jan/15  Resolved: 15/Jan/15

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

Type: Defect Priority: Minor
Reporter: Denis Johnson Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: cljs, test
Environment:

Linux Ubuntu 14.04
Java 1.7.0_17 Java HotSpot(TM) 64-Bit Server VM
leiningen 2.5.1

:dependencies
[[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2665"]
[reagent "0.5.0-alpha"]
[alandipert/storage-atom "1.2.3"]
[com.andrewmcveigh/cljs-time "0.3.0"]]
:plugins [[lein-cljsbuild "1.0.3"]]



 Description   

examples:

  • ignores: (deftest test:change-step-contiguous-forward> ....)
  • works: (deftest test:change-current-step ...) ignores: (deftest test-change-current-step ...)


 Comments   
Comment by David Nolen [ 15/Jan/15 1:26 AM ]
(deftest test:change-current-step: ...)
(deftest test:change-current-step@ ...)

Removed these two from ticket description these are not valid Clojure symbols see http://clojure.org/reader

Comment by David Nolen [ 15/Jan/15 1:37 AM ]

I tested the remaining names could not reproduce. Please only reopen this ticket if a full minimal case is provided.





[CLJS-972] Node.js REPL eats errors in required ns when using require Created: 12/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

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


 Comments   
Comment by David Nolen [ 30/Jan/15 4:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/87a14e1b8d253e28c2647c98ea91a42cd24a9972





[CLJS-971] at REPL :reload should work for require-macros special fn Created: 12/Jan/15  Updated: 13/Jan/15  Resolved: 13/Jan/15

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

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


 Comments   
Comment by David Nolen [ 13/Jan/15 8:03 AM ]

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





[CLJS-970] assert - generate Error string when compiling Created: 10/Jan/15  Updated: 12/Jan/15

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

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

Attachments: File cljs-generate-assert-message.diff    
Patch: Code

 Description   

Currently when using assert in CLJS the assert macro will generate the assert error message through CLJS. We can do that in CLJ which reduces the amount of code generated.

(let [bar 1]
  (assert (string? bar)))

Currently generates

if(typeof bar_11332 === 'string'){
} else {
throw (new Error([cljs.core.str("Assert failed: "),cljs.core.str(cljs.core.pr_str.call(null,cljs.core.list(new cljs.core.Symbol(null,"string?","string?",-1129175764,null),new cljs.core.Symbol(null,"bar","bar",254284943,null))))].join('')));
}

But could generate

if(typeof bar_23183 === 'string'){
} else {
throw (new Error("Assert failed: (string? bar)"));
}

Attached patch does that.



 Comments   
Comment by Thomas Heller [ 12/Jan/15 9:16 AM ]

Forgot to account for the fact that

(assert (string? something) "this might not be a string"))

New patch only generates the string of the form when compiling.





[CLJS-969] Node REPL fails with TypeError under Windows Created: 08/Jan/15  Updated: 27/Jan/15  Resolved: 27/Jan/15

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

Type: Defect Priority: Major
Reporter: Adrian Sampaleanu Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Windows 8.1, Node.js 0.10.35



 Description   

With latest source and following the steps outlined in The Node.js REPL of My Dreams, the following error is emitted when noderepljs is invoked:

$ ./script/noderepljs
To quit, type: :cljs/quit
ClojureScript Node.js REPL server listening on 58343
"Error evaluating:" (do (.require js/goog "cljs.core") (set! *print-fn* (.-print (js/require "util")))) :as "goog.require(\"cljs.core\");\r\n\r\ncljs.core._STAR_print_fn_STAR_ = require(\"
util\").print;\r\n"
TypeError: Cannot read property 'nameToPath' of undefined
    at Object.goog.require (repl:2:49)
    at repl:1:6
    at Socket.<anonymous> ([stdin]:30:80)
    at Socket.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:765:14)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:427:10)
    at emitReadable (_stream_readable.js:423:5)
    at readableAddChunk (_stream_readable.js:166:9)
    at Socket.Readable.push (_stream_readable.js:128:10)
ClojureScript:cljs.user>

Though the correct prompt appears, the REPL is non functional.



 Comments   
Comment by David Nolen [ 08/Jan/15 7:19 AM ]

Thanks for the report. Sadly there are no cycles to work on Windows issues ourselves. Patches from Windows users are alway welcome.

Comment by David Nolen [ 27/Jan/15 12:23 PM ]

Should be fixed in master





[CLJS-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 16/Feb/15

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

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: Unassigned
Resolution: Unresolved Votes: 1
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".





[CLJS-967] "java.net.ConnectException: Connection refused" when running node repl Created: 07/Jan/15  Updated: 12/Feb/15  Resolved: 12/Feb/15

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

Type: Defect Priority: Major
Reporter: Brian Muhia Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: cljs, errormsgs
Environment:

Processor: 1.33GHz Intel Atom, Ubuntu 12.04, OpenJDK 1.7, Clojure 1.6.0, ClojureScript 0.0-2665, nodejs v0.10.26


Attachments: Text File backtrace.txt    

 Description   

I used trampoline like so:

rlwrap lein trampoline run -m clojure.main scripts/repl.clj

run the script repl.clj with contents:

(require
  '[cljs.repl :as repl]
  '[cljs.repl.node :as node])

(repl/repl* (node/repl-env)
  {:output-dir "out"
   :optimizations :none
   :cache-analysis true                
   :source-map true})

Instead of getting a repl, I got the exception: Exception in thread "main" java.net.ConnectException: Connection refused, compiling/home/brian/projects/essence-cljs-redux/scripts/repl.clj:3:30)

The full stack trace is attached in backtrace.txt.



 Comments   
Comment by David Nolen [ 08/Jan/15 7:21 AM ]

The issue is that we use a 300ms timeout to connect to the Node.js process, we need to instead redirect the process out and wait for the Node.js REPL server start string.

Comment by David Nolen [ 12/Feb/15 7:22 AM ]

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





[CLJS-966] :foreign-libs should not go through Closure Compiler Created: 07/Jan/15  Updated: 23/Jan/15  Resolved: 23/Jan/15

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently the files listed in foreign libs will go through Closure Compiler when using :advanced, they should instead be in the preamble.

https://github.com/thheller/cljs-foreign-bug

lein cljsbuild once release

will create a foreign.min.js [1] which includes the advanced optimized source of js/sample.js (grep for I WONT SURVIVE). In this case the source actually keeps working since the source is quite simple. But we don't even need to include the js/sample.js in the HTML file since the source won't access Foreign.sayHi since it got renamed and uses the optimized code.

[1] https://github.com/thheller/cljs-foreign-bug/blob/master/foreign.min.js



 Comments   
Comment by David Nolen [ 23/Jan/15 6:21 PM ]

fixed in master





[CLJS-965] Make :foreign-libs more useful Created: 07/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

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


 Description   

:foreign-libs almost does what we need to make working with JavaScript dependencies painless. They need to additionally specify :requires. :foreign-libs should automatically be preambled in advanced compilation, dependency order can be solved since they provide :requires. Coupled with the deps.clj support using JS deps from JARs would be as simple and as painless as CLJS JARs.

The following conditions must be met:

A) do not break existing usage of deps.clj - means we need to check :foreign-lib true in deps.clj
B) Support optimizations :none, means we must emit goog.addDependency in deps.js
C) Support optimizations :advanced, means we should sort in dependency order and place foreign between :preamble and Closure advanced build
D) users should be able to specify a JS file to use under :none to support debugging



 Comments   
Comment by Yehonathan Sharvit [ 08/Jan/15 1:46 PM ]

Is it currently possible to include a js file inside a cljs lib, without requiring from the library clients to download manually the js file?

Comment by David Nolen [ 08/Jan/15 1:51 PM ]

It is not possible and such a feature is out of scope.

Comment by Francis Avila [ 08/Jan/15 2:45 PM ]

Isn't this done all the time, even with foreign-libs? Or do I misunderstand the question? Yehonathan, what specific scenario are you thinking of? Serving a foreign-lib from a cdn?

Comment by David Nolen [ 30/Jan/15 3:13 PM ]

fixed in 0.0-2719





[CLJS-964] Redefining exists? does not emit a warning like redefining array? does. Created: 06/Jan/15  Updated: 07/Jan/15

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

Type: Defect Priority: Minor
Reporter: Uday Verma Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

[org.clojure/clojurescript "0.0-2665"]



 Description   

ClojureScript:cljs.user> (def array? 2)
WARNING: array? already refers to: /array? being replaced by: cljs.user/array? at line 1 <cljs repl>
2
ClojureScript:cljs.user> (def exists? 2)
2
ClojureScript:cljs.user>



 Comments   
Comment by Mike Fikes [ 06/Jan/15 11:41 PM ]

Additionally, if you define exists? as as a function in your namespace, as in

(defn exists? [] 1)

You need to call it indirectly via its var:

ClojureScript:cljs.user> (@#'exists?)
1

If you instead call it directly, you will get this (this is in the Node.js REPL; similar occurs in JavaScriptCore environment):

ClojureScript:cljs.user> (exists?)
clojure.lang.ExceptionInfo: Wrong number of args (2) passed to: core/exists? at line 1 <cljs repl> {:tag :cljs/analysis-error, :file "<cljs repl>", :line 1, :column 1}
	at clojure.core$ex_info.invoke(core.clj:4403)
	at cljs.analyzer$error.invoke(analyzer.clj:297)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1562)
	at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1605)
	at cljs.analyzer$analyze$fn__1673.invoke(analyzer.clj:1696)
	at cljs.analyzer$analyze.invoke(analyzer.clj:1689)
	at cljs.analyzer$analyze.invoke(analyzer.clj:1684)
	at cljs.repl$evaluate_form.invoke(repl.clj:100)
	at cljs.repl$evaluate_form.invoke(repl.clj:96)
	at cljs.repl$eval_and_print.invoke(repl.clj:188)
	at cljs.repl$repl_STAR_.invoke(repl.clj:322)
	at user$eval3528.invoke(NO_SOURCE_FILE:3)
	at clojure.lang.Compiler.eval(Compiler.java:6703)
	at clojure.lang.Compiler.eval(Compiler.java:6666)
	at clojure.core$eval.invoke(core.clj:2927)
	at clojure.main$eval_opt.invoke(main.clj:288)
	at clojure.main$initialize.invoke(main.clj:307)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.ArityException: Wrong number of args (2) passed to: core/exists?
	at clojure.lang.AFn.throwArity(AFn.java:429)
	at clojure.lang.AFn.invoke(AFn.java:36)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:628)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1568)
	... 21 more

None of this occurs for the other predicates defined near exists? in cljs.core, like array?, true?, etc.

Comment by Thomas Heller [ 07/Jan/15 4:58 AM ]

I suspect that is due to exists? only being a macro with no function in cljs.core. array? and other predicates exist as an inlining macro and a function.

(defn ^boolean array? [x]
  (cljs.core/array? x))

Should just be a matter of creating a similar function for exists?.

Comment by Thomas Heller [ 07/Jan/15 5:00 AM ]

Oops no. Won't be that simple.

;; TODO: x must be a symbol, not an arbitrary expression
(defmacro exists? [x]
  (bool-expr
    (core/list 'js* "typeof ~{} !== 'undefined'"
      (vary-meta x assoc :cljs.analyzer/no-resolve true))))




[CLJS-963] goog dep.js is not recomputed when sources are recompiled Created: 06/Jan/15  Updated: 06/Jan/15  Resolved: 06/Jan/15

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

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


 Description   

Encountered this in mies when switching from Node.js REPL to Browser REPL



 Comments   
Comment by David Nolen [ 06/Jan/15 7:51 AM ]

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





[CLJS-962] Inconsistent hashing of empty collections Created: 05/Jan/15  Updated: 06/Jan/15  Resolved: 06/Jan/15

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

Type: Defect Priority: Major
Reporter: Arthur Ulfeldt Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None
Environment:

clojurescript 0.0-2496"
clojure 1.6.0


Attachments: Text File 0001-CLJS-962-fix-inconsistent-hashing-of-empty-collectio.patch    

 Description   

I get different results when hashing a literal empty vector than when hashing one from read-string, while all the other collections produce the same hash code:

``` clojure
homepage.core> [(hash [2]) (hash (cljs.reader/read-string "[2]"))]
[-1917711765 -1917711765]
homepage.core> [(hash [0]) (hash (cljs.reader/read-string "[0]"))]
[965192274 965192274]
homepage.core> [(hash []) (hash (cljs.reader/read-string "[]"))]
[0 -2017569654] ;; <--- this one looks suspicious.
homepage.core> [(hash {}) (hash (cljs.reader/read-string "{}"))]
[-15128758 -15128758]
homepage.core> (hash #{}) (hash (cljs.reader/read-string "#{}"))
[0 0]
homepage.core> clojurescript-version
"0.0-2496"
```



 Comments   
Comment by David Nolen [ 05/Jan/15 11:24 AM ]

Seems to be a bug that effects all empty collections, the hash values should match Clojure's.

Comment by Michał Marczyk [ 05/Jan/15 6:34 PM ]

Patch with test.

Comment by David Nolen [ 06/Jan/15 7:57 AM ]

Michal, wow thanks for the quick fix!

Comment by David Nolen [ 06/Jan/15 8:00 AM ]

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

Comment by Michał Marczyk [ 06/Jan/15 2:55 PM ]

Thanks for the quick merge!





[CLJS-961] REPL process should always be killable Created: 04/Jan/15  Updated: 30/Jan/15  Resolved: 30/Jan/15

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

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


 Description   

In the browser REPL case if user never connects browser cannot kill REPL with Ctrl-C or Ctrl-D.



 Comments   
Comment by David Nolen [ 30/Jan/15 3:12 PM ]

Duplicate CLJS-989





[CLJS-960] On carriage return REPLs should always show new REPL prompt Created: 04/Jan/15  Updated: 20/Feb/15  Resolved: 20/Feb/15

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

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


 Comments   
Comment by Bruce Hauman [ 10/Feb/15 8:55 AM ]

You could delegate this to IJavaScriptEnv's. I solved this in the FigwheelEnv.

I have to wait for a connection which is a valid wait, and it should wait for as long as it takes for a connection to connect. This happens both on -evaluate and -setup.

But then there is the case where an -evaluate fails to respond. In this case I just have an 8 second timeout to return control to the REPL.

Comment by David Nolen [ 20/Feb/15 3:02 PM ]

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





[CLJS-959] Protocol methods specifying an empty docstring fail to compile Created: 03/Jan/15  Updated: 05/Jan/15  Resolved: 05/Jan/15

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

Type: Defect Priority: Minor
Reporter: Julien Eluard Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File protocol-empty-docstring.diff    

 Description   

Starting ClojureScript 2411 protocols with method specifying an empty docstring fail to compile. Non-empty docstrings are fine.

Following code compiles fine:

(defprotocol Protocol
  (method [_]))
(defprotocol Protocol2
  (method [_] "some doc"))

While following code throws an exception:

(defprotocol Protocol
  (method [_] ""))
clojure.lang.ExceptionInfo: failed compiling file:src/mies_hello_world/core.cljs {:file #<File src/mies_hello_world/core.cljs>}
	at clojure.core$ex_info.invoke(core.clj:4403)
	at cljs.compiler$compile_file.invoke(compiler.clj:999)
	at cljs.compiler$compile_root.invoke(compiler.clj:1029)
	at cljs.closure$compile_dir.invoke(closure.clj:358)
	at cljs.closure$fn__2534.invoke(closure.clj:398)
	at cljs.closure$fn__2484$G__2479__2491.invoke(closure.clj:306)
	at cljs.closure$fn__2527.invoke(closure.clj:412)
	at cljs.closure$fn__2484$G__2479__2491.invoke(closure.clj:306)
	at cljsbuild.compiler.SourcePaths$fn__134.invoke(compiler.clj:67)
	at clojure.core$map$fn__4245.invoke(core.clj:2557)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$apply.invoke(core.clj:624)
	at clojure.core$mapcat.doInvoke(core.clj:2586)
	at clojure.lang.RestFn.invoke(RestFn.java:423)
	at cljsbuild.compiler.SourcePaths._compile(compiler.clj:67)
	at cljs.closure$build.invoke(closure.clj:1018)
	at cljs.closure$build.invoke(closure.clj:972)
	at cljsbuild.compiler$compile_cljs$fn__145.invoke(compiler.clj:81)
	at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:80)
	at cljsbuild.compiler$run_compiler.invoke(compiler.clj:179)
	at user$eval277$iter__295__299$fn__300$fn__312.invoke(form-init1238899578077056082.clj:1)
	at user$eval277$iter__295__299$fn__300.invoke(form-init1238899578077056082.clj:1)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$dorun.invoke(core.clj:2855)
	at clojure.core$doall.invoke(core.clj:2871)
	at user$eval277.invoke(form-init6397079933373263622.clj:1)
	at clojure.lang.Compiler.eval(Compiler.java:6703)
	at clojure.lang.Compiler.eval(Compiler.java:6693)
	at clojure.lang.Compiler.load(Compiler.java:7130)
	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
	at clojure.main$load_script.invoke(main.clj:274)
	at clojure.main$init_opt.invoke(main.clj:279)
	at clojure.main$initialize.invoke(main.clj:307)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: Invalid protocol, Protocol defines method method with arity 0 at line 8 src/mies_hello_world/core.cljs {:tag :cljs/analysis-error, :file "src/mies_hello_world/core.cljs", :line 8, :column 1}
	at clojure.core$ex_info.invoke(core.clj:4403)
	at cljs.analyzer$error.invoke(analyzer.clj:297)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1520)
	at cljs.analyzer$analyze_seq.invoke(analyzer.clj:1563)
	at cljs.analyzer$analyze$fn__1360.invoke(analyzer.clj:1654)
	at cljs.analyzer$analyze.invoke(analyzer.clj:1647)
	at cljs.compiler$compile_file_STAR_$fn__2213.invoke(compiler.clj:909)
	at cljs.compiler$with_core_cljs.invoke(compiler.clj:867)
	at cljs.compiler$compile_file_STAR_.invoke(compiler.clj:888)
	at cljs.compiler$compile_file.invoke(compiler.clj:993)
	... 44 more
Caused by: java.lang.Exception: Invalid protocol, Protocol defines method method with arity 0
	at cljs.core$defprotocol$fn__3323.invoke(core.clj:1017)
	at cljs.core$defprotocol.doInvoke(core.clj:1015)
	at clojure.lang.RestFn.applyTo(RestFn.java:146)
	at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:628)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1526)
	... 51 more

I figured it was worth reporting giving the error message is a little unexpected.



 Comments   
Comment by David Nolen [ 05/Jan/15 11:44 AM ]

fixed https://github.com/clojure/clojurescript/commit/7561c37231100fbdbe620b5dda7368e7bd2da632





[CLJS-958] Node.js REPL: Upon error, last successfully item printed Created: 03/Jan/15  Updated: 03/Jan/15  Resolved: 03/Jan/15

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

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

0.0-2657
Node.js REPL set up as per http://swannodette.github.io/2014/12/29/nodejs-of-my-dreams/

uname -a
Linux ubuntu 3.13.0-32-generic #57-Ubuntu SMP Tue Jul 15 03:51:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

java -version
java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)

node --version
v0.10.25



 Description   

If at the REPL, you evaluate a form that results in an error, the last successfully printed item prints again.

Here is an example:

ClojureScript:cljs.user> (time (reduce + (range 1000000)))
"Elapsed time: 427 msecs"
499999500000
ClojureScript:cljs.user> (first (js/Date.))
Error: Sat Jan 03 2015 19:48:17 GMT-0500 (EST) is not ISeqable
    at seq (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:672:20)
    at first (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:681:16)
    at repl:1:81
    at repl:9:3
    at Socket.<anonymous> ([stdin]:27:80)
    at Socket.EventEmitter.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:746:14)
    at Socket.EventEmitter.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:408:10)
    at emitReadable (_stream_readable.js:404:5)
499999500000

Note, specifically the `499999500000` re-printed after the error.



 Comments   
Comment by David Nolen [ 03/Jan/15 7:28 PM ]

The problem is we're allowing Node.js to print the error instead of letting the REPL infrastructure handle it.

Comment by David Nolen [ 03/Jan/15 7:48 PM ]

fixed https://github.com/clojure/clojurescript/commit/9072b4bfdaf5cf9c66c4770c545f066fbec6256e

Comment by Mike Fikes [ 03/Jan/15 7:54 PM ]

Confirmed fixed in my environment with https://github.com/clojure/clojurescript/commit/9072b4bfdaf5cf9c66c4770c545f066fbec6256e

Comment by David Nolen [ 03/Jan/15 8:04 PM ]

Thanks for the confirm.





[CLJS-957] Parallel compilation Created: 03/Jan/15  Updated: 02/Feb/15

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


 Description   

For non-interacting branches of the dependency graph we could compile in parallel.



 Comments   
Comment by Thomas Heller [ 03/Jan/15 3:06 PM ]

Just a heads-up: I attempted this in shadow-build.

The problem with this is macros, specifically their require. Granted you said non-interacting but suppose two namespaces require cljs.core.async.macros, they might trigger the clojure.core/require at the same time (parallel after all).

This resulted in a very weird exception: https://www.refheap.com/91731

I'm not sure how thread-safe clojure.core/require is supposed to be but it was as close as I was able to get tracking this weird error down. Removing reducers fixed everything. FWIW the gain was minimal to begin with.

Comment by David Nolen [ 03/Jan/15 3:09 PM ]

Making this really work I think largely depends on how much time you are willing to spend on the problem. Fork/join work stealing seems ideal for this not sure if you tried that. RE: macros, seems like you could use information collected from ns analysis to serialize calls to require.

Comment by Thomas Heller [ 03/Jan/15 3:28 PM ]

Lets throw some reducers at it was about as much thought I put into it.

Just beware of macros when you attempt to do something is all I wanted to say. Treating analysis seperate from cljs.compiler/emit might be a good idea though.

Comment by Dusan Maliarik [ 06/Jan/15 7:20 AM ]

I always thought parallel compilation belongs to the build tool (process), and should be parallelized per source file. Would there be any gain in using pmap in here ? It's used by lein-cljsbuild I suppose. Or parallelize this part?

Comment by David Nolen [ 06/Jan/15 7:54 AM ]

pmap won't work because of dependencies. A work queue approach is probably going to be most fruitful. Let's keep this thread clean unless you are actually going to work on this ticket and have a clear picture how to do it. Thanks.

Comment by John Chijioke [ 02/Feb/15 8:14 AM ]

David, Where exactly should the parallelization happen?





[CLJS-956] No *print-fn* fn set for evaluation environment new REPL Created: 03/Jan/15  Updated: 03/Jan/15  Resolved: 03/Jan/15

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None
Environment:

0.0-2644
Node.js REPL
Ubuntu



 Description   

Some evaluations print, like

ClojureScript:cljs.user> (first [1 2 3])
1

But others result in an error regarding print-fn, and interestingly re-print the latest successful print

ClojureScript:cljs.user> (require '[clojure.string :as string])

ClojureScript:cljs.user> (string/join ", " [1 2 3])
"1, 2, 3"
ClojureScript:cljs.user> (time (reduce + (range 1000000)))
Error: No *print-fn* fn set for evaluation environment
    at _STAR_print_fn_STAR_ (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:26:12)
    at string_print (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:8009:4)
    at pr_with_opts (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:8139:4)
    at cljs.core.prn.prn__delegate (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:8191:4)
    at cljs.core.prn.prn (/home/mfikes/clojurescript/.cljs_node_repl/cljs/core.cljs:8190:6)
    at repl:3:15
    at repl:6:3
    at repl:14:3
    at Socket.<anonymous> ([stdin]:27:80)
    at Socket.EventEmitter.emit (events.js:95:17)
"1, 2, 3"


 Comments   
Comment by David Nolen [ 03/Jan/15 10:56 AM ]

This is actually a dupe of http://dev.clojure.org/jira/browse/CLJS-954.

Comment by Mike Fikes [ 03/Jan/15 1:48 PM ]

Confirmed fixed in 0.0-2655.

Comment by David Nolen [ 03/Jan/15 1:53 PM ]

Thanks for the confirmation.





[CLJS-955] Move incorrect *analyze-deps* check Created: 03/Jan/15  Updated: 09/Jan/15  Resolved: 09/Jan/15

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

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File move-incorrect-check.diff    
Patch: Code

 Description   

A new analyze-deps check was added recently [1] which is at the wrong location. When running with analzye-deps false (which shadow-build always does) it will also disable all macro loading. The attached patch moves this check to the "correct" position.

[1] https://github.com/clojure/clojurescript/commit/08805fd519a46ca80aa95b9a50af8c8293df0d3f



 Comments   
Comment by Thomas Heller [ 03/Jan/15 6:20 AM ]

Wait ... this new behavior completely disables all warnings for shadow-build since it runs with analyze-deps false.

I don't think this flag should affect the warnings at all. Either a new flag should be introduced to suppress the warnings or the warning code should be moved to a analyzer pass which can be toggled on demand. Moving more side effects out of parsing is probably a good idea anyways (see CLJS-948).

Willing to provide a patch for either case.

Comment by David Nolen [ 03/Jan/15 10:56 AM ]

I need wrap up some things around REPL support before this can be addressed. Happy to look at it when things have settled down.

Comment by Thomas Heller [ 03/Jan/15 11:15 AM ]

Ok, I'll keep an eye of HEAD and provide a new patch if required.

The issue is pretty straightforward though. If cljs.analyzer/parse-ns should not load macros it should bind cljs.analyzer/load-macros to false, currently analzyze-deps sort of hijacks that flag because of (and analyze-deps load-macros).

Comment by David Nolen [ 03/Jan/15 3:24 PM ]

I don't see a problem for your use case other than the API defaults have changed. Why can't you just set analyze-deps to true and load-macros to false?

Comment by Thomas Heller [ 03/Jan/15 3:31 PM ]

because (and true false) equals false.

Comment by David Nolen [ 03/Jan/15 3:36 PM ]

Sorry not understanding your point. You say you want to analyze deps but you don't want to analyze macros. Then false is the right thing in your case. Not wanting to analyze deps but wanting to load macros doesn't make sense to me.

Comment by Thomas Heller [ 03/Jan/15 3:53 PM ]

I use a completely different approach to compiling in dependency order than CLJS does. Basically I do one discovery pass (extracts goog.provide/require from JS files, parse NS of CLJS file) then sort everything and compile top-down (dependency order). analyze with analyze-deps is recursive and compiles bottom-up. Since we were talking about parallel compile, bottom-up doesn't work parallel.

Also since I control the loop that does the file compilation I can compile in memory without touching files (analyze-deps ALWAYS uses anaylze-file).

Given that I can obviously run with analyze-deps on the second pass through since I compile in dep order the deps are already compiled. I just don't want it to ever enter analyze-deps because it doesn't need to.

Also your change didn't change API defaults it broke the API.

load-macros was meant to to be able to skip the require of macros (since I don't need them on the first pass)
analyze-deps used to control if dependencies should be compiled, now it also controls warnings?

Comment by David Nolen [ 03/Jan/15 5:15 PM ]

Well there's not a true API broken here since none of this stuff is officially anything we support.

analyze-deps never controlled whether dependencies should be compiled, it only performs analysis in dependency order so that warning & optimization information can be properly propagated. It falls out of not necessarily compiling files in dependency order. And I believe the current bottom-up behavior still has utility for analysis tools that aren't involved in building.

Still I see the value of keeping these two bits separate in your case now.

Comment by Thomas Heller [ 03/Jan/15 5:26 PM ]

Sorry mixed up compile with analyze. Of cource analyze-file doesn't compile, still it touches files and does the whole caching bits.

Still analyze-deps didn't use to toggle the warnings, but CLJS-948 will probably address all this anyways.

Side Note: I used the two pass approach in shadow-build since I wanted to discover closure-compliant JS files that aren't mentioned in deps.js. I have a few plain JS dependencies in my app which I converted to be closure-aware when I rewrote my project from JS->CLJS. cljs.closure (or lein-cljsbuild) still don't recognize these unless you configure it very explicitly. I think there is an open issue for this as well.

Comment by David Nolen [ 03/Jan/15 5:37 PM ]

I've committed the following to master https://github.com/clojure/clojurescript/commit/7b683ed43c9316d8163c2f764cc6e9b2710c3f46

Hopefully this clears up how parse-ns is actually used by the compiler, which might have been
a source of confusion. I split apart the two flags, however cljs.analyzer/parse-ns has defaults which differ
from the root bindings of *analyze-deps* and *load-macros* which must be
explicitly over-ridden. It's intention was always as a helper for build tools not for running any actual
analysis or loading any macros.

If this is satisfactory then I'll close this ticket.

Comment by David Nolen [ 09/Jan/15 7:12 AM ]

Fixed in master

Comment by Thomas Heller [ 09/Jan/15 10:27 AM ]

CLJS-948 sort of reintroduces the problem.

It is because we combine *analyze-deps* to mean analyze deps and warn about deps. I don't want shadow-build to analyze deps but I want the warnings. Would you be ok with introducing a *check-deps* or should I find another way to address this in shadow-build?

We should resolve CLJS-948 first, I'll come back to this once we have.





[CLJS-954] require needs to follow Clojure semantics more closely, support :reload, :reload-all Created: 02/Jan/15  Updated: 03/Jan/15  Resolved: 03/Jan/15

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

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


 Description   

Currently :reload-all is implicit, should be explicit.



 Comments   
Comment by David Nolen [ 02/Jan/15 8:33 PM ]

We need :reload-all and :reload support in the ns form. goog.require only supports a single arity, so we can trivially monkey-patch a two arity function. We need to discard this information in any situation other than :none, otherwise will get in the way of static dependency loading. We can and should follow the logic that is already present in Clojure here around loaded-libs.

Comment by David Nolen [ 02/Jan/15 8:51 PM ]

Plan, we can add a loaded-libs for use by REPLs that monkey-patch the behavior of goog.require. For :reload case we can just emit goog.require("foo.core", true); for :reload-all we can save loaded-libs reload everything and merge the new loaded-libs with the old value same as clojure.core/load-all.

Comment by David Nolen [ 03/Jan/15 1:34 PM ]

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





[CLJS-953] require REPL special fn can only take one argument Created: 02/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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


 Description   

Not variadic like Clojure



 Comments   
Comment by David Nolen [ 02/Jan/15 4:46 PM ]

fixed https://github.com/clojure/clojurescript/commit/02524d15db5e25dff4c1934ae4b28a2010ea2fb9





[CLJS-952] Bad type hinting on bit-test Created: 02/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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

Attachments: Text File cljs-952.patch    
Patch: Code

 Description   

bit-test returns a boolean, but is incorrectly type-hinted as returning a number.



 Comments   
Comment by David Nolen [ 02/Jan/15 4:47 PM ]

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





[CLJS-951] goog.require emitted multiple times under Node.js REPL Created: 02/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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


 Description   

This causes a namespace to be required multiple times.



 Comments   
Comment by David Nolen [ 02/Jan/15 3:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/251b76be8d7c072e272ca8d90edec64d6d18d5b6





[CLJS-950] In 0.0-2629, Can not run compiled source with node targert Created: 02/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

Type: Defect Priority: Major
Reporter: tunde ashafa Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

// Compiled by ClojureScript 0.0-2629 {}
#!/usr/bin/env node

Script hashbang on the second line seems to be an issue when running script with node via command line.



 Comments   
Comment by David Nolen [ 02/Jan/15 11:24 AM ]

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

Comment by tunde ashafa [ 02/Jan/15 11:53 AM ]

Thank you





[CLJS-949] In 0.0-2629, Can not run compiled source with node targert Created: 02/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

Type: Defect Priority: Major
Reporter: tunde ashafa Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

// Compiled by ClojureScript 0.0-2629 {} |;; This buffer is for notes you don't want to save, and for Lisp evaluation.
#!/usr/bin/env node



 Comments   
Comment by tunde ashafa [ 02/Jan/15 10:51 AM ]

Please delete this issue. Hit a shortcut to create this ticket prematurely. Updated ticket is #CLJS-950





[CLJS-948] Simplify macro usage Created: 02/Jan/15  Updated: 12/Feb/15  Resolved: 12/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: File cljs-948.diff     File cljs-948-no-passes.diff    

 Description   

The usage of macros from CLJS is very explicit and users of any given namespace must remember whether it comes with macros and which vars are macros. This leads to rather verbose :require statements which are confusing and lead to unexpected results if incomplete (eg. missing :refer-macros).

(ns my-ns
  (:require [my-app.other-ns :as other :refer (something) :refer-macros (something) :include-macros true]))

(something) ;; macro (when :refer-macros, otherwise no macro)
(other/thing) ;; maybe macro (only when :include-macros, otherwise no macro)

I think the user should not need to know whether something is a macro or not, they should just be able to use it.

I implemented a proof-of-concept that

(ns my-ns
  (:require [my-app.other-ns :refer (something)])

will work just find and do the correct thing with regards to macros (assuming the namespace has a corresponding clj namespace, :require-macros is unaffected). No changes to any code are necessary as the implementation uses ana/passes, it is also fully backwards compatible.

Implementation points which should be discussed

  • The CLJS namespace with macros currently has to opt-in through a bit of metadata (eg. (ns my-ns-with-macros {:load-macros true})), that might not be necessary and maybe should default to true.
  • The implementation assumes compilation happens in dependency order. shadow-build always does this, I'm not too sure about CLJS though. Given ana/analyze-deps equals true that is guaranteed but I'm not sure that is always the case.

If there is any interest I can provide a patch, until then refer to the proof-of-concept [1].

[1] https://github.com/thheller/shadow-build/blob/f37cfa598f1e90dd66e333d1e45580ea25650025/src/clj/shadow/cljs/passes.clj#L30-L82



 Comments   
Comment by David Nolen [ 02/Jan/15 11:25 AM ]

This is interesting. Will think about it.

Comment by Thomas Heller [ 02/Jan/15 1:15 PM ]

Is it ok if I leave implementation notes here?

Things that would need addressing should this be implemented:

  • cljs.analyzer/check-uses runs when parsing the ns form, since the macro passes run AFTER the parsing finished the check-uses will warn incorrectly if a :refer is not defined in CLJS but is a macro. Solution: check-uses should probably just become a pass itself which runs after the macro ones. Moving a side-effect out of the parse function is probably well worth it.
  • cljs.analyzer/load-macros would obsolete and the entire functionality it was toggling can be moved to the load-macros pass.
Comment by David Nolen [ 03/Jan/15 3:33 PM ]

I looked a bit at your code. It looks interesting. Definitely not going to do the namespace metadata bit. It seems to me this should work only if the required namespace loaded macros from a Clojure namespace of the same name.

Comment by Thomas Heller [ 03/Jan/15 3:36 PM ]

Yeah, probably. Also played with the idea of just looking for (io/resource (str (ns->path) ".clj")). If that is nil the namespace doesn't have macros, if its not null we require it.

But I like your approach of just using (ns my-ns (:require-macros [my-ns])) more.

Comment by David Nolen [ 03/Jan/15 3:40 PM ]

OK, let's try a patch for this. This is an interesting simplification that unifies Clojure & ClojureScript macro usage.

Comment by Thomas Heller [ 03/Jan/15 4:10 PM ]

Yay! Coming right up.

One issue though: I thought it would be a good idea to put the passes into a separate file (eg. cljs.analyzer.passes), that is currently not possible due to cljs.analyzer/analyze (it controls the passes defaults) [1]. cljs.analyzer cannot depend on cljs.analyzer.passes since it most likely depends on cljs.analyzer (circular dependency). Technically the passes currently only require the analyzer because of ::ana/namespaces but faking out a circular dependency is not a good idea.

If we remove the (or passes [infer-type]) and instead force opts to contain :passes (or bind passes beforehand) we should be fine.

Although I'm perfectly fine with putting the new passes into cljs.analyzer, just thought it would be cleaner not to.

Also, how should I address the invalid refer warning issue? Since the check-uses runs before the pass it can never find the macro. I was going to make check-uses a pass in itself if thats alright with you.

[1] https://github.com/clojure/clojurescript/blob/2d1e2167f5ab8bd76ac5c8bafd198990cc88d34a/src/clj/cljs/analyzer.clj#L1711

Comment by Thomas Heller [ 03/Jan/15 5:01 PM ]

I cleaned up the reference in shadow-build [1]. This is how a cljs.analyer.passes could look, the default passes would then be

[cljs.analyzer.passes/load-macros
 cljs.analyzer.passes/infer-macro-require
 cljs.analyzer.passes/infer-macro-use
 cljs.analyzer.passes/check-uses
 cljs.analyzer/infer-type]

This would basically make ana/load-macros obsolete since this can now be controlled via passes. Also addresses CLJS-955.

[1] https://github.com/thheller/shadow-build/blob/220d3bb0bef2cdb5696487b639ca5aaa169c56f2/src/clj/shadow/cljs/passes.clj

Comment by David Nolen [ 03/Jan/15 5:40 PM ]

This looks OK to me at the moment.

Comment by Thomas Heller [ 03/Jan/15 5:40 PM ]

Should stop refering to specific hashes, please see [1].

There is a slight issue with cljs.core itself. There is a bit of code that makes cljs.core macros special. I'm not quite sure why this is, since it would not be required if it did.

(ns cljs.core
  (:require-macros [cljs.core]))

Getting an error [2] on the recently changed def to (defonce print-fn). Haven't exactly figured out why this is yet.

[1] https://github.com/thheller/shadow-build/blob/master/src/clj/shadow/cljs/passes.clj
[2] https://www.refheap.com/61bc05e4b4a74a3b573797721

Comment by Thomas Heller [ 03/Jan/15 5:44 PM ]

Wait, are other namespaces allowed to use their own macros unqualified?

(ns my-ns
  (:require-macros [my-ns]))

(this-is-a-macro)
;; vs
(my-ns/this-is-a-macro)

I don't think so? The recent defonce seems to be an exception?

Comment by David Nolen [ 03/Jan/15 5:50 PM ]

yes cljs.core and core macros are an exception

Comment by Thomas Heller [ 03/Jan/15 5:54 PM ]

Yeah, but cljs.core calls all macros directly.

(defn ^boolean identical?
  "Tests if 2 arguments are the same object"
  [x y]
  (cljs.core/identical? x y))

The recently changed defonce vars are the exception here. Although even if I change them to cljs.core/defonce I still can't explain the error.

Comment by David Nolen [ 03/Jan/15 5:56 PM ]

The inlining macros are an exception, it's done that way to break circularity which would otherwise stack overflow the compiler.

Comment by Thomas Heller [ 03/Jan/15 7:09 PM ]

Alright, resolved that macro issue.

I updated [1] so the analyzer passes are initialized correctly and outside cljs.analyzer. The patch would also change the cljs.analyzer/parse 'def since that functionality is moved to cljs.analyzer.passes.

(when (and *analyze-deps* (seq @deps))
      (analyze-deps name @deps env opts))
    #_ (when (and *analyze-deps* (seq uses))
         (check-uses uses env))
    (set! *cljs-ns* name)
    #_ (when *load-macros*
         (load-core)
         (doseq [nsym (concat (vals require-macros) (vals use-macros))]
           (clojure.core/require nsym))
         (when (seq use-macros)
           (check-use-macros use-macros env)))

The question is where do we require cljs.analyzer.passes (must be required somewhere so the default is configured correctly)?

cljs.closure is probably the best bet.

Not a big fan of this alter-var-root but I'm not sure how we could do without. I doubt anyone but me currently uses cljs.analyzer/passes and therefore the default behavior of (or passes [infer-type]) always triggers. Since that would no longer work correctly we need to initialize to another default.

cljs.analyzer/parse-ns would also need to change again given the removal of load-macros. Same functionality can be achieved within the same binding though.

[1] https://github.com/thheller/shadow-build/blob/master/src/clj/shadow/cljs/passes.clj

Comment by Thomas Heller [ 03/Jan/15 8:16 PM ]

My head hurts, I need to get some sleep.

I cannot figure out why the attached patch does not work, but ./script/test produces errors in parse-ns.

Will get back to it tommorrow.

Exception in thread "main" java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Symbol, compiling:(/Users/zilence/code/oss/clojurescript/bin/../bin/cljsc.clj:18:68)
	at clojure.lang.Compiler.load(Compiler.java:7142)
	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
	at clojure.main$load_script.invoke(main.clj:274)
	at clojure.main$script_opt.invoke(main.clj:336)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:436)
	at clojure.lang.Var.invoke(Var.java:388)
	at clojure.lang.AFn.applyToHelper(AFn.java:160)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: Don't know how to create ISeq from: clojure.lang.Symbol
	at clojure.lang.RT.seqFrom(RT.java:505)
	at clojure.lang.RT.seq(RT.java:486)
	at clojure.core$seq.invoke(core.clj:133)
	at cljs.analyzer$parse_ns$fn__1660.invoke(analyzer.clj:1757)
	at cljs.analyzer$parse_ns.invoke(analyzer.clj:1747)
	at cljs.analyzer$parse_ns.invoke(analyzer.clj:1741)
	at cljs.compiler$compile_root.invoke(compiler.clj:1069)
	at cljs.closure$compile_dir.invoke(closure.clj:358)
	at cljs.closure$eval3084$fn__3085.invoke(closure.clj:398)
	at cljs.closure$eval3020$fn__3021$G__3011__3028.invoke(closure.clj:306)
	at cljs.closure$eval3071$fn__3072.invoke(closure.clj:412)
	at cljs.closure$eval3020$fn__3021$G__3011__3028.invoke(closure.clj:306)
	at cljs.closure$build.invoke(closure.clj:1018)
	at cljs.closure$build.invoke(closure.clj:972)
	at user$eval3279.invoke(cljsc.clj:21)
	at clojure.lang.Compiler.eval(Compiler.java:6703)
	at clojure.lang.Compiler.load(Compiler.java:7130)
	... 9 more
Comment by Thomas Heller [ 04/Jan/15 5:40 AM ]

That is why you should probably not write code tired at 3am. Was using infer-tag as a pass instead of infer-type.

I decided to drop the cljs.analyzer.passes namespace and instead moved the passes to cljs.analyzer. The extra namespace introduced a weird circular dependency which cljs.analyzer/parse-ns couldn't quite handle.

The patch now applies cleanly, would probably be nice to have a test that actually tests that it does what its supposed to.

Comment by David Nolen [ 07/Jan/15 8:21 AM ]

The patch has not been updated with these changes.

Comment by Thomas Heller [ 07/Jan/15 8:37 AM ]

Sorry, my mistake. Fixed.

Comment by Thomas Heller [ 08/Jan/15 7:19 AM ]

One issue I found is related to caching. Depends on how/where cljs.analyzer/parse-ns is called.

If the new passes aren't executed and the result of that analysis are used to write out the cache it won't contain the new macros information. I believe cljs.analyzer/parse-ns does this.

On the next run if it was decided to use the cache, we are missing some information AND won't require the macros. This was true before the changes but since the dependant namespaces would require the macros on their own the issue was somewhat hidden. Now they assume that macros were already loaded, therefore the must require macros when reconstructing from cache.

How to do that best in CLJS I don't know since shadow-build does its own caching. The passes could be refactored so they can be called with the cached information as well.

Note that this issue was basically introduced by load-macros (CLJS-940), but since that flag is already gone it is now moved to the :load-macros option of parse-ns.

The simplest way to address this would be to always run the new passes, basically how it was before CLJS-940.

Comment by David Nolen [ 08/Jan/15 7:39 AM ]

Thomas to me this illustrates the pitfalls of trying to bring a multipass approach to an existing compiler that isn't already fundamentally multipass. It seems to me that an alternative approach is possible without bothering at all about passes. In the analyzer parse 'ns case for each required CLJS lib you could leverage parse-ns to see if it requires a macro file of the same name - if it does add an implicit :require-macros.

While I like the passes approach in theory it will just make it harder for contributors to understand the structure of the compiler, they have to look at two different approaches to determine where things happen.

Comment by Thomas Heller [ 08/Jan/15 7:56 AM ]

Well, yeah we could to away with the passes and do it all in (defmethod parse 'ns ...) but that doesn't change the issue as long as there is a way to toggle this at all (ie. :load-macros or load-macros). It is not really about whether it is in a pass or not.

I just liked using passes since it sort of allows cherry-picking what you want and don't want to run. infer-types doesn't need to run in parse-ns for example, but it always did before this.

But I'm not committed to passes, happy to move everything so it runs in the parse method.

Let me know, if I should rewrite the patch.

Comment by Thomas Heller [ 08/Jan/15 8:35 AM ]

Hmm, you are right. Will move it out of the passes.

The only reason it was in there in the first place is due to shadow-build. It was the only place I could put it to make it work without patching CLJS. But since we agree that this should be in core the passes option really doesn't make sense any more.

Going to deliver a new patch soon.

Comment by David Nolen [ 08/Jan/15 8:37 AM ]

The difference for inter-types is that is something that needs to be run for a large number of nodes - it's a real global pass.

Let's go with moving the logic in to parse 'ns. I still don't understand the *load-macros* issue you are describing. We absolutely need to be able to disable this and dependency analysis for legitimate uses of parse-ns.

Comment by Thomas Heller [ 08/Jan/15 9:42 AM ]

Attached the no-passes version. The does get a bit simpler since it doesn't have to manually patch the compiler env.

I'm not totally sure how the @reload logic works, maybe some adjustments are required.

Comment by Thomas Heller [ 08/Jan/15 9:53 AM ]

The *load-macros* issue manifests as follows:

(ns demo.ns1
  (:require-macros [demo.ns1 :as m]))

(ns demo.ns2
  (:require [demo.ns1 :as x :refer (a-macro)]))

Compile 1: Everything is fine, cache is written.
Compile 2:

  • ns1 not modified, parse-ns decides to use cached version
  • ns2 modified, compiler attempts to compile, fails with NullPointerException
Caused by: java.lang.NullPointerException
	at cljs.analyzer$get_expander.invoke(analyzer.clj:1532)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.clj:1543)
	... 80 more

This is because the load from cache does not require the macro namespace. Before these changes demo.ns2 would have had a :require-macros or equivalent in the ns form which would then have require'd the macro ns. Now only demo.ns1 would trigger the require, but since it loads from cache it doesn't UNLESS *load-macros* is true. Since parse-ns does revive the cache (I think) it is not.

The require must be triggered somewhere before actual compilation happens. In shadow-build this is pretty simple but I'm not sure where to put this for CLJS.

Comment by Thomas Heller [ 09/Jan/15 10:25 AM ]

As per IRC the *load-macros* issues does not present in CLJS since it is true when it needs to be.

Comment by David Nolen [ 01/Feb/15 4:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/28e6070691e89f5fe35ae8f8a196ae1048fbf07d

Comment by Thomas Heller [ 01/Feb/15 9:11 PM ]

Nice work, your solution looks much better.

Comment by Thomas Heller [ 04/Feb/15 5:48 AM ]

I take that back, I no longer like the solution since you added the call to parse-ns, which basically supersedes the *analyze-deps* option.

Since you cannot guarantee that files are processed in dependency order the extra parse-ns call will create an entry in the compiler env for the namespace, analyze-deps will then skip the analyze-file since parse-ns already created the entry (which is incomplete since it stopped at the ns)

In addition the sequence of calls is wrong and check-uses does not work correctly since it does not check for macros.

(ns macro-use.bug
  (:require [macro-use.core :as core :refer (a-macro)]))

Since check-uses now throws this will result in

clojure.lang.ExceptionInfo: Referred var macro-use.core/a-macro does not exist at line 1 src/macro_use/bug.cljs {:tag :cljs/analysis-error, :file "src/macro_use/bug.cljs", :line 1, :column 1}

Demo repo here:
https://github.com/thheller/macro-use-bug/blob/master/src/macro_use/bug.cljs

Comment by David Nolen [ 04/Feb/15 6:49 AM ]

parse-ns does not change the compiler environment by default nor is it recursive by default. All that it is used for in this context is to extract namespace information, nothing more or less.

Expecting :refer to work like that for macros was never under consideration for this issue. I should have been more clear about that. We can open a new minor enhancement issue for this addition.

Comment by Thomas Heller [ 04/Feb/15 7:05 AM ]

parse-ns calls analyze [1] which calls (defmethod parse 'ns ...) which does a swap! [2] and therefore touches the compiler environment?

[1] https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L1818
[2] https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L1357

When opening the issue my goal was to make everything as close to Clojure as possible which included the transparent handling of :refer for macros. All my patches included that functionality so I'm slightly confused that this was not "under consideration". Especially since my example basically consisted of exactly that use-case. We can take this to another issue if we must ...

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

Thomas the previous environment gets restored, https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L1840.

Because parse-ns is not recursive by default there are no problems.

Like I said sorry I wasn't clear about what I wanted to see implemented. The :refer bit is just nice sugar and not fundamental. Please open a separate ticket.





[CLJS-947] REPL require of goog namespaces does not work Created: 01/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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


 Comments   
Comment by David Nolen [ 02/Jan/15 4:30 PM ]

fixed https://github.com/clojure/clojurescript/commit/935b0e466c6bd327799ca0a04f062b8aa23dcebe





[CLJS-946] goog.require in REPLs will not reload recompiled libs Created: 01/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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


 Description   

This makes for a bad REPL experience. (require 'foo.core) will recompile, but the emitted goog.require does not correctly reload the already seen namespace. We should monkey patch goog.require for all REPLs so that this works correctly.



 Comments   
Comment by David Nolen [ 01/Jan/15 4:53 PM ]

This is actually like a problem with Node.js integration, we need to invalidate the cache http://nodejs.org/docs/latest/api/globals.html#globals_require_cache

Comment by David Nolen [ 01/Jan/15 10:23 PM ]

fixed https://github.com/clojure/clojurescript/commit/4665f9a787c2bcb9d76066ba9ddccb89cf378dc3

Comment by David Nolen [ 01/Jan/15 10:25 PM ]

Not quite fixed, the paths are correct, but it appears goog.require has some logic that prevents from even getting to Node.js require

Comment by David Nolen [ 02/Jan/15 1:04 PM ]

fixed https://github.com/clojure/clojurescript/commit/187a45ff599eafd9e8b4f95bb31778a84ac9322f





[CLJS-945] Compile core with :static-fns true by default Created: 01/Jan/15  Updated: 03/Jan/15  Resolved: 03/Jan/15

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

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


 Comments   
Comment by David Nolen [ 03/Jan/15 7:06 PM ]

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





[CLJS-944] deps file produced by :none needs compilation comment Created: 01/Jan/15  Updated: 02/Jan/15  Resolved: 02/Jan/15

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

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


 Description   

Without this a different version of the compiler believes that recompilation is not necessary.



 Comments   
Comment by David Nolen [ 01/Jan/15 2:31 PM ]

Fixed in master but is not the root reason project is not recompiled. Needs more investigation.

Comment by David Nolen [ 02/Jan/15 11:28 AM ]

This is a bug in cljsbuild. cljsbuild does it own file change detection to trigger rebuilds - this is at odds with the compiler checking compiled artifacts for staleness.





[CLJS-943] REPL require special fn is brittle Created: 31/Dec/14  Updated: 01/Jan/15  Resolved: 01/Jan/15

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

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


 Description   

Spec merging needs to be more robust. Additionally all errors during ns form parsing should throw not just print a warning. Otherwise the information about the ns due to REPL interaction can enter a corrupted state.



 Comments   
Comment by David Nolen [ 01/Jan/15 12:41 PM ]

Fixed https://github.com/clojure/clojurescript/commit/1a7546ecba81683b67278a47dec490761e13718e





[CLJS-942] Randomized port for Node.js REPL Created: 31/Dec/14  Updated: 31/Dec/14  Resolved: 31/Dec/14

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

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


 Description   

We need to randomize the port so the user can start multiple repls without bother to reason about which port to use.



 Comments   
Comment by David Nolen [ 31/Dec/14 4:21 PM ]

fixed https://github.com/clojure/clojurescript/commit/133721a20a4879253b5ce5f913bd2f6555b8e3c6





[CLJS-941] Warn when a symbol is defined multiple times in a file Created: 31/Dec/14  Updated: 19/Feb/15  Resolved: 19/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Shaun LeBron Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 941-fix2.patch     Text File 941-fix.patch     Text File 941.patch    

 Description   

The only reason Clojure allows vars to be re-def-ed is so you can fix bugs in running programs.
- Rich Hickey

(ns example.core)
(defn foo [] "hi")
(defn foo [] "hello")

;; WARNING: foo at line 2 is being replaced at line 3 src/example/core.cljs

;; The warning only applies to files, not REPLs.
;; Also ignoring the "cljs" namespace since many symbols are redefined there.



 Comments   
Comment by Shaun LeBron [ 31/Dec/14 11:53 AM ]

Included a short patch. Is ignoring the entire `cljs` namespace a good idea? I noticed a lot of redefs in there.

Comment by David Nolen [ 31/Dec/14 12:36 PM ]

What is the intent of the cljs-file check? Also the exclusion of core is not desirable. I looked at the warnings and most of them are legitimate problems in cljs.core with the exception of ITER_SYMBOL and imul. These two are problematic cases for this patch as they represent a real useful pattern I expect to appear in many ClojureScript codebases.

I think the warning should only be emitted if the defs are not embedded in a conditional expression.

Comment by Shaun LeBron [ 31/Dec/14 2:05 PM ]

updated patch. The cljs-file check was an attempt to prevent the warning when redefining symbols from a REPL. But I just changed it to check if it's "<cljs repl>".

I'll look into preventing the warning for defs in conditional expressions.

Should we correct the duplicated function definitions in cljs.core in a separate patch?

Comment by Shaun LeBron [ 31/Dec/14 2:09 PM ]

the updated patch also prevents this warning if a `declare` comes after a `def`. No harm there.

Comment by David Nolen [ 31/Dec/14 2:15 PM ]

I'd rather just have a single squashed patch. Agree that declare after def is harmless and this is in line with the behavior of Clojure.

Comment by Shaun LeBron [ 31/Dec/14 4:14 PM ]

updated patch:

  • removed the redefs for cljs.core [sequence rand rand-int] that triggered redef warnings
  • allow redefs inside an if-block for conditional def'ing (using a dynamic var allow-redef)
Comment by David Nolen [ 31/Dec/14 4:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/378a75e4b812ebc5a3596bc94d1afd3f6fb1506f

Comment by David Nolen [ 31/Dec/14 4:42 PM ]

Shaun, reopening as I had to disable this because it still interacts badly with REPLs. To see the problem reenable the warning and follow the instructions here: http://swannodette.github.io/2014/12/31/the-old-way-the-new-way/

Comment by Shaun LeBron [ 31/Dec/14 6:18 PM ]

updated patch.

I saw the warnings for every symbol after starting the rhino REPL. It was compiling the same files from local maven and then ".repl/" for some reason, triggering the redef warnings.

To fix, I just added a check to trigger the warning only if we're overriding a symbol from the same file path, which is the only case we want to consider anyway.

Comment by David Nolen [ 31/Dec/14 6:48 PM ]

Thanks for the extra information, will look into why an extra compile is being triggered.

Comment by David Nolen [ 01/Jan/15 12:46 PM ]

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

Comment by David Nolen [ 01/Jan/15 1:21 PM ]

I've had to disable this again in master. Running the Rhino REPL from a checkout still spills warnings. Before turning this back on it's going to need a lot more testing under the various REPLs and incremental build scenarios.

Comment by Shaun LeBron [ 01/Jan/15 6:25 PM ]

Was able to reproduce. repl-rhino is now binding relative file paths to cljs-file for some reason, causing the filename inequality check to fail.

A general solution is for the analyzer to uniquely identify a file "session", so redefs emit warnings within a session, but not across them to allow redefs from the REPL or file-reloads.

This patch binds a new cljs-file-session var to a timestamp alongside cljs-file. Seems to have worked for the following cases:

  • tested in repl.rhino
  • tested in repl.node
  • tested in figwheel
  • tested in weasel
Comment by Andy Fingerhut [ 02/Jan/15 10:46 AM ]

Eastwood only works for Clojure on the JVM right now, and includes a warning like this because they seemed unwilling to include such a thing in Clojure itself, particularly due to interactive reloading of files during many people's typical development workflows. ClojureScript might be enough different that it makes more sense to include it in the compiler, but the number of times it has been undone makes me wonder.

Comment by Shaun LeBron [ 02/Jan/15 12:05 PM ]

I think since the analyzer functions work in sessions (i.e. one file at a time, or form at time in REPL), we can get the benefit of this warning in the compiler itself by preventing redefs on a session basis. Just took me a few tries to realize that.

Would be nice as a default capability in the compiler since its expected behavior for most compilers to detect this. Thanks for insight on Clojure and Eastwood!

Comment by David Nolen [ 03/Jan/15 2:04 PM ]

Shaun the patch is interesting but I'm concerned about using System/currentTimeMillis for the identifier. One idea that's been rolling around my head for some time is parallel compilation. This is not going to work for that. Is there any reason not to use a proper sentinel?

Comment by Shaun LeBron [ 03/Jan/15 6:09 PM ]

good point, updated fix2 patch:

  • use a uuid instead of timestamp for the file session
  • for extra caution, don't warn redefs if file session is unbound (nil)
Comment by David Nolen [ 10/Feb/15 11:44 PM ]

Can we get a patch rebased on master and can we use a proper sentinel instead of UUID? i.e. (Object.). Thanks.

Comment by Shaun LeBron [ 11/Feb/15 1:35 AM ]

updated 941-fix2.patch to use a (Object.) for sentinel and rebased on master.

Comment by David Nolen [ 11/Feb/15 12:33 PM ]

Looks like this patch needs to be rebased to master again.

Comment by Shaun LeBron [ 11/Feb/15 2:01 PM ]

rebased patch.

I tested this rebased patch against a "mies" project, and it is failing after a second compile with an "Unreadable form" exception in the project's core.cljs. I will have to look at this tonight.

Comment by Shaun LeBron [ 12/Feb/15 1:26 AM ]

Problem: the analyzer was caching the session sentinels as edn. The cache couldn't be read back correctly since they were Object literals.

Solution: I updated the patch to dissoc the file session sentinels from the analysis before caching.

Details: We don't need to cache the file session ids because they don't need to exist after a session is complete. They simply need to be different from the current session id, so nil meets this requirement. If the current session is nil for some reason, then we ignore redefs.

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

I don't understand why *file-session* ever made it into analysis information. Why isn't this being done just purely via dynamic binding? As far as I can tell there is no reason to capture this ever. i.e. what's wrong with an atom containing a set that you dynamically bind around the body of cljs.analyzer/analyze-file and cljs.compiler/compile-file that simply tracks ns defs?

Comment by Shaun LeBron [ 12/Feb/15 8:17 PM ]

Yeah, I agree; I kind of rube-goldberg'd this one. Will update the patch when I get a chance, hopefully tonight.

Comment by Shaun LeBron [ 13/Feb/15 5:11 PM ]

updated. using file-defs to hold the set atom, bound in the body for analyze-file and compile-file.

Comment by David Nolen [ 18/Feb/15 7:04 PM ]

Sorry can we get a rebased patch? The patch looks great and will apply right away this time.

Comment by Shaun LeBron [ 19/Feb/15 10:30 AM ]

np, rebased the patch and tested it

Comment by David Nolen [ 19/Feb/15 10:46 AM ]

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





[CLJS-940] Make macro loading of the 'ns form optional Created: 31/Dec/14  Updated: 31/Dec/14  Resolved: 31/Dec/14

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

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File optional-macro-loading.diff    

 Description   

When parsing the (ns ...) form it has the costly side effect of immediately require'ing all macro namespaces it finds. While this is nescessary for parsing the rest of the file it is not required when just parsing the ns form.

Tools should be able to disable the loading of the macros to allow faster inspection of the ns form without this side-effect.

In shadow-build I have a function which basically does what cljs.build.api/parse-js-ns does just for ClojureScript files, this function is slow initially and I'd like be able to delay the require of macro namespaces as they are not required when just looking at the ns form.



 Comments   
Comment by Thomas Heller [ 31/Dec/14 11:16 AM ]

Whoops, the first patch didn't come out right. Fixed.

Comment by David Nolen [ 31/Dec/14 12:24 PM ]

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





[CLJS-939] Quit noderepljs with EOF Created: 31/Dec/14  Updated: 09/Feb/15  Resolved: 09/Feb/15

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

Type: Defect Priority: Minor
Reporter: Dusan Maliarik Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Description   

As is common in every other REPL out there, Ctrl+D on a new line should quit the REPL



 Comments   
Comment by David Nolen [ 09/Feb/15 6:30 PM ]

CLJS-989





[CLJS-938] actual configurable port for Node.js REPL Created: 30/Dec/14  Updated: 31/Dec/14  Resolved: 31/Dec/14

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

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


 Description   

Currently hardcoded on the Node.js side to 5001



 Comments   
Comment by David Nolen [ 31/Dec/14 4:27 PM ]

fixed by CLJS-942 https://github.com/clojure/clojurescript/commit/133721a20a4879253b5ce5f913bd2f6555b8e3c6





[CLJS-937] local fn name should be namespace munged Created: 30/Dec/14  Updated: 30/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   
(ns foo.bar)

(fn console []
  ...)

The local name should be foo$bar$console



 Comments   
Comment by David Nolen [ 30/Dec/14 3:12 PM ]

See CLJS-833





[CLJS-936] Multi arity bitwise operators Created: 30/Dec/14  Updated: 06/Jan/15  Resolved: 06/Jan/15

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

Type: Defect Priority: Trivial
Reporter: Justin Tirrell Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 936.patch     Text File 936.patch    
Patch: Code

 Description   

Unlike regular Clojure, bitwise operators only support two arguments

(bit-or 0 1)

not

(bit-or 0 0 1)



 Comments   
Comment by David Nolen [ 30/Dec/14 1:14 PM ]

Looks good but can we get a new patch that includes tests? Also have you sent in your CA? Thanks!

Comment by Justin Tirrell [ 30/Dec/14 2:13 PM ]

Just attached a new patch that includes tests. I submitted the CA yesterday, should I attach it here?

Comment by David Nolen [ 06/Jan/15 1:39 PM ]

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





[CLJS-935] script/noderepljs leaves node running after exit Created: 30/Dec/14  Updated: 30/Dec/14  Resolved: 30/Dec/14

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

Type: Defect Priority: Major
Reporter: Michael Nygard Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

After starting noderepljs, then exiting it (via :cljs/quit), the node process is left running and listening on port 5001. This causes all subsequent executions of noderepljs to report "Error: listen EADDRINUSE"

clojurescript master mn › ./script/noderepljs
To quit, type: :cljs/quit
ClojureScript Node.js REPL server listening on 5001
ClojureScript:cljs.user> :cljs/quit

clojurescript master mn › ps
PID TTY TIME CMD
17400 ttys000 0:00.10 -bash
20391 ttys000 0:00.17 node
17634 ttys001 0:00.06 -bash

clojurescript master mn › ./script/noderepljs
To quit, type: :cljs/quit
ClojureScript Node.js REPL server listening on 5001
Error: listen EADDRINUSE
at errnoException (net.js:904:11)
at Server._listen2 (net.js:1042:14)
at listen (net.js:1064:10)
at Server.listen (net.js:1138:5)
at [stdin]:42:4
at Object.<anonymous> ([stdin]-wrapper:6:22)
at Module._compile (module.js:456:26)
at evalScript (node.js:536:25)
at ReadStream.<anonymous> (node.js:154:11)
at ReadStream.emit (events.js:117:20)
goog.require could not find: cljs.core
Error: goog.require could not find: cljs.core
at Error (<anonymous>)
at Object.goog.require (/Users/mtnygard/work/clojurescript/.cljs_node_repl/goog/base.js:470:13)
at repl:1:6
at Socket.<anonymous> ([stdin]:26:80)
at Socket.emit (events.js:95:17)
at Socket.<anonymous> (_stream_readable.js:764:14)
at Socket.emit (events.js:92:17)
at emitReadable_ (_stream_readable.js:426:10)
at emitReadable (_stream_readable.js:422:5)
at readableAddChunk (_stream_readable.js:165:9)
ClojureScript:cljs.user>



 Comments   
Comment by David Nolen [ 30/Dec/14 11:22 AM ]

fixed https://github.com/clojure/clojurescript/commit/86fa7da377c94aa9bab5b7f53374d16aa9eb2298





[CLJS-934] In the REPL return vars after defs Created: 30/Dec/14  Updated: 14/Jan/15

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

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


 Description   

We don't want to emit these in during normal compilation. However it's nice to unify the REPL experience with Clojure's. Currently we just display the value of the def. REPLs could set a :repl build flag which is checked by the emit* :def case. For this to work the analyzer should compile the var AST and include it in the def AST so the compiler can optionally use it.



 Comments   
Comment by Mike Fikes [ 14/Jan/15 4:03 PM ]

A change was recently made to introduce new behavior if a :repl-verbose flag is set.

Perhaps all flags that control REPL behavior could be prefixed with :repl-, with the flag controlling this ticket controlled by :repl-def-vars or somesuch.





[CLJS-933] Redef warning omiting namespace of original symbol Created: 29/Dec/14  Updated: 12/Feb/15  Resolved: 12/Feb/15

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