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

Refine shadow-cljs doc #3360

Merged
merged 3 commits into from
Jul 2, 2023
Merged

Refine shadow-cljs doc #3360

merged 3 commits into from
Jul 2, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Jun 25, 2023

Refines some bits.

@vemv vemv requested a review from bbatsov June 25, 2023 19:59

=== Tutorials

* https://cestlaz.github.io/post/using-emacs-63-clojurescript/[Using shadow-cljs with CIDER]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing against it per se, but it seems to me that if people needed a tutorial, we'd be failing at creating high-quality documentation.

Most practically, this video is a few years old now and we're not going to watch it each time something is updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me.

@@ -54,15 +54,26 @@ Alternatively you can start the server manually with something like:

[source,sh]
----
$ npx shadow-cljs server
$ shadow-cljs watch app
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this means the command that CIDER uses should probably be updated as well - I recall it used to be the one that's currently in the docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is that npx shadow-cljs server is a fine command for cider-jack-in-cljs since it's universal - it needs not to assume anything (is shadow installed? which build to use?)

But for cider-connect-cljs, given that the user is fully in control of the command, and can observe its output etc, should choose the best possible command.

@@ -32,7 +32,9 @@ On the bright side - this also means that you can host side by side Clojure and
REPLs in a single nREPL connection! This opens up all sorts of interesting possibilities
that we'll discuss later on.

== Differences with the Standard ClojureScript REPL
NOTE: `shadow-cljs`'s REPL is implemented in a very similar fashion, but its mechanism is provided by its own nREPL middleware - not Piggieback's.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes we speak of piggieback as middleware, sometimes we speak of its middleware. It'd nice if the wording was consistent. I'm not sure if it's worth mentioning that the API that shadow exposes is the same as Piggieback's so that clients don't have to do anything different for it. (e.g. it even has session vars named the same was as in piggieback).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's worth mentioning that the API that shadow exposes is the same as Piggieback's so that clients don't have to do anything different for it. (e.g. it even has session vars named the same was as in piggieback).

Perhaps that would invite people to add piggieback. Which might blow up at some point despite the compat effort.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

@@ -17,7 +17,7 @@ and how to setup the most popular ClojureScript REPLs.
== nREPL and ClojureScript

nREPL doesn't natively support ClojureScript evaluation, that why an additional
middleware is needed. CIDER relies on the popular Piggieback middleware for its
middleware is needed. For most REPLs (with the notable exception of `shadow-cljs`), CIDER relies on the popular Piggieback middleware for its
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also the self-hosted cases to consider (where you need a native nREPL implementation in the first place) - e.g. nbb works like this.

@bbatsov bbatsov merged commit dea866c into master Jul 2, 2023
@bbatsov bbatsov deleted the shadow-cljs-doc branch July 2, 2023 06:54
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.

2 participants