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

Improved interplay with nREPL #561

Closed
bbatsov opened this issue Sep 15, 2019 · 4 comments
Closed

Improved interplay with nREPL #561

bbatsov opened this issue Sep 15, 2019 · 4 comments

Comments

@bbatsov
Copy link

bbatsov commented Sep 15, 2019

This issue is mostly a reminder to continue the conversation we've started a couple of weeks ago about the state of ClojureScript support in nREPL.

I completely agree that Piggieback is probably not the way to go, and obviously there's the historical problem of ClojureScript not being around when nREPL was created and not being mature when Piggieback was created. I do think, however, we can improve the current situation.

The way I see it we can do one of the following:

  • Try to modernize piggieback, although this might be problematic as it has to work with ClojureScript REPLs with pretty different capabilities. We already dropped support for rhino a while ago, as it was really messy to support it and we might limit the focus only to the essential REPLs going forward. Unfortunately here we'll also be limited by the underlying concept of having the ClojureScript be transparent and re-using the standard eval and load-file ops.
  • Build something cleaner based on shadow's existing middleware. It can potentially introduce different ClojureScript specific ops.
  • Tweak nREPL to support better ClojureScript natively - e.g. we can just add something like runtime to all sessions. This will be a minimal change to nREPL. I haven't thought much how the delegation to other runtimes should look, though - probably different session types can just look for ops named op-runtime or whatever and fallback to the Clojure ops if there's not runtime-specific implementation of an op.

Generally it seems to that the approach of just piggiebacking on the eval and load-file was a bad idea that really limitted what we can do with ClojureScript and we should move in a direction where we clearly separate Clojure from ClojureScript ops. There's also the option to implement nREPL natively in ClojureScript, of course, but this would kill the nice feature we've got today allowing us to host different types of REPLs on the same nREPL server.

My main issue is that my knowledge of ClojureScript is basic and I definitely need some expert like @thheller to help with this, but one thing is certain - I'd love to start working in a more focused manner towards resolving issues like:

@thheller
Copy link
Owner

I'd be more than happy to help with this but unfortunately most the problems come from cljs.repl not providing enough hooks to get to the underlying implementation details. There is no concept of a "runtime" or "session" exposed or accessible anywhere.

We can however get rid of some of the nasty piggieback hacks and make that easier to work with.

I'm working through some REPL issues right now and I want to get rid of the piggieback emulation badly anyways. I'll see if I can write something generic that supports the default CLJS REPLs as well.

@theophilusx
Copy link

I would also be happy to try and contribute here. I know enough clojure to be dangerous and have a reasonable amount of JS and node experience. Looking for something to ramp up my clojure skills and ahve a little apre time until the end of the year. Let me know if I can help in some way.

@thheller
Copy link
Owner

thheller commented Jun 4, 2020

I don't really know what I can do on my end.

I will write more shadow.remote docs soon which may provide some guidance on how I think this should all work to be agnostic about CLJ or CLJS. Maybe that can inform a some nREPL design for CLJS. The piggieback approach is fundamentally flawed and cannot be fixed. The trade-offs are going to remain. IMHO, YMMV.

Closing this since its not really actionable and there are already too many started-but-never-finished discussions about this elsewhere.

@thheller thheller closed this as completed Jun 4, 2020
@bbatsov
Copy link
Author

bbatsov commented Jun 5, 2020

Fair enough. In theory another approach down the road might be to use a native ClojureScript nREPL implementation like https://github.com/djblue/nrepl-cljs, but I guess we'll always have to support Piggieback regardless.

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

No branches or pull requests

3 participants