Clojurians Log v2

Wait, I think I found it at last, here: (some-> cljs.env/*compiler* deref :shadow.build.cljs-bridge/state :shadow.build/config ,,,)

:shadow.build.cljs-bridge/state is the shadow-cljs build state yes, but be careful when using that

the problem is cache invalidation. it only checks a few places, so just modifying random stuff won't ensure cache is invalidated properly when changed

I recommend sticking to the approach used before, unless you are really sure caching is not a problem

(some-> cljs.env/*compiler* deref :options :replicant/asserts?) :options here is :compiler-options from the build config. you can set that in the target impl as well via :shadow.build/config, like the test impl already did

oh wait, by the time the target process fn is invoked it is already :compiler-options directly in the build state, not from the config. thats already merged at that point.

Yeah, the problem I'm seeing is that those values are very different

(update state :compiler-options assoc :your-stuff 1) in the build target

(some-> cljs.env/*compiler* deref :options :your-stuff) in the macro

AHA!

😄

Thanks! 🙏

don't put too much into compiler-options though. it is persisted to every cache file for cache invalidation purposes. so if you put something in there that changes on every run (e.g. current time) it'll cause the cache to never be used

Yeah, I just want to make it possible to put some light config in the build target and pass it to the bundled runner.

if you mark the runner to be always compiled via :dev/always true metadata on ns, then you don't have to worry about caching. so you could just use the state directly. (assoc state :your-stuff 1) in the target, (some-> cljs.env/*compiler* deref :shadow.build.cljs-brige/state :your-stuff) in the macro

only concern here really is the caching, but otherwise its fine to access whatever that way

Am I right to conclude that (assoc-in state [:compiler-options :whatever] ,,,) only works in modify-config, not in the resolve hook?

Well cool, this now works. That means most basic setups no longer need a "runner" namespace at all. Will make for a very cool getting started experience.

:compiler-options {:infer-externs true} is the default. you never need to set that

:output-dir only being set in :dev means a :release build will fail

just commenting on the rest of the build config, portfolio part looks good 🙂

I had a hunch that this was outdated 😅

Thanks for the pointers

well :infer-externs :auto is the default, but thats what you should be using. true means manual opt-in, so only warns with (set! *warn-on-infer* true) which nobody does and then wonders why they have extern issues 😉

Hehe, yeah. I noticed a lot of new inference warnings when removing it

(set! *warn-on-infer* false) works if you are sure you don't need any, for a given ns or section of code. but otherwise you should be hinting to ensure :advanced works

I was actually missing quite a few ^js hints in the Portfolio code, so that was a good catch.

if its a test build you could use the :ns-regexp it provides to find all namespaces given a certain pattern? like the default "-test$"? maybe encourage a certain naming pattern, so they are easy to find?

otherwise yeah dynamic loading stuff doesn't work, since a compiler needs to be present to actually compile them first

self-hosted can do that if you want to go with the overkill route

but if -scenes is already the encouraged pattern, then the test targets might work

:ns-regexp sounds like just what I'm looking for 👍

you can also create a :runner-ns that can setup and run portfolio. :ns-regexp ensures that all the matching namespaces are compiled before :runner-ns is compiled. so in a macro it can gather all the metadata it needs from those potentially

:portfolio
  {:target :browser
   :modules {:main {:entries [matnyttig.scenes]
                    :ns-regexp "-scenes$"}}
   :dev {:output-dir "dev-resources/public/portfolio-js"}
   :compiler-options {:externs ["datascript/externs.js"]}}

:ns-regexp only exists for the test targets. so :browser-test :node-test

Ah, right

doesn't portfolio already have a server side?

No, it's pure cljs currently

Can I convert this build to a browser-test build even if it doesn't run tests?

the test part only means that the default :runner-ns is setup to run cljs.test tests. if you swap that it can do whatever. the compiler doesn't even know anything about tests

I could also walk you through creating :target :portfolio if you want. like :browser-test is only a very thin wrapper over :browser https://github.com/thheller/shadow-cljs/blob/ea54e4b943c762645178303778aef094387f1a85/src/main/shadow/build/targets/browser_test.clj

That would be amazing 🤩

just a .clj file on the classpath you could ship with portfolio as a shortcut

How does shadow find this?

:target :portfolio looks for shadow.build.targets.portfolio/process function

Aha, so Portfolio can ship that namespace

yes. can also use symbols do :target portfolio.build would look for portfolio.process/process

https://github.com/thheller/shadow-cljs/blob/ea54e4b943c762645178303778aef094387f1a85/src/main/shadow/build/targets/browser_test.clj#L84-L94

this fn then gets called for all the "stages" a compile goes through. and this impl basically just fills the config so that it looks like a regular :browser build

essentially it fills :modules {:test {:entries [...all-discovered-tests-here...]}}

(tu/find-test-namespaces state config) does the actual finding based on :ns-regexp

This sounds perfect

I will try this route, thanks a lot 🙏

https://github.com/thheller/shadow-cljs/blob/ea54e4b943c762645178303778aef094387f1a85/src/main/shadow/build/targets/browser_test.clj#L81

that line is kinda important and makes it work

but I guess you can just copy browser-test and fill it with defaults for portfolio

Yeah, I'll try that. May return with questions 😄

there are a few subtle issues in the runner part, but checkout https://github.com/thheller/shadow-cljs/blob/ea54e4b943c762645178303778aef094387f1a85/src/main/shadow/test/browser.cljs

{:dev/always true} is the important bit in ns basically. otherwise it never gets recompiled when scenes changes and macros can never pick up new metadata. if that is needed at all

recompilation on scene change is essential at least!

that happens like normal anyway

but for the default test runner there is (env/get-test-data), which is actually a macro that discovers all the tests to know what to run. since that is a macro it wouldn't know about "changes" unless it also gets recompiled when tests change

so if you have something that needs to run when scenes are recompiled thats the place to do it. you might not need that to begin with if scenes already register themselves somewhere

deftest doesn't register anywhere, so the macro is needed

Ah, I see. I don't think so. In Portfolio you define scenes with defscene which is a side-effecting macro that updates an atom that causes Portfolio to re-render.

yeah then you don't need that

probably fine to have :runner-ns that just does

(require '[portfolio.ui :as ui])

(ui/start!)

Ah, as a default. That would be nice.

Is there a specific reason why you use test-dir instead of :dev {:output-dir "..."}?

Ah, I see you use it for both the HTML file and the js files, so not strictly the same

yeah legacy reasons mostly

I've been meaning to clean up all the test targets mess, but never got around to it. the cljs-test-display lib is kinda outdated and has a lot of issues

I see

Would you not recommend copying this bit? I don't think I should be putting the index.html file in (:output-dir (:dev config))

At least that's not how our project is set up currently.

yeah, basically this is all just a :browser build still. just with some parts automated, so automate what makes sense for portfolio

I expect most people will want to fill in the runner etc for themselves, but having some defaults will make for a very nice onboarding solution. And certainly some things could just as well be configured in shadow.cljs.edn instead, so this will be a great addition to the library.

[:portfolio] Build failure:
failed to merge config value
{:a #object[java.io.File 0x3a291307 "dev-resources/public/portfolio-js"], :b #object[java.io.File 0x5b09924d "dev-resources/public"]}

that error message could be improved I guess 😉 its from config-merge which tries to merge build configurations but doesn't know how to merge file instances since those can't come from build configs normally

so do you set :output-dir somewhere to a file instance?

Seems to come from :public-dir in my config

Removing it from shadow-cljs.edn made the error go away

:output-dir is sort of a special option, so can only exist once

And setting :public-dir creates it?

shadow-cljs doesn't know what :public-dir is and never uses it no

unless you do something with it in the test target?

I tried to use it to generate the output dir, heh

browser-test also does that from test-dir? https://github.com/thheller/shadow-cljs/blob/ea54e4b943c762645178303778aef094387f1a85/src/main/shadow/build/targets/browser_test.clj#L40

just make sure its a string at that point

Yeah, I basically just changed your code slightly

[:portfolio] Build failure:
no common dependency found for src
{:src [:shadow.build.classpath/resource "goog/base.js"], :deps #{:portfolio :test}}
ExceptionInfo: no common dependency found for src

Nevermind, that's me

I hadn't changed all occurrences of :test so it was working on two modules

yeah the way this is all constructed in the code is a bit weird. dunno why I wrote it that way. should have probably just overwritten or thrown if user build config has :modules

Don't be so hard on yourself 😊 It was my mistake, the code was just partially updated.

Ooh, something is working

OMG, it's working now 😄

Great, I have everything working as I want now. Thanks a lot for your help! I'll write some docs and get this packaged up later today 💃

@thheller I've previously accessed the config via a macro. For instance, Replicant supports doing this:

:compiler-options
{:main "myapp.dev"
 :optimizations :none
 ,,,
 :replicant/asserts? false} ;; <==

still a macro, same way as always

Yeah, the macro wasn't the problem. The question is rather, what do I assoc on the state in the build target code and where do find it in the macro later?