Fork me on GitHub
#sci
<
2023-09-29
>
Jakub Holý (HolyJak)10:09:53

Update: It works with shadow :optimizations none but not w/ :simple Update2: The problem is with my rewrite of defsc macro; if I change Root to Root2 at both occurrences then I get to see the updates. Update3: FIXED - I had to change a defonce inside the defsc sci macro to def and now I can redefine component classes even in release mode @borkdude & @tony.kay I need your awesome brains to figure out why https://blog.jakubholy.net/2023/interactive-code-snippets-fulcro/#_demo does not modify the DOM. I see that the script is stateful, but that shouldn’t be a problem? I know the script runs fine (I can add 1, 2 , .. to the bottom to see it change) but when I change the text inside the Root fulcro component (which becomes a JS function), the output (mounted below the editor) does not change. In a normal Fulcro app, the code works fine. I thought this did work before for me, but I could be mis-remembering… When I run the https://github.com/holyjak/sci.configs/blob/add-fulcro/README.md#development in sci.configs, the following works, which IMO is essentially the same as the in-blog editor being modified and re-evaluated: 🧵

1
Jakub Holý (HolyJak)11:09:30

sci.configs/development.cljs - eval first the one, so the second:

(comment
  (sci/eval-string* full-ctx "
  (do
    (ns test (:require 
      [com.fulcrologic.fulcro.dom :as dom]
      [com.fulcrologic.fulcro.application :as app]
      [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
    (defonce app2 (do (println \"Def app!\") (app/fulcro-app)))
    (comp/defsc Root [_ _] (dom/div (dom/h3 \"One\")))
    (app/mount! app2 Root \"app\"))
  ")

  (sci/eval-string* full-ctx "
  (do
    (ns test (:require 
      [com.fulcrologic.fulcro.dom :as dom]
      [com.fulcrologic.fulcro.application :as app]
      [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
    (defonce app2 (do (println \"Def app!\") (app/fulcro-app)))
    (comp/defsc Root [_ _] (dom/div (dom/h3 \"TWO\")))
    (app/mount! app2 Root \"app\"))
  ")
  ,)

Jakub Holý (HolyJak)11:09:33

So the question is, what is the difference, and how to troubleshoot this? 🙏

Jakub Holý (HolyJak)11:09:43

Hm, I have a similar dev playground in my blog, and there it works fine as well, changing stuff in the editor and seeing the dom updated. So either it is something with release build of the editor or something else…

borkdude11:09:58

could you first figure out if it's a release (advanced) issue?

Jakub Holý (HolyJak)11:09:23

Yes and no. If I switch from :advanced to :optimizations :simple , I still have the problem. But with no optimizations, i.e. running shadow-cljs watch, it works.

borkdude11:09:44

so it's none vs simple/advanced? interesting

Jakub Holý (HolyJak)11:09:38

Most likely. Trying :whitespace now…

borkdude11:09:48

you can try to debug the build with --debug and insert lots of printlns etc. without knowing the code/fulcro, it's hard for me to say

👍 1
Jakub Holý (HolyJak)11:09:41

I will look into that, thank you. BTW, how can I make it possible to use js/console.log , js/document etc from SCI?

borkdude11:09:22

{:classes {'js js/globalThis :allow :all}}

🙏 1
Jakub Holý (HolyJak)11:09:13

Hm, :whitespace result fails to load with TypeError: goog is undefined 👀

borkdude11:09:10

interesting

borkdude11:09:33

perhaps a macro is expanding into something with goog?

borkdude11:09:59

normally the goog namespace exists under optimizations none

borkdude11:09:09

but this gets compiled away under simple and more

Jakub Holý (HolyJak)11:09:38

I’ll ask in #C6N245JGG,thx!

borkdude11:09:04

one of your macros I mean

borkdude11:09:12

it's likely not a shadow issue

borkdude11:09:12

hmm, not seeing any goog references here:

borkdude@m1 ~/dev/sci.configs (holyjak-add-fulcro) $ rg "goog"
src/sci/configs/cljs/test.cljs
102:   See 
borkdude@m1 ~/dev/sci.configs (holyjak-add-fulcro) $

borkdude11:09:50

try to require [goog.object] explicitly when you use it from any code

borkdude11:09:00

there was a difference in a recent ClojureScript version about this

borkdude11:09:56

seems to be ok in fulcro

Jakub Holý (HolyJak)11:09:08

Fulcro itself uses goog. But it fails at some place that seems unrelated to Fulcro itself to mee https://clojurians.slack.com/archives/C6N245JGG/p1695987235272819?thread_ts=1695987098.691529&amp;cid=C6N245JGG

borkdude11:09:22

how can I reproduce this issue btw?

⚒️ 1
Jakub Holý (HolyJak)12:09:47

1. Get latest https://github.com/holyjak/sci.configs/blob/add-fulcro/ 2. Run bb dev:release 3. Do cd www; python3 -m http.server 8009 4. Go to http://localhost:8009/

borkdude12:09:24

can you maybe push you changes to the PR?

borkdude12:09:35

I already have that one locally

borkdude12:09:56

oh you already did

Jakub Holý (HolyJak)12:09:07

Yep, the two are the same

Jakub Holý (HolyJak)12:09:56

Oh, by reproduce do you mean the :whitespace issue, which ☝️ does, or the dom not updating issue?

borkdude12:09:27

I'm seeing the same goog issue here:

goog.provide("goog.events.EventWrapper");
   

👍 1
borkdude12:09:08

It could be an issue with asset-path

borkdude12:09:25

I changed the config to just this:

:builds   {:dev     {#_#_:compiler-options {:output-feature-set :es8
                                         :optimizations :none #_whitespace}
                      :target     :browser
                      :output-dir "www/js"
                      #_#_:asset-path "/js/dev"
                      :modules    {:dev {:init-fn development/init}}
                      :devtools   {:after-load development/reload}}}

borkdude12:09:31

and <script src="js/dev.js" type="text/javascript"></script>

borkdude12:09:43

and now I see at least Init run

borkdude12:09:47

and no errors in the console

Jakub Holý (HolyJak)12:09:27

:optimizations :none always worked, at least wrt the goog problem?!

borkdude12:09:44

in release this is advanced

borkdude12:09:53

I don't think shadow supports anything else than none or advancd

borkdude12:09:05

when I print the result of the evalstring in advanced I see:

"<h3>Hello from Fulcro!</h3>"
in the console

borkdude12:09:23

so things seem to be working if you just get rid of those extra settings

borkdude12:09:50

I moved the first sci/eval-string* from the comment form to the init function

Jakub Holý (HolyJak)12:09:51

I am quite sure that shadow supports at least simple , and most likely whitespace . Both are mentioned in the guide, and simple worked for me (and was clearly bigger than advanced)

borkdude12:09:04

thheller just told you that it doesn't :)

Jakub Holý (HolyJak)12:09:49

To replicate the Fulcro problem: 1. Modify development.cljs to have init as shown below 2. Run bb dev:release 3. Run --port 8081 --dir www 4. Access the page. It will show One and not TWO as expected

(defn init []
  (println "Init run")
  (sci/eval-string* full-ctx "
    (do
      (ns test (:require 
        [com.fulcrologic.fulcro.dom :as dom]
        [com.fulcrologic.fulcro.application :as app]
        [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
      (defonce app2 (do (println \"Def app!\") (app/fulcro-app)))
      (comp/defsc Root [_ _] (dom/div (dom/h3 \"One\")))
      (app/mount! app2 Root \"app\"))
    ")
  
  (sci/eval-string* full-ctx "
    (do
      (ns test (:require 
        [com.fulcrologic.fulcro.dom :as dom]
        [com.fulcrologic.fulcro.application :as app]
        [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
      (defonce app2 (do (println \"Def app!\") (app/fulcro-app)))
      (comp/defsc Root [_ _] (dom/div (dom/h3 \"TWO\")))
      (app/mount! app2 Root \"app\"))
    ")
  )

borkdude12:09:07

yes, it shows One here

borkdude12:09:55

When I remove the first expression, I see Two

Jakub Holý (HolyJak)12:09:45

Of course. That is the whole problem: somehow, we can only set the dom once, and never changed it - but only with :optimizations . w/o them it works fine.

borkdude12:09:54

Does this have anything to do with it?

Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: 

👎 1
Jakub Holý (HolyJak)12:09:38

If, by first expression, you mean the first eval-string* call No, the warning can be safely ignored. You get it in both working and broken version (and in my prod app, which too works just fine)

borkdude12:09:08

ok, then I suspect there is something going on with advanced compilation

borkdude12:09:22

this is not uncommon

Jakub Holý (HolyJak)12:09:26

Exactly. That is why I bug Thomas, as he is the most knowledgable person 🙂

borkdude12:09:45

let's not bug Thomas with this, it's not a bug in shadow-cljs

borkdude12:09:04

it's just probably something you emit in a SCI macro that is prone to renaming

borkdude12:09:16

like some interop thing

Jakub Holý (HolyJak)12:09:45

Could be. But how do I find that out? I used --debug but did not see any observable effect.

borkdude12:09:22

> But how do I find that out? By debugging stuff, bisecting, splitting problems into sub-problems

borkdude12:09:48

I'd try to insert some printlns just before it would (re)mount the component to see what it's doing

borkdude12:09:06

perhaps necessary to add a :local/root dependency to fulcro to insert the right stuff at the right place

borkdude13:09:00

Guess what, it does start to work when you rename some variables:

(defn init []
  (println "Init run")
  (sci/eval-string* full-ctx "
    (do
      (ns test (:require 
        [com.fulcrologic.fulcro.dom :as dom]
        [com.fulcrologic.fulcro.application :as app]
        [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
      (defonce app2 (do (println \"Def app!\") (app/fulcro-app)))
      (comp/defsc Root [_ _] (dom/div (dom/h3 \"One\")))
      (app/mount! app2 Root \"app\"))
    ")
  
  (sci/eval-string* full-ctx "
    (do
      (ns test (:require 
        [com.fulcrologic.fulcro.dom :as dom]
        [com.fulcrologic.fulcro.application :as app]
        [com.fulcrologic.fulcro.components :as comp :refer [defsc]]))
      (defonce app3 (do (println \"Def app!\") (app/fulcro-app)))
      (comp/defsc Root2 [_ _] (dom/div (dom/h3 \"TWO\")))
      (app/mount! app2 Root2 \"app\"))
    ")
  )

borkdude13:09:11

renaming the second Root to Root2 seems to be enough

Jakub Holý (HolyJak)13:09:55

Nice catch! Thank you. So it seems I can mount a new function, but not change an existing one, with :optimizations . Now to find out why, and how to make it work without renaming…

borkdude13:09:15

so maybe it's an issue with mount! OR it is an issue with defsc (in the SCI config)

borkdude13:09:09

ok, to recap: if we change defonce in defsc to def it starts working, but strangely enough it works in dev mode even with the defonce

👍 1
borkdude18:09:11

@U0522TWDA Congrats on your cool article :) Is the PR ready for merge?

Jakub Holý (HolyJak)18:09:41

Thank you! Pls wait, we are looking into some issues w/ state management with Tony. I will draftify it for now

Jakub Holý (HolyJak)18:09:46

I was thinking that it might be nice to end up with 2 commits, one which adds fulcro, and one adding the dev support. If you agree then I will clean up history so it is that way. If you want to squash it into one, then I don’t need to do that…

borkdude18:09:11

whatever you want

Jakub Holý (HolyJak)18:09:34

The PR is now ready 🙏 I did not manage to make just 2 commits, so it might be best to squash then… 😭

borkdude18:09:22

I always squash PRs

borkdude19:09:46

If you want you could contribute a playground with all libraries which we could deploy via github pages (I already have a good pages setup e.g. for https://github.com/oxalorg/4ever-clojure, so just the html + JS compiled is what I would need) no hurry, might just be cool to test a bit with the existing sci.configs

borkdude19:09:15

some other time

Jakub Holý (HolyJak)19:09:49

Thank you! > playground with all libraries which we could deploy via github So essentially something like development.cljs, but with sci prebuilt (in the way I do for my blog), and having a static page with an editor and this sci version, so people could play with it? Essentially invoke sci.configs’ bb dev:release + combine with https://gist.github.com/holyjak/4c7048f2fe385c1ae826c6603fbcb560 ?

borkdude19:09:06

yeah indeed

borkdude19:09:22

anyway, totally optional

borkdude19:09:27

thanks for your PR!