search

bhauman / devcards

Devcards aims to provide a visual REPL experience for ClojureScript
1.53k stars 116 forks source link

Removing call to start-devcard-ui for production build #132

Closed teawaterwire closed 6 years ago

teawaterwire commented 6 years ago

Hello, is there a particular reason why this has been reverted? (https://github.com/bhauman/devcards/commit/1b88761e31813c42bd136096bde7d37f822651d0#diff-78e519fe58fa3217d6178f8d7080c17c):

  (defmacro start-devcard-ui! []
-  (when (utils/devcards-active?)
-    `(devcards.core/start-devcard-ui!*)))
+  `(devcards.core/start-devcard-ui!*))

It seems that removing the call to start-devcard-ui!* when building for production could be needed if we want devcards not to have an impact on production code (as it is mentioned in the README) ?

Or maybe I'm doing it wrong and I should keep the "devcards" files separated from the rest? Given that even when I change the macro code to remove the call to start-devcard-ui!* the production build is still carrying some devcards code (finding 341 occurrences of /devcards/ in it) – there must be something I'm missing here!

Thanks 🤗

bhauman commented 6 years ago

This is simply because there shouldn't be a call to start-devcard-ui! included in a production build. start-devcard-ui! should only ever be called from your devcards build.

It is a subtle delicate balancing act to prevent devcards from being included in an advanced compiled production build. You should only use :require-macros to include it in any file that will end up in production.

If you :require it anywhere in your dependencies graph it will be included in an advanced build.

So as your project becomes more complex and moves toward something that will be deployed to production the recommendation is to move your cards into a separate namespace just as you would with tests.

teawaterwire commented 6 years ago

Thank you for the enlightening answer 😊

I didn't realize that :require-macros was doing the trick and that start-devcard-ui! shouldn't be in a production build!

I understand the recommendation of moving my cards into a separate namespace, but I'm still quite keen on keeping cards just right next to the components – switching files makes me lose focus 😉

I'm thinking of putting the call to start-devcard-ui! in a namespace that will be in the :preloads option of the cljs compiler.

Thanks a lot 🙌