Fork me on GitHub
#re-frame
<
2023-07-13
>
alexdavis18:07:10

How do I set debug-enabled? to false in dev without it breaking production builds?

(set! interop/debug-enabled? false)
works fine for turning it off in dev (we get so many warnings it makes browsers crash), but you can't do a build with that in your code anywhere (even in a conditional) because of some closure issue
Closure compilation failed with 1 errors
--- app/globals.cljs:7
@define re_frame.interop.debug_enabled_QMARK_ has already been set at re_frame/interop.cljs:13:17.
So far the only workaround I have is to manually eval that line everytime I start a new dev server which is obviously terrible

p-himik18:07:02

As you can see, it is bound to goog/DEBUG. You can make your build tool override the default value of goog/DEBUG.

alexdavis18:07:08

I don't want to change goog/DEBUG though since its used for other things in the app... I guess I'd have to go through and point all those uses at a new debug var I define myself

alexdavis18:07:17

ok I'll do that, but I think this is an issue that should be fixed, it can't be that uncommon especially since reframe warnings are quite frequent for anyone using uix in the same project (due to false positives about subscriptions outside components)

p-himik18:07:26

As an alternative, you can copy the whole re_frame/interop.cljs file into your own sources and change the value there. But it'll require syncing every time the original file changes.

p-himik18:07:31

> it can't be that uncommon FWIW, first time I hear about it. :) And that warning is not a false positive. You are using subscriptions outside of reactive contexts (which might still be within or "within" component code).

alexdavis20:07:57

Hmm I guess I don’t understand that rule about subscriptions then, but there are far too many instances of it to clean up in this codebase (was on an old version of reframe for a long time without the warning), do you have any pointers to where I can read more about the specifics of this warning?

p-himik20:07:19

Probably the PR that added it along with the original issue. tl;dr: when there's that warning, there's a potential memory leak. Unless a subscription that's a subject of that warning is also used in a reactive context somewhere, it will never be removed from re-frame's internal subscription cache. Can't tell for sure right now but it also might incur some performance issues since cached reactions will have to be marked as dirty every single time app-db is changed. A reactive context is basically a view that's mounted or updated by Reagent. Since you've mentioned UIx, you're either using it together with Reagent or instead of Reagent with some UIx-compatible fork of re-frame. If it's the former, then Reagent has to be the "driver" of the views that use subscriptions. If it's the latter, then it's on that fork to update the warning logic.

Kimo18:07:35

Hey @U7KPK060K, This seems to work. Any caveats? https://github.com/day8/re-frame/pull/787 TLDR: try the examples/simple app on this branch. Check the values of goog.DEBUG and re_frame.interop.is_debug_enabled_QMARK_ . In your dev build, you can https://github.com/day8/re-frame/pull/787/commits/b779aaf0de42ee64be79bac9c6f8d7338c8800b2.

p-himik18:07:35

Just in case - that PR is unlikely to ever get merged as is because it has a lot of whitespace-only changes.

p-himik18:07:58

And you don't have to rename the var - seems like you can just replace def with goog-define. If that doesn't work, I'd rather add a new define than rename the existing def that other people use.

Kimo18:07:32

Yeah, we're going to merge the whitespace changes to master soon. Trying to make all our repos cljfmt-compliant.

Kimo18:07:06

Yeah, I considered that, but it didn't match our idiom elsewhere.

p-himik18:07:14

Ah, didn't realize you're with Day8. Idioms are less important than backwards compatibility.

Kimo18:07:05

The original re-frame.interop/debug-enabled? is still there unchanged for anyone using it, so I'm not sure I see the compatibility problem.

Kimo18:07:19

At least the name is there.

p-himik18:07:46

Ah, right, so seems that debug-enabled? will work exactly as before since goog-define ends up as a def anyway. Still, IMO adding a new var just as an alias seems more confusing than it is worth. Even if you already have some other var named like that (I see only is-trace-enabled?), I'd rename that (create an alias if public) instead since x? is more idiomatic in the Clojure world than is-x?.