Why are environment checks done before `warning()`, but not `invariant()`?

[joshwcomeau]

While going through the lib source of React, after an NPM install, I noticed some blocks like this:

// EventPropagators.js, line 41
if (process.env.NODE_ENV !== 'production') {
  process.env.NODE_ENV !== 'production' ? warning(domID, 'Dispatching id must not be null') : undefined;
}

All calls to warning() feature their own ternary environment check, so wrapping it in an environment conditional seems unnecessary.

I've already gone through and found/replaced most of the instances, but I wanted to see if there was a philosophical reason for this before submitting a PR.

Thanks!

[jimfb]

It is intentional, though I don't remember the reasoning off hand. Maybe @spicyj remembers

IIRC, It might have had to do with dead code elimination. By wrapping the warning call in a __DEV__ block, you avoid invoking a no-op function in an outside module, which makes the code path easier to optimize. In many cases the __DEV__ block is necessary anyway because you want to avoid doing work (like the warning deduplication logic, or creating values to print in the error message) if the warning is never going to be shown.

[sophiebits]

Yes, this is so the warning() calls can be completely eliminated by the minifier.

[joshwcomeau]

Ahh, makes perfect sense. Thanks!