Investigation: Bundling and shaking out our invariant error messages

[phryneas]

Due to our new bundle size check, I noticed that the invariant error messages do not get removed from production bundles with esbuild, so I started getting deeper into this.

General findings:

• No bundler will optimize something like maybe(() => process.env.NODE_ENV) !== "production" - the bundlers have no concept of maybe, so it is essentially reduced down to unknownFunction(() => "production") !== "production", not to false - and as a result, prevents code from being stripped. Replacing it with something like typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" works and results in false or true.
• Using our global file for checking for __DEV__ has a similar problem. That file essentially exports maybe(() => globalThis) || maybe(() => window) || ..., and neither will any of those maybes resolve, even if we removed them, we still end up with something like globalThis || ... || maybe(function() { return maybe.constructor("return this")() }) - with that last one explicitly trying to trick static analysis tools. So as long as global is used here, no bundler will ever be able to fill in __DEV__ with a correct value, and as a result it will not be able to strip out our invariant error messages.

So, from those findings, I went to this:

--- a/src/utilities/globals/DEV.ts
+++ b/src/utilities/globals/DEV.ts
@@ -1,13 +1,12 @@
-import global from "./global";
-import { maybe } from "./maybe";
-
 export default (
-  "__DEV__" in global
+  // @ts-ignore
+  typeof __DEV__ !== "undefined"
     // We want it to be possible to set __DEV__ globally to control the result
     // of this code, so it's important to check global.__DEV__ instead of
     // assuming a naked reference like __DEV__ refers to global scope, since
     // those references could be replaced with true or false by minifiers.
-    ? Boolean(global.__DEV__)
+    // @ts-ignore
+    ? Boolean(__DEV__)

     // In a buildless browser environment, maybe(() => process.env.NODE_ENV)
     // evaluates to undefined, so __DEV__ becomes true by default, but can be
@@ -16,5 +15,5 @@ export default (
     // If you use tooling to replace process.env.NODE_ENV with a string like
     // "development", this code will become something like maybe(() =>
     // "development") !== "production", which also works as expected.
-    : maybe(() => process.env.NODE_ENV) !== "production"
+    : typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production"
 );

Webpack

This does work in webpack using the @size-limit/webpack plugin, and manually adding config.plugins.push(new (require("webpack").DefinePlugin)({__DEV__: false}));.

It does not work in enviroments where the bundler is not aware that it has to define __DEV__, as the bundler will assume that this is a runtime thing, and will not optimize it away.

There doesn't seem to be a way around this at this moment.

ESbuild

This does not seem to work in esbuild - even doing something like

export default false

in the DEV.ts doesn't help, as esbuild doesn't seem to do this kind of optimization over file boundaries. I'm still investigating this.

The generated code essentially ends up as

  var g = !1;
// ...
var f = g;
// ...
    f
      ? (0, a.invariant)(
          'createContext' in l,
          'Invoking `getApolloContext` in an environment where `React.createContext` is not available.\nThe Apollo Client functionality you are trying to use is only available in React Client Components.\nPlease make sure to add "use client" at the top of your file.\nFor more information, see https://nextjs.org/docs/getting-started/react-essentials#client-components'
        )
      : (0, a.invariant)('createContext' in l, 29);

[phryneas]

For a related experiment, see https://github.com/apollographql/apollo-client/pull/10884

That are the savings we could have by replacing every mention of __DEV__ by typeof __DEV__ !== "undefined" ? Boolean(__DEV__) : typeof process !== "undefined" && process.env.NODE_ENV !== "production".

That's of course not a good solution in the long run :/

[benjamn]

I feel the need to clarify: I never imagined bundlers would automatically understand __DEV__, without configuration.

What I did imagine is: anyone can set up their bundlers to replace the __DEV__ identifier with a chosen constant, and that will unlock massive bundle size savings, as long as you (or your minifier) propagate the consequences by eliminating any resulting dead code.

While it may not be easy/possible for bundlers to automatically infer the value of __DEV__ in cases where it is not stripped, that is still a scenario that needs to work correctly (albeit with a larger bundle size), as when @apollo/client is loaded from an ESM-aware CDN like https://cdn.jsdelivr.net/npm/@apollo/client/+esm

[phryneas]

anyone can set up their bundlers to replace the DEV identifier with a chosen constant

I think that's actually not possible in many bundlers - in esbuild, you could replace a global __DEV__ variable, but not one imported from another file, like we do it.