Require react directly

[filipesilva]

Hi there,

Devcards seems to depend on the global React instead of requiring it directly. When using https://github.com/thheller/shadow-cljs with Devcards, you need to directly import cljsjs.react and cljsjs.react.dom before importing devcards.core, otherwise it will error out at runtime:

  (:require [cljsjs.react]
            [cljsjs.react.dom]
            ; devcards needs cljsjs.react and cljsjs.react.dom to be imported
            ; separately for shadow-cljs to add shims.
            [devcards.core :refer [start-devcard-ui!]]
            ...

If devcards imports React directly instead, it should work though:

(:require ["react" :as react])

[lfborjas]

I ran into this same issue! If you're using React and ReactDOM from NPM (and maybe also from CLJSJS, though that's not my case,) you can also export those globals in your shadow configuration instead of explicitly requiring the cljsjs ones--my project uses NPM dependencies, and I didn't want to use CLJSJS only for the benefit of a dev tool hehe.

e.g. (the relevant bit being the :js-options entry, rest is for context: )

:devcards
    {:target :browser
     :output-dir "target/cljsbuild/public/js/devcards"
     :asset-path "/js/devcards"
     :compiler-options {:devcards true}
     :modules {:main {:entries [myapp.devcards]}}
     :js-options
     {:resolve
      {"react" {:export-globals ["React"]}
       "react-dom" {:export-globals ["ReactDOM"]}}}

However, I would also like to see this resolved in the library to not have to reach for this relatively arcane Shadow CLJS option (I only found out about it while perusing the shadow source code and finding this fun gem: https://github.com/thheller/shadow-cljs/blob/989590bb373e56b1d0ccf3a4029e2d3ec294ffe3/src/main/shadow/build/resolve.clj#L69)

Also, caveat, this codebase assumes that React is a globally defined property of js, there's a few js/Reacts scattered in a handful of files, even though the require you propose is already in place. There's an old PR by one of the maintainers of Reagent where this was pointed out and later fixed in a slightly different way. Finding out that these globals were being presupposed is what led me to configure Shadow to provide them, felt cleaner than requiring React in a different way than the strategy I use for my actual app.

[filipesilva]

Requiring cljsjs.react or cljsjs.react.dom with shadow-cljs will actually use the shims in https://github.com/thheller/shadow-cljsjs, and these are included by default (somewhat related to #156).

So following my repro doesn't actually need cljsjs, but now that I think about it some more it is extra confusing to be referring to something that isn't anywhere in the deps too.

Looking at #127 I guess this was meant be fixed in 0.2.6, but isn't? I suppose the js/React and js/ReactDOM references you mentioned, and are different between that PR and 4881652, are to blame.

[suud]

It has been fixed in v0.2.7.

[filipesilva]

Right you are @suud!