Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Figwheel + Devtools #309

Closed
wants to merge 8 commits into from
Closed

Figwheel + Devtools #309

wants to merge 8 commits into from

Conversation

darwin
Copy link
Contributor

@darwin darwin commented Dec 9, 2015

I want to be able to type REPL commands in devtools and let them compile & execute by figwheel.

This is a motivational screenshot from proof-of-concept code:
Works like a magic

Devtools could have used nREPL interface, but that was too hard. There is currently no clojurescript implementation of nREPL client and whole nREPL landscape seems to be a mess.

Instead I decided to talk directly to Figwheel and use its already existing connection.

This pull request is not meant to be merged. I just want to open discussion what is the best approach to implement something like this.

Open questions:

  1. Does figwheel want to provide such service to cljs-devtools in the first place?
  2. Should that functionality by automatic or configure via a plugin
  3. What should be the story for multiple repl sessions? Current implementation simply pastes whatever comes over wire into current REPL session (which is ok solution for now IMO). An alternative solution would be to deliver REPL commands only if REPL session matches build-id of issuing client. I didn't see any infrastructure in figwheel to decide on incoming websocket messages depending from which build-id they are coming.

Note: echoing REPL results back to devtools is not directly related to this PR, it is done via
https://github.com/binaryage/cljs-devtools/wiki/Figwheel-REPL-plugin

darwin added a commit to binaryage/cljs-devtools-sample that referenced this pull request Dec 10, 2015
@bhauman
Copy link
Owner

bhauman commented Dec 10, 2015

@darwin this looks really cool! Will have to take a look!

@arichiardi
Copy link
Contributor

👍 this is very cool indeed. I have always wanted to have less window switching (when I am not able to use two monitors of course) and sometimes you really want to just tune in a detail in the browser.

@darwin
Copy link
Contributor Author

darwin commented Dec 10, 2015

I got the basic functionality working. It works pretty nicely.

But I realised that special-fns and compiler warnings are not presented on client-side (and in devtools). I have a solution in progress, but it complicated the implementation quite a bit. And I have to keep some global state around. Capturing some print calls, but not some others, is not really something which would be expected in cljs.repl design.

@arichiardi
Copy link
Contributor

It's great indeed! Basically at some point I thought about including a client side repl in figwheel using replumb (were all the eval quirks should be addressed). But if devtools works fine there will be no need. We will already have everything in one window.

@darwin
Copy link
Contributor Author

darwin commented Dec 10, 2015

Another thing I'm thinking about is that devtools could connect directly to figwheel server. So HUD could work even if you unintentionally break your client-side app. Basically your figwheel HUD can work independently of your app. This can be helpful not only in situations when your app is intermittently broken but also in cases where there is "no UI" where to present figwheel HUD. Imagine web-workers or some background extension pages when developing chrome extensions.

But this will be maybe some future step and is unrelated to this PR.

@darwin darwin changed the title Figwheel + devtools Figwheel + Devtools Dec 10, 2015
@darwin
Copy link
Contributor Author

darwin commented Dec 11, 2015

Just sharing my progress. Today I had bad time fighting Clojure.

As I wrote above I need to present the output as it appears in terminal. Unfortunately not all REPL ouput goes through :print function specified in REPL opts. For example compiler warnings are printed directly. And even some special-fns in figwheel don't use repl-println consistently. For example calling pprint in print-config uses standard out directly.

I needed to hijack stdout and strderr to call my functions and then do their normal job. It turned out this is more Java thing than Clojure thing and one has to know something about interop and how Java works (I'm a noob).

The current solution is in sniffer namespace. I'm pretty happy with it. It will guarantee that I will capture everything as it appears in the terminal no matter what. The only drawback is that some output should be skipped because I don't want to see it in devtools console. But this will be solvable on case-by-case basis.

Another problem was serialization of messages via prn-str and read-string, it didn't work for some messages where I was shoveling javascript pieces around. For some reason prn-str decided not to put double quotes around some keys in the map. I'm no sure why. I switched to transit which works without problems for me. This is up for discussion if we want to bring transit dependency on client-side or we will be able to solve that pesky prn-str issue.

Another pending task is REPL prompt. I want to present it in the devtools, but in some smart way. I don't want to print it again after each executed command. Will probably introduce some message for REPL prompt updating.

@darwin darwin force-pushed the devtools branch 2 times, most recently from ae350e6 to f2d803b Compare December 14, 2015 17:57
The motivation here is to allow REPL functionality on client side.
darwin added a commit to binaryage/cljs-devtools that referenced this pull request Dec 14, 2015
@darwin
Copy link
Contributor Author

darwin commented Dec 14, 2015

I have squashed my commits and cleaned up new code so it does not touch much of existing codebase. Most new code was added into separate files.

No major changes were needed to current figwheel implementation.

On server side:

  • wrapping start-cljs-repl call with repl-driver (there should be one driver per REPL instance)
  • repl-driver is responsible for:
    • multiplexing REPL input (coming from user or from network)
    • recording printing to stdout/stderr (this was the hardest part and source of complexity in driver.clj)
    • notifying client-side about current namespace changes, start/end of evaluation jobs, etc.
  • also I had to replace prn-str/read-string with transit to get reliable serialization

On client side:

  • new functionality is implemented as repl-driver plugin

cljs-devtools usage of figwheel-driver API can be seen here:
binaryage/cljs-devtools@198a443

I would be grateful for code-review and potential merging. I can also work on documentation if required.

@bhauman
Copy link
Owner

bhauman commented Dec 15, 2015

Hey the holidays has me running around quite a bit. Rest assured, I'm going to look at this soon.

@darwin
Copy link
Contributor Author

darwin commented Dec 15, 2015

No worries, this does not have high priority. I will be using the fork.

btw. today I was able to embed "Dirac" into a Chrome extension:
https://dl.dropboxusercontent.com/u/559047/dirac-embedded-as-chrome-extension.png

That means people should have very similar experience as with normal DevTools (they just install a Chrome extension, which includes our DevTools fork).

@darwin
Copy link
Contributor Author

darwin commented Jan 1, 2016

I'm still working on this. Dirac is in pretty good shape now. Also for initial release I wanted to implement an ability to eval REPL commands in the context of paused javascript debugger.

Unfortunately this exposed a flaw in my initial design. Figwheel client runs inside javascript context which is paused when stopped on a breakpoint. That means that Figwheel client functionality does not run when figwheel client receives websocket messages. This forced me to implement a secondary communication channel directly between Figwheel and Dirac. This way I can ask Figwheel server to do CLJS compilation for commands entered while javascript execution was paused. Please note that I can execute javascript code snippets even while stopped in debugger. DevTools uses chrome debugger protocol to do that (it is needed for javascript commands entered into DevTools javascript console).

Unfortunately this forced me to do more invasive changes in Figwheel code. The good news is that this work will be useful for a second phase when we could alternatively implement live reloading and HUD functionality via Dirac DevTools. This way people can benefit from Figwheel feedback even while javascript context is paused.

@bhauman
Copy link
Owner

bhauman commented Jan 1, 2016

I'm really glad you are exploring this. But I think that you should probably start viewing this as a seperate project that obtains the compile-env from Figwheel. Things are diverging a bit too much.

I just don't want you expecting that this is simply going to get merged.

It's probably better to explore this space as a separate project so you can figure out what it is that is being created. In the end you may just need an endpoint from figwheel to simply eval cljs code.

And then we can figure out the very best way to do this.

@darwin
Copy link
Contributor Author

darwin commented Jan 1, 2016

Yep, you are right. I started optimistically expecting only few hooks to be added. But then I encountered that whole REPL recording issue and now paused javascript execution problem. It has complicated things dramatically. Also my attempts not to modify Figwheel too much are hampering my progress at this point.

I'm going to work on a proof-of-concept thing in my fork.

@darwin darwin closed this Jan 1, 2016
darwin added a commit to darwin/lein-figwheel that referenced this pull request Jan 1, 2016
at this point the fork diverged to much from Figwheel and will live on as an experimental project

bhauman#309
darwin added a commit to binaryage/dirac that referenced this pull request Jan 1, 2016
unfortunately whole client-server design will have to change

I will bring Figwheel client into DevTools, responsive code cannot run inside app's javascript context

see bhauman/lein-figwheel#309 (comment)
@bhauman
Copy link
Owner

bhauman commented Jan 2, 2016

Hey I really think you can use evaluate-formhttps://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/repl.cljc#L431
with an existing populated compiler-env and it should be able to give you everything you need. This could simplify redirecting input and capturing output.

@darwin
Copy link
Contributor Author

darwin commented Jan 2, 2016

Thanks for the tips. I will look into that.

Yesterday I revisited the idea of implementing it on top of nREPL. I studied a bit more about whole nREPL stuff and it looks promising. I have piggieback+weasel working and now trying to modify them to fit my needs.

This way no changes in Figwheel will be needed, all Dirac REPL supporting code will be implemented as nREPL middleware and some support code in cljs-devtools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants