Defining __DEV__ in build process conflicts with warning package

[sdemjanenko]

Is it required that __DEV__ be defined in the build process in order to optimize the build or is setting process.env.NODE_ENV enough?

When I tried setting __DEV__ to be true/false, my build process ran into an error.

 [commonjs] Unexpected keyword 'false' (17:4) in .../node_modules/warning/warning.js
file: .../node_modules/warning/warning.js:17:4
15:  */
16: 
17: var __DEV__ = process.env.NODE_ENV !== 'production';

I tracked this error down to:
https://github.com/BerkeleyTrue/warning/blob/master/warning.js#L17

It appears that the __DEV__ name is not unique enough such that the definition only targets apollo-client.

[benjamn]

@sdemjanenko There should be nothing wrong with that code declaring its own local __DEV__ variable that shadows the global one.

It sounds like you might be using a build-time transform that mistakenly replaces every occurrence of the __DEV__ identifier with true or false, including the one that comes immediately after the var keyword on line 17, even though that __DEV__ identifier is not a global variable reference. I mention this because I've seen tools that work this way before: #8347 (comment)

It appears that the __DEV__ name is not unique enough such that the definition only targets apollo-client.

The __DEV__ variable is not intended to be unique to Apollo Client. In fact, we chose it because it's already used by a number of projects, and Apollo Client benefits from that precedent. However, it's (always) important to use build tools that don't mangle your code (if that's what's wrong here—please let me know what you find out!).

[sdemjanenko]

@benjamn thanks for the fast response. I did a little more digging into ViteJS (the builder I am using) and it appears that there is an open issue for this vitejs/vite#4271.

Just to confirm for my understanding, I need to define __DEV__ at build time so that the builder can then optimize the apollo code.

[benjamn]

Yep, vitejs/vite#4271 looks like an accurate description of the problem!

Apollo Client should still work even if you don't do anything with __DEV__ at build time, so you don't need to do anything.

However, you can strip a significant amount of code (about 3kB after minification and gzip) from your JS bundle if you configure your build tool to replace any __DEV__ global variable references (not just any __DEV__ identifiers!) with true or false, because then your minifier can remove unreachable development-only code during production minification.

On balance, I think I would recommend not worrying about this optimization until vitejs/vite#4271 is fixed. Having an extra 3kB of bundle size for now probably isn't the end of the world (for most apps).

[sdemjanenko]

Awesome. Thanks for that clarification. I'm closing this issue since the bug lies elsewhere.