What would be an alternative? IIRC React also does it, only it checks for the value of process.env.NODE_ENV and not some custom constant.

Also, given that there's no comparable setting, would it make sense to implement it? Not sure how feasible it is with GCC.

do you have an example of where this is actually used?

I know what DefinePlugin does, but I need to see the actual code shadow-cljs is processing to comment

Sure, here's a piece of code from Sentry:

function flush(timeout) {
  const client = core.getCurrentHub().getClient();
  if (client) {
    return client.flush(timeout);
  }
  (typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__) && utils.logger.warn('Cannot flush events. No client defined.');
  return utils.resolvedSyncPromise(false);
}

do you have the file that is in please?

http://unpkg.com or something, not the source file. the actual published file in the npm package

Ah, sure: https://unpkg.com/@sentry/[email protected]/cjs/sdk.js

Yeah, that was from the package. :)

what should work is just search&replace (typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__) with false before shadow-cljs processes it

Sounds like it would be incredibly brittle, no?

Just as a guess, tried (js* "/** define {boolean} */\nvar __SENTRY_DEBUG__ = false;") in the first loaded ns but didn't work.

thats the only solution I can offer 😛

yeah, this isn't a closure define. so it won't be recognized as such

well, doing it on the CLJS parts won't do anything since they aren't compiled together

So GCC doesn't use the values of what it considers constants? Only goog.defines?

thats not the main problem there

the first problem that would need solving is making it part of the code that is actually getting compiled

there currently is no :prepend-js option for npm code

appending __SENTRY_DEBUG__ = false to the files in the npm package might work

less brittle than search and replace, still manual though

can't offer you anything built-in from shadow-cljs because there is nothing

Ah, so the problem is that my project's code is basically appended to the NPM code that's already been processed? Those two aren't processed together?

What if it's not an NPM module but a plain JS file in my project's sources?

that is not compiled together with npm sources

the problem is this is commonjs

so you can't just set a var in one file and expect that to be available in all files

and I'm not sure the closure compiler will recognize global.__SENTRY_DEBUG__ = false;

thats why I'm saying to append it to every file

that should work

Ah, every file. Gotcha. And --define='__SENTRY_DEBUG__=false' also won't work, right?

don't know what --define is in this context? it is not a shadow-cljs option

Yeah, it's for GCC.

Dunno if it's exposed in its SDK.

no clue either. never used the CLI code from GCC

I'd assume this is just setting a closure-define, in which case the answer is no

I'm not aware of a setting in the closure compiler that would be equiv to the webpack DefinePlugin

if you find something let me know

It redefines a global constant at compile-time. But if I understand the docs correctly, it would only work if there is a constant with that name.

they also likely define "global constant" difrently than commonjs

Struggling to find a way to make the CLI work. Maybe you have a clue? Running

google-closure-compiler --module_resolution NODE \
 --process_common_js_modules \
 --js ./test.js \
 --js 'node_modules/@sentry/**/cjs/**.js' \
 --js 'node_modules/@sentry/**.json'

/** define {boolean} */
var __SENTRY_DEBUG__ = false;

node_modules/@sentry/browser/cjs/client.js:3:13: ERROR - [JSC_JS_MODULE_LOAD_WARNING] Failed to load module "@sentry/core"
  3| const core = require('@sentry/core');
                  ^
[...]

echo "var __SENTRY_DEBUG__ = false;" > node_modules/@sentry/browser/hack.js

(:require ["@sentry/browser/hack.js"])

would compile it together with everything else

but I can already say this won't do anything

Nah, I would like to experiment a bit with the CLI first. :) Maybe there's a way to do it without hacks.

sure, have fun. can't help, never used it beyond compiling a single file without any requires

And while I'm busy wasting my time with that - Sentry also distributes ESM modules. Would that make any difference w.r.t. the original problem?

no

I think I was able to successfully inline the __SENTRY_DEBUG__=false and subsequently eliminate all the blocks that used it. However, I had to use advanced optimizations and IIRC node modules only go through simple optimizations, which does not include variable inlining.

simple does eliminate some stuff

https://unpkg.com/[email protected]/index.js

node.getQualifiedName().equals("process.env.NODE_ENV");

How does GCC know about process.env.NODE_ENV?

https://github.com/thheller/shadow-cljs/blob/de065f237031f2e8233642c96b0030477ce2be30/src/main/shadow/build/closure/NodeEnvInlinePass.java#L26

I'm replacing it as an optimization pass

so that it becomes if ('production' === 'production') { and simple does eliminate that

Ahh, so it's shadow-cljs that enables that particular inlining. :) Perhaps that inlining step could be made configurable? So people would be able to replicate DefinePlugin by inlining whatever they want.

not sure what kind of performance impact this would have. generally not a fan of replicating every webpack feature. how much does this actually eliminate?

Not sure how to compare that properly. I can easily run one of the following: • Advanced + inlining of false with a subsequent elimination of the debugging lines • Advanced + inlining of true, so without elimination of those lines • Simple without inlining To properly compare with what a potential change to shadow-cljs I'd need to be able to run "simple with inlining and elimination". > not a fan of replicating every webpack feature Doesn't this feature sound like something that's useful in general, regardless of what webpack can do?

I gave you a couple suggestions on how you could potentially get the inlining. try them and report the results. then I might consider it. in the dark no, I don't think this is useful. JS libs are riddled with these non-standard webpack specific things

should be a 5 line script to append const __SENTRY_DEBUG__ = false; to every file

The first 2 options, when compared, have a difference of 3% in size: 32693 vs 33605. But it's just this particular case, for the particular files that I checked - not the whole of Sentry, far from it. > try them and report the results Sure, what's up?

is that size diff gzip'd?

Raw.

and gzipd?

why are you wasting your time on less than 1kb?

12405 vs 12738. I'm wasting my time on the concept, not on the size. :) In the process, I'm learning more about GCC and how different things work. It would be more interesting to compare the sizes of the whole of Sentry. Not sure how hard that would be.

Well, definitely harder than adding a bunch of files to the CLI arguments - started getting Inline JSDoc on default parameters must be marked as optional errors, no clue how to proceed, but probably not going to.

cd node_modules/@sentry/browser; find . -name "*.js" -type f -exec sh -c 'echo "\nconst __SENTRY_DEBUG__=false;" >> "$0"' {} \;

seems to eliminate the debug stuff

about 1kb gzip in total

do all the sentry packages do this or just the browser thing?

With this:

(ns hgs.x
  (:require ["@sentry/react"]
            ["@sentry/integrations"]
            ["@sentry/replay"]
            ["@sentry/tracing"]))

(defn main []
  (js/console.log 0))

Note that there's not only @sentry but also @sentry-internal.

why would you bloat your build this massively with this mess 😛

Because it's worth it. Have you seen what it actually does?

Mere 100 kB for me to be able to never ask a user to record a video - sign me up! And that's just one of the features.

Heh, according to a shadow-cljs report my own sources + CLJS in that particular project go well beyond 2MB ungzipped. So yeah, I'm not too concerned about 100 kB at this point. :) Let alone 5% of 100 kB. I'm simply curious whether it's a change that would be worthwhile to have at the tool level, for everyone.

so far nobody has ever asked for this, so can't imagine this being too common

Fair! Hmm, was sure I saw something similar in Vega and MUI, but apparently not - can't find it in the docs. In any case, seems like there's a clear way forward if that feature becomes needed. I'd like to work on it myself although of course can't provide any time estimates at the moment.

FWIW I thought about this a bit. its definitely not just a simple replacement. since (typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__) would become (typeof false === 'undefined' || false) which doesn't do what you'd want 😛

Ah, right. But how does it work with React then, with process.env.NODE_ENV?

There are a couple of build warnings, but not from shadow-cljs itself, so I don't imagine that should matter.

Oh, I see :ignore-warnings ... maybe that IS it ?

Yeah, that was it. Ok.

from what else would you get warnings? 😛

I wouldn't expect some warning from a function that a library deprecated would just make reloading stop working, with no logging or anything. It's a legitimate decision, but I think it's funny to act like no one would be surprised by this.

> but not from shadow-cljs itself

I wondered what else you expected the warning to be from?

or I misunderstood

> I think it's funny to act like no one would be surprised by this.

Maybe I didn't explain well - I know what the warnings were from - just not always how to resolve them. So in this case, I was ignoring a deprecated function warning because I don't know how NOT to use that function at the moment (reagent.dom/dom-node) ... I was just surprised when that made shadow-cljs not reload things any more.

I'm not surprised by where the warnings came from,

don't know where that came from. I'm not surprised that you didn't know this setting, why would you

I was surprised that the presence of warnings caused shadow-cljs to not reload.

it doesn't reload because it may break stuff. some warnings will actually lead to runtime errors when trying to run it anyways

thats why its the default to now reload with warnings

Yeah ... and I'm reluctant to ignore these warnings or turn them off for that very reason.

For now I'll just use :ignore-warnings and then I will turn it back off again.

Sorry for the noise and distraction!

I do not understand that description. what does a WS connection have to do with where the output lives?

shadow-cljs doesn't care where the output is? or what serves it for that matter

I'm not sure myself. I just know that when I open the page in the browser the console warns me that shadow cljs can't connect.

and what does it say exactly?

is the apache just running on the different host? domain?

No same domain, but actually ignore me, it works now. Thanks.

Idk what changed between yesterday and today.