<< Back to previous view

[CLJS-489] Browser-REPL automation & usability enhancements Created: 22/Mar/13  Updated: 05/Aug/13  Resolved: 05/Aug/13

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

Type: Enhancement Priority: Minor
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Declined Votes: 2
Labels: None

Attachments: File CLJS-489.diff    
Patch: Code

 Description   

The current implementation of browser-REPL yields a number of usability problems:

  • only one browser-REPL can be used at a time due to globals used to track REPL state
  • Browser-REPL currently involves two web servers: the one you're serving your app from, and the one that the REPL itself uses
    • This makes setup difficult in the interactive case — especially for newcomers, the less experienced, or the tired
    • Perhaps more significantly, coordinating the two servers and their associated state is less than easy if one wants to automate the browser-REPL for testing of DOM-related cljs code

Attached is a patch that addresses these issues:

  • Multiple concurrent browser-REPLs can be safely used
  • The browser-REPL's HTTP server is now always-on
  • Replaced the custom HTTP server with com.sun.net.httpserver.* bits, which are a part of J2SE 6+, not random implementation details http://docs.oracle.com/javase/7/docs/technotes/guides/net/enhancements-6.0.html
  • Each browser-REPL session supports a new top-level "entry" URL that can be used to easily start the REPL in a browser or other JS runtime (i.e. you don't need to have a separate webapp running to initiate the browser-REPL connection)
  • The entry (and REPL) URLs are available in slots on the browser-REPL's environment, making it trivial to automate browser-REPL sessions with e.g. phantomjs or a headless chromium or etc. This pattern is implemented by a new `exec-env` function; the default is to start the REPL in phantomjs using a temporary script, but it's super-easy to use a regular browser. e.g. this will use Chrome on OS X in the background:
(cljs.repl/repl (cljs.repl.browser/exec-env
                  :exec-cmds ["open" "-ga" "/Applications/Google Chrome.app"]))

(This enhancement has been discussed previously {here|http://groups.google.com/group/clojurescript/browse_thread/thread/963d6b7334bdf61c}.)



 Comments   
Comment by Chas Emerick [ 22/Mar/13 3:24 AM ]

Aside: if this is accepted, a fair bit of documentation on the github wiki (and maybe dev.clojure.org wiki?) will need to be updated (something I'm happy to do FWIW).

Comment by Chas Emerick [ 22/Mar/13 3:54 PM ]

I just discovered that cljs.repl.reflect depends directly upon cljs.repl.server. I'll have to unroll that in order to be able to remove the latter...

Comment by Brenton Ashworth [ 29/Mar/13 5:24 PM ]

Thanks for working on this Chas. This has been in need of improvement for a long time. Can't wait to take this for a spin.

Comment by Chas Emerick [ 30/Mar/13 8:43 AM ]

I've never used cljs.repl.reflect, so I had to take a close look at what it supports, thus the delay.

First, background: cljs.repl.reflect provides backing support for two operations:

  • getting the documentation for a cljs "var" (i.e. any top-level function or other definition), and
  • obtaining the macroexpansion of a Clojure macro as provided by the ClojureScript analyzer

These operations are exposed within the ClojureScript REPL via clojure.reflect/doc and clojure.reflect/macroexpand, respectively.

I see a couple of issues here. First, aside from the problems in their current implementations (e.g. clojure.reflect/doc does not re-sugar arglists prior to printing them), both of these operations (doc and macroexpand) can simply be provided by macros, eliminating any dependence on a particular REPL environment/implementation. e.g., this does well for `doc`:

(defmacro doc
  [fq-sym]
  (let [meta (get-meta fq-sym)]
    `(do
       (println "-------------------------")
       (println ~(:name meta))
       (println ~(pr-str (map (partial mapv :name) (read-string (:method-params meta)))))
       (println " " ~(:doc meta)))))

A similar macro for macroexpand is trivial to produce.

Second, doc and macroexpand just don't belong in clojure.reflect. AFAICT, clojure.* namespaces in ClojureScript should be maximally isomorphic to their Clojure namesakes. This means that doc should be in clojure.repl; placing macroexpand is a little trickier, but perhaps a default-referred macro in cljs.core would make sense? (clojure.core/macroexpand is obviously already taken.)

So, my tl;dr plan (which I'll make concrete in a patch after circulating this comment on various mailing lists to get feedback) is:

1. reimplement doc and macroexpand as macros in more appropriate namespaces
2. rm the current cljs.repl.reflect and clojure.reflect namespaces

This should clear the way for the improved browser-REPL to land.

Comment by Chas Emerick [ 14/Jun/13 10:45 AM ]

Disregard the above plan; later discussion here clarified the objectives of the browser REPL reflect bits, and contains a basic outline of the plan of attack for achieving them alongside the browser-REPL reimplementation.

Comment by Chas Emerick [ 14/Jun/13 2:59 PM ]

New patch attached that retains reflection capabilities working with the new browser-repl backend. As discussed in the previously-linked ML discussion, the user-facing reflection namespace is now cljs.repl.browser.reflect. Otherwise, all usage instructions discussed previously apply as before for phantomjs, head-ful browsers, and other external processes via exec-env.

Two things:

  • I am seeing a bunch of gclosure compilation warnings on master. They're ugly, but not seeming to affect functionality. The CLJS changes I've made are minimal, so I'll just chalk them up to transient fickleness of master for now...
  • Since reflection requests are entirely asynchronous (i.e. responses are processed via a callback on an xhr-connection), there's no way to either (a) print them nicely in a REPL context (the prompt is often going to be interspersed), and (b) those results will never end up in e.g. *1. This is how the prior reflection utilities were, and I assume that that is intentional/desirable/low priority at the moment; in any case, I'd consider improvement of the reflection facilities (outside of the minor cleanup I've done) to be out of scope for this particular effort.
Comment by David Nolen [ 18/Jun/13 7:42 AM ]

I've taken my first pass over the patch, it looks nice!

A couple of concerns, the instructions for the REPL sample now in the repo no longer work. For people who don't want or need to setup another server this seems unfortunate, but it also complicates basic browser REPL testing for compiler devs.

So I think we need two things before moving forward with this:

  • Updated instructions for browser REPL sample
  • Preserve the old behavior (serve the directory) at port 9000 or some user specified port. The new behavior should have to be explicitly set.
Comment by Chas Emerick [ 18/Jun/13 8:51 AM ]

Yeah, definitely happy to refresh all the related docs, etc.

But, part of the point of the work was to eliminate the server/port contention/management that the prior impl implied. That the new repl environment carries around a URL string (@ :entry-url) that can be dropped into a (connect ...) call in your app makes it so you never have to think about ports, session identifiers, etc.

Stepping back:

  • Basic browser-repl testing should be much easier, especially if you use exec-env. Its default is to use phantomjs, but using e.g. Chrome is trivial, as described in at the top of the ticket.
  • That the browser-repl can serve non-REPL-related static content is clever, but I can't imagine that anyone is using it as their only or primary server...or if they are, they should probably stop (in neither browser-repl impl is the HTTP server up to snuff for anything other than servicing browser-REPLs). Implicitly recommending this in a sample app seems not-good, whatever the details of the browser-repl's implementation.

I can update the sample and documentation to match the the prior experience, but surely the sample's current particulars shouldn't wag the dog of the browser-REPL implementation.

Comment by David Nolen [ 18/Jun/13 9:01 AM ]

Updating the sample & documentation is definitely a must. We should touch base with the lein cljsbuild folks to make sure this isn't going to cause massive breakage.

Comment by Chas Emerick [ 18/Jun/13 9:14 AM ]

It's definitely going to cause breakage: the lifecycle of the supporting server is changing, which (positively) affects the semantics of calling repl-env. That breakage should make the browser-REPL more reliable and easier to use by all parties, including tool authors.

Anyway, yeah, any feedback from all corners is always welcome.

Comment by Brandon Bloom [ 08/Jul/13 1:01 PM ]

Doesn't work for me. Here's the output

$ rlwrap ./script/repl
Clojure 1.5.1
user=> (require 'cljs.repl)
nil
user=> (require 'cljs.repl.browser)
nil
(cljs.repl/repl (cljs.repl.browser/exec-env
                  :exec-cmds ["open" "-ga" "/Applications/Google Chrome.app"]))
Browser-REPL ready @ http://localhost:59261/3583/repl/start
"Type: " :cljs/quit " to quit"
ClojureScript:cljs.user> 5
Jul 08, 2013 2:00:42 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: file:/Users/brandon/Projects/clojurescript/lib/goog.jar!/goog/net/xpc/crosspagechannel.js:26: ERROR - required "goog.async.Deferred" namespace never provided
goog.require('goog.async.Deferred');
^

Jul 08, 2013 2:00:42 PM com.google.javascript.jscomp.LoggerErrorManager println
SEVERE: file:/Users/brandon/Projects/clojurescript/lib/goog.jar!/goog/net/xpc/nativemessagingtransport.js:26: ERROR - required "goog.async.Deferred" namespace never provided
goog.require('goog.async.Deferred');
^

Jul 08, 2013 2:00:42 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 2 error(s), 0 warning(s)
ERROR: JSC_MISSING_PROVIDE_ERROR. required "goog.async.Deferred" namespace never provided at file:/Users/brandon/Projects/clojurescript/lib/goog.jar!/goog/net/xpc/crosspagechannel.js line 26 : 0
ERROR: JSC_MISSING_PROVIDE_ERROR. required "goog.async.Deferred" namespace never provided at file:/Users/brandon/Projects/clojurescript/lib/goog.jar!/goog/net/xpc/nativemessagingtransport.js line 26 : 0
Comment by Brandon Bloom [ 08/Jul/13 1:08 PM ]

Got it working via IRC.

Here's the precise code to run:

cd $CLJS_HOME
./script/clean
./script/bootstrap
rlwrap ./script/repl

Then evaluate:

(require 'cljs.repl)
(require 'cljs.repl.browser)
(cljs.repl/repl (cljs.repl.browser/exec-env :exec-cmds ["open" "-ga" "/Applications/Google Chrome.app"]))
Comment by Michał Marczyk [ 24/Jul/13 1:10 PM ]

I've been doing some initial testing on this. I've really not much to say so far beyond "it works". I've tested both the classic bREPL experience with cljs.repl.browser/repl-env and the new exec-env version (the URI to give to the browser when using repl-env without specifying a port is printed out by the Clojure process). The reflection facilities work.

If there's anything in particular I should test to make everybody's lives easier, I'll be happy to do so, just let me know.

For now, I'm planning to spend my next session with this ticket on investigating impact on lein-cljsbuild.

Comment by David Nolen [ 24/Jul/13 1:27 PM ]

This ticket is more or less tabled until we get a patch that doesn't override the current behavior. I already talked to Chas about this on IRC.

Comment by Michał Marczyk [ 24/Jul/13 1:30 PM ]

Good to know, thanks!

Comment by Chas Emerick [ 24/Jul/13 2:27 PM ]

Yeah, just to expound on the status: the current browser-repl will be remaining as-is, and I'll be releasing my alternative "browser-repl" (I need to come up with an actual, non-confusing name) implementation as a separate project so it can evolve separately from ClojureScript proper. I'll certainly be making an announcement in various channels when I've finished the supporting materials (docs, examples, etc).

Comment by Chas Emerick [ 05/Aug/13 8:26 AM ]

The functionality discussed here (and more) is now available as a separate project:

https://github.com/cemerick/austin

I'll close this ticket now, since its been entirely related to Austin's implementation/approach. Enhancements to the existing browser-REPL would be better served by having their own ticket(s).

Generated at Thu Oct 23 08:47:29 CDT 2014 using JIRA 4.4#649-r158309.