Fork me on GitHub
#shadow-cljs
<
2019-10-06
>
dabrazhe07:10:02

Hi guys, I'd like to ask your view on my cljs setup and possible improvements 1 Using Cursive on ItelliJ idea 2 shadow-cljs edn has :build config (app in this case) , and dependencies listed in the :dependencies map and the nrepl port 3 other JS dependencies are added with yarn add xxxlib and a record is added to package json 4 to add/remove dependencies I have to restart the shadow-cljs server 5 to connect to repl in Cursive I created a Clojure configuration in IntelliJ with the specified nrepl port 6 once the server is started I launch the configuration and then start a repl command from the context menu to start cljs node repl I do find it a rather verbose process, especially when I need to add new dependencies, to repeat the steps 3-6 every time. Is there a simpler way?

Space Guy08:10:00

You shouldn't have to restart like that on adding new JS yarn add dependencies. The guide does still recommend a restart on adding new CLJ/CLJS :dependencies, though

Space Guy08:10:51

There are tools to add CLJ libraries at runtime (pomegranate, tools.deps 'add-lib') but I haven't found them very reliable for CLJS libraries ("x.cljs is classpath as a resource, but shadow/figwheel can't see it")

p-himik09:10:31

Some thoughts: 2. I don't think Cursive is able to read shadow-cljs.edn, so you may want to use either :deps true (my preference) or :lein true. 4-5-6. Personally, I rarely use CLJS repl - with hot reload, I find it simple enough to just make a change and see how it works. With that being said, I embed shadow-cljs so that when I start my app in dev environment, shadow-cljs is already running.

thheller09:10:58

@dennisa that is pretty much what you have to do. restarting when :dependencies change is the only way to ensure everything works reliably. I have been experimenting with adding them dynamically but it hasn't been worth the hassle to be honest

thheller09:10:24

you don't need to restart when JS deps change though

👍 1
dazld17:10:29

hey @thheller - thank you for adding that log-style option, to fix the unreadable text in dark mode. I’m not sure how to use it though - env var?

dazld17:10:27

hah, not in 2.8.59

dazld17:10:49

i guess once it’s there it’ll be something like (goog-define shadow.cljs.devtools.client.env/log-style "color: white")?

dazld17:10:56

I did a dirty thing while waiting:

(set! shadow.cljs.devtools.client.browser/devtools-msg
        (fn devtools-msg [msg & args]
          (js/console.log.apply
            js/console
            (into-array
              (into [(str "%cshadow-cljs: " msg) "color: #5981D8;"]
                    args)))))
🙂

thheller18:10:43

@dazld oh sorry. though I mentioned it in the ticket. you set :devtools {:log-style "color: green;"} in your build config

thheller19:10:29

oh actually that isn't released yet

thheller19:10:19

@dazld I just released 2.8.60 where :devtools {:log-style "color: green;"} should work

👍 1
dabrazhe21:10:17

Thank you all for responses! @p-himik how do you embed it? Because that what I’d like to achieve: having the whole chain start automatically in shadow cljs node repl.

p-himik05:10:52

I don't think it makes sense to go into detail on how I embed it simply because: 1. I use https://github.com/juxt/kick.alpha 2. I don't usually use CLJS REPL However, you still can embed it yourself. Despite thheller saying that it was not created with embedding in mind, doing so was quite easy in my case. The main thing is to call shadow.cljs.devtools.server/start! and shadow.cljs.devtools.api/watch with the required arguments.

p-himik05:10:19

The source code of shadow-cljs is easy to read in understand for the most part. Well, at least when we talk about all of the plumbing and not actual compilation, and plumbing is exactly what you need.

p-himik05:10:16

But given that you want it all to run within Cursive REPL, you may end up having to understand what's going on on a deeper level and write quite a bit of code. Cursive REPL has some limitations: https://github.com/cursive-ide/cursive/issues/835 https://github.com/cursive-ide/cursive/issues/2215

thheller10:10:28

@p-himik shadow-cljs was absolutely built with embedding in mind. it just works better if you let it take full control instead of trying to fit it into another system

p-himik10:10:58

@thheller Well, that were your words about some of its components not being made for embedding. 🙂 After all, load-cljs-edn and its callers are still used in many places.

p-himik10:10:11

Also, what does it mean to "work better" in this context? The output is the same (at least, it works the same way), the UX is better in my case, the integration with the rest of the build system is better.

thheller10:10:12

well, yes it is not as customizable as some people would like. that was never the goal though.

thheller10:10:25

works better as in takes care of the most common dependency conflicts so you don't have to

thheller10:10:28

server-mode

thheller10:10:46

once embedded and running it works exactly the same yes

p-himik10:10:11

Hmm, I have never had to deal with any dependency conflicts. And when I had to, standalone shadow-cljs was giving me just the same errors. But maybe that's because I embed the whole shadow-cljs server process, I don't know.

thheller10:10:55

you didn't. you also know how to fix them if you have them. beginners don't.

p-himik10:10:32

Yeah, makes sense.

thheller10:10:34

but just to make this clear: embeddable operation is absolutely a goal of the entire thing

bmo 1
thheller10:10:50

I would consider it a must-have feature

thheller10:10:45

just overriding certain behavior like where the config comes from is not really a goal or something I'd want to support still

p-himik10:10:14

Where does the aversion come from? After all, there are definitely use cases where someone would want to change some of the behavior. E.g. write some custom config loader to support custom reader macros, like in juxt/aero.

thheller10:10:48

having to support it outside of code

thheller10:10:17

it will leak into some template that some people will use. then people come asking questions since all of the docs don't apply to their setup.

thheller10:10:37

and I can't answer any of them because they use a setup I don't know

thheller10:10:40

the entire thing is already complex enough, don't need to make it more complex for little gain. you can already do everything you could possible want via the CLJ API if you really need to

p-himik10:10:11

> you can already do everything you could possible want via the CLJ API Hmm. Can I change :js-options via CLI options? Because I only want some of them enabled iff I have to debug the compiled JS and not for the regular development. As an alternative - is it possible to inherit builds, in a way? So that I can have two builds with exactly the same config except for the :js-options value.

thheller10:10:52

I said CLJ not CLI, but yes you can change :js-options from the command line

thheller10:10:10

shadow-cljs release app --config-merge '{:js-options {:whatever true}}'

thheller10:10:02

same works for (shadow/release :app {:config-merge [{:js-options {:whatever true}]}) but note the extra vector

thheller10:10:47

but it is only an additive merge currently, so you can't simply dissoc values

thheller10:10:04

happy to add a (shadow/release :app {:config-rewrite some-fn}) if that would make sense

thheller10:10:18

mostly didn't because no idea how to do that from the CLI

thheller10:10:14

there is also shadow-cljs release app --debug or --pseudo-names meant to aid debugging

thheller10:10:30

if that should set some :js-options it isn't currently setting I'm happy to change that

thheller10:10:55

ie. it currently only sets :compiler-options {:pretty-print true :pseudo-names true} for --pseudo-names

p-himik10:10:22

Yeah, I know - I meant CLI of whatever I embed shadow-cljs into. Should've made it more precise. Nice, thanks! To be honest, :config-merge already looks a bit sketchy to me, let alone :config-rewrite. 🙂 At this point, I cannot really say if that will be more useful for the tool in general. As you have probably guessed by now, my personal choice would still be to allow heavy customization and adopt the caveat emptor principle. It should scale well and if something leaks and becomes popular enough and remain broken enough to cause a lot of angry noises, it may just be useful enough to actually include it in the main tool. Or create a separate tool/plugin with a separate maintainer. But that's all just thinking out loud, don't take it to heart.

thheller10:10:04

if you provide a clear description/example of what exactly you want to do I can think about it

thheller10:10:25

you can already do builds that aren't defined in the config file at all

thheller10:10:09

"allow heavy customization" it isn't clear what you mean but if you clarify I'm happy to see what can be done

p-himik11:10:00

A few things that I think are missing or suspicious (given my past experience and what I could quickly find in the code): 1. Some of the resolvers and mutations in shadow.cljs.devtools.graph.builds use shadow-cljs.edn and it's impossible to override this behavior. But I have no clue what the resolvers and mutations do (the graph functionality seems to be used only in tests and the dev webserver), so maybe it's completely justified. 2. NREPL :init-ns cannot be changed. But that's not really an issue since, I think, it's possible to write and use a custom middleware that does it. 3. Impossible to embed build report generation because it reads the EDN config. Right now I have to generate a shadow-cljs.edn file specifically for this feature every time I need. 4. The code from compile, release, check, make-runtime, with-runtime could be reused if it accepted an optional config map. Right now I have a copy of these functions in my own code that just adds a single config argument. That's pretty much all I could find and/or had experience with. As I mentioned before, I don't use shadow's CLJ[S] REPL at all, so maybe there are some things there as well. Given these, and all potential cases where the EDN file could be assumed to exist, "allowing heavy customization" probably means something like "make the config customizable with with-bindings". BTW it seems there's a support for plugins but it's undocumented. Is it something internal?

thheller12:10:38

@p-himik 1. the graph stuff is internal and not part of the public API (yet), it is mostly for the UI or just experiments. 2. :init-ns can be changed via :nrepl {:init-ns the.ns}? 3. happy to add a function that takes a build config and creates a build report for it 4. compile,release,check are functions that are meant to be used directly from the REPL. compile*, release* take a config directly and don't touch shadow-cljs.edn

thheller12:10:21

plugins are undocumented, sort of experimental until someone actually has a use case for them (I failed to come up with something useful)

p-himik12:10:11

2. Yes, but it can be provided only within the file:

(let [config
      (config/load-cljs-edn)

      init-ns
      (or (get-in config [:nrepl :init-ns])
          (get-in config [:repl :init-ns])
          'shadow.user)]
  ...)
3. Awesome! 4. Indeed, and that's exactly what I'm doing. However, order to make sure that nothing breaks, I still have to do exactly what release and similar methods do under the hood: call rt/init, rt/start-all, runtime/set-instance!, runtime/reset-instance!, rt/stop-all in the correct way. And preserving the logging is also nice. It's about 40 lines in total if I preserve the original code as much as possible.

thheller13:10:17

you are most definitely touching things you shouldn't be touching if you call rt/start-all

thheller13:10:31

if you properly describe what you want to do I can add functions to do exactly that

thheller13:10:55

but if you are going into the internals that aren't part of any public API then you are on your own

thheller13:10:45

the stuff is designed exactly for the internal use after all. it was never meant to be used from the outside

p-himik13:10:27

So ideally, I should only have to use stuff from the api namespaces, right? OK, let's see. This is the code for the api/release function:

(defn release
  ([build]
   (release build {}))
  ([build opts]
   (try
     (with-runtime
       (let [build-config (config/get-build! build)]
         (release* build-config opts)))
     :done
     (catch Exception e
       (e/user-friendly-error e)))))
So, I can provide my custom config to the release* function, that's good. But release also calls with-runtime which in turn calls make-runtime, so let's look at its code:
(defn make-runtime []
  (let [config (-> (config/load-cljs-edn!)
                   ;; just in case someone gets the idea to put :server-runtime true into their config
                   (dissoc :server-runtime))]

    (log/debug ::runtime-start)

    (-> {::started (System/currentTimeMillis)
         :config config}
        (rt/init (common/get-system-config config))
        (rt/start-all)
        )))
Notice that cheeky (config/load-cljs-edn!) that spoils the fun.

thheller13:10:43

happy to change that if you create a github issue describing how exactly you want to call things

p-himik13:10:26

Will do, thanks!

thheller13:10:39

the runtime isn't something you should ever need to mess with but I can make it more explicit so that it can be passed in instead using bindings

p-himik13:10:24

Well, and I really don't want to. 🙂 I just want to alter the config. The main issue is that I cannot get those modifications to the runtime.

thheller13:10:00

its just written this way because I didn't want users at the REPL having to worry about managing and passing around a runtime

p-himik13:10:17

Yeah, sure, I get the motivation. And adding optional arguments should not change that. BTW, you don't really want to introduce config bindings and instead prefer to use explicit arguments, right?

thheller13:10:30

so far as I understand it your problem is the "runtime" handling

thheller13:10:56

meaning that if none is running one is initialized with a hardcoded load-cljs-edn!

thheller13:10:50

and I hate bindings yes, want to avoid them at all cost whenever I can

p-himik13:10:36

Yes! And runtime initialization with a custom config is not in the API. Can you give a bit of context on why you hate them? I've never used them myself. Are they hard to debug with/reason about/something else?

thheller13:10:13

you have always been able to initialize with a custom config

thheller13:10:38

(shadow.cljs.devtools.server/start! the-config-map)

thheller13:10:52

the issue is that you don't really need the full "server" runtime

thheller13:10:04

as that starts too much stuff you don't need if you just want a release build

thheller13:10:24

so in effect there is a missing "give me a minimal runtime"

thheller13:10:38

which is what make-runtime does

thheller13:10:07

but since I never though someone would want to customize that I never exposed it

p-himik13:10:38

Ah, right. Everything is just the way you describe, yes.

thheller13:10:17

not sure whyI hate bindings so much. something about "hidden" function arguments just bothers me

thheller13:10:32

if a function needs something it should be passed in

p-himik13:10:31

I think it's just a question of whether this lack of referential transparency worth saving the effort to pass the same arguments over and over and over again, especially if 80% of the use cases are happy using the default values. At least, that's how it looks like in the clojure.core code that has quite a few dynamic vars. And I always just pass some sort of a context around instead. But I only develop applications - I've never written anything that other developers would use.

thheller13:10:21

"runtime" is meant to capture "shared" services. so things that can be shared between builds. for a singular release build that obviously isn't very useful and a hinderence in the API

thheller13:10:28

it is however crucial for server-mode

thheller13:10:55

and I'll happily pass around one extra arg all day if saves me from using a binding

p-himik13:10:58

Yeah, I get that.

dabrazhe21:10:20

@thheller dynamic restart on :dependencies change is not really an issue, it doesn’t happen too often, and most are ok with restarting lein or boot. It’s the whole verboseness that bothers me, I’d like to create one command to achieve editor integration with repl, if possible.

thheller10:10:52

can't do much about the cursive side of things

thheller10:10:37

nrepl is also a bit of a hurdle

thheller10:10:02

you can probably just create a REPL command in cursive to do it all for you