new __DEV__ handling (#10915)

Co-authored-by: Jerel Miller <[email protected]>

.changeset/three-grapes-marry.md

@@ -0,0 +1,5 @@
+---
+'@apollo/client': minor
+---
+
+Changes how development-only code is bundled in the library to more reliably enable consuming bundlers to reduce production bundle sizes while keeping compatibility with non-node environments.

.github/workflows/compare-build-output.yml

@@ -0,0 +1,31 @@
+name: Compare Build Output
+on:
+  pull_request:
+    branches:
+      - main
+      - release-*
+
+concurrency: ${{ github.workflow }}-${{ github.ref }}
+
+jobs:
+  comparebuildoutput:
+    name: Compare Build Output
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repo
+        uses: actions/checkout@v3
+        with:
+          # Fetch entire git history so we have the parent commit to compare against
+          fetch-depth: 0
+      - name: Setup Node.js 18.x
+        uses: actions/setup-node@v3
+        with:
+          node-version: 18.x
+      - name: Install dependencies (with cache)
+        uses: bahmutov/npm-install@v1
+
+      - name: Run comparison script
+        id: attw
+        run: ./config/compare-build-output-to.sh $(git merge-base HEAD origin/${{ github.base_ref }}) > $GITHUB_STEP_SUMMARY
+        env:
+          RUNNER_TEMP: ${{ runner.temp }}

.github/workflows/size-limit.yml

@@ -4,6 +4,7 @@ on:
     branches:
       - main
       - release-*
+      - pr/*
 jobs:
   size:
     runs-on: ubuntu-latest

.size-limit.cjs

@@ -44,6 +44,15 @@ const checks = [
     "tslib",
     "zen-observable-ts"
   ],
-}));
+})).flatMap((value) => value.path == "dist/apollo-client.min.cjs" ? value : [{...value, limit: undefined}, {
+  ...value,
+  name: `${value.name} (production)`,
+  modifyEsbuildConfig(config){
+    config.define = {
+      "globalThis.__DEV__": `false`,
+    }
+    return config
+  }
+}]);
 
 module.exports = checks;

config/compare-build-output-to.sh

@@ -0,0 +1,89 @@
+#!/usr/bin/env bash
+set -euo pipefail
+upstream=$1
+comparison="${RUNNER_TEMP:-/tmp}/comparison_checkout"
+root=$(git rev-parse --show-toplevel)
+
+temp=$(mktemp --tmpdir="${RUNNER_TEMP:-/tmp}")
+trap 'rm -f "$temp"' EXIT
+
+patterndiff(){
+  cd dist || { echo "dist folder not found"; exit 1; }
+  count=0
+  while IFS= read -r -d '' file
+  do
+    if ! filediff="$(diff <(tr "'" '"' < "$comparison/dist/$file") <(tr "'" '"' < "$root/dist/$file") -w)"; then
+      (( count++ ))
+      echo "$file"
+      if [[ "$file" == *.min.* ]]; then
+        echo "> Minified file differs."
+      else
+        echo "$filediff"
+      fi
+    fi
+  done >"$temp" < <(find . -name "$1" -print0)
+
+  output="$(cat <"$temp")"
+
+  cat <<EOF
+
+## differences in $1 files
+
+<details>
+  <summary>
+
+### $count files with differences
+
+  </summary>
+
+\`\`\`diff
+
+$output
+
+\`\`\`
+
+</details>
+EOF
+
+  cd ..
+}
+
+[ -z "$upstream" ] && { echo "need upstream argument"; exit 1; }
+
+git worktree add --force --detach --checkout "$comparison" "$upstream" || { cd "$comparison" && git checkout "$upstream"; } || exit 1
+
+cd "$comparison" || { echo "checkout failed"; exit 1; }
+cp -r "$root/node_modules" .
+npm i >&2
+git status >&2
+npm run build >&2
+cd "$root" || exit 1
+git status >&2
+npm run build >&2
+
+set +e
+
+patterndiff "*.js"
+patterndiff "*.cjs"
+patterndiff "*.d.ts"
+
+cat <<EOF
+
+
+## differences in other files
+
+<details>
+  <summary>
+
+### $(diff -qr "$comparison/dist" "dist" -x "*.map" -x "*.native.*" -x "*.js" -x "*.cjs" -x "*.d.ts" -w | wc -l) files with differences
+
+  </summary>
+
+\`\`\`diff
+
+$(diff -r "$comparison/dist" "dist" -x "*.map" -x "*.native.*" -x "*.js" -x "*.cjs" -x "*.d.ts" -w)
+
+\`\`\`
+
+</details>
+EOF

config/jest.config.js

@@ -3,6 +3,9 @@ const defaults = {
   preset: "ts-jest",
   testEnvironment: "jsdom",
   setupFilesAfterEnv: ["<rootDir>/config/jest/setup.ts"],
+  globals: {
+    __DEV__: true,
+  },
   testEnvironmentOptions: {
     url: "http://localhost",
   },

config/postprocessDist.ts

@@ -3,6 +3,15 @@ import * as path from "path";
 import resolve from "resolve";
 import { distDir, eachFile, reparse, reprint } from './helpers';
 
+const globalTypesFile = path.resolve(distDir, "utilities/globals/global.d.ts");
+fs.writeFileSync(globalTypesFile, 
+  fs.readFileSync(globalTypesFile, "utf8")
+    .split("\n")
+    .filter(line => line.trim() !== 'const __DEV__: boolean;')
+    .join("\n"), 
+  "utf8"
+);
+
 // The primary goal of the 'npm run resolve' script is to make ECMAScript
 // modules exposed by Apollo Client easier to consume natively in web browsers,
 // without bundling, and without help from package.json files. It accomplishes
@@ -22,28 +31,6 @@ eachFile(distDir, (file, relPath) => new Promise((resolve, reject) => {
     const tr = new Transformer;
     const output = tr.transform(source, file);
 
-    if (
-      /\b__DEV__\b/.test(source) &&
-      // Ignore modules that reside within @apollo/client/utilities/globals.
-      relPath.split(path.sep, 2).join("/") !== "utilities/globals"
-    ) {
-      let importsUtilitiesGlobals = false;
-
-      tr.absolutePaths.forEach(absPath => {
-        const distRelativePath =
-          path.relative(distDir, absPath).split(path.sep).join("/");
-        if (distRelativePath === "utilities/globals/index.js") {
-          importsUtilitiesGlobals = true;
-        }
-      });
-
-      if (!importsUtilitiesGlobals) {
-        reject(new Error(`Module ${
-          relPath
-        } uses __DEV__ but does not import @apollo/client/utilities/globals`));
-      }
-    }
-
     if (source === output) {
       resolve(file);
     } else {

config/processInvariants.ts

@@ -127,14 +127,7 @@ function getErrorCode(
 }
 
 function transform(code: string, relativeFilePath: string) {
-  // If the code doesn't seem to contain anything invariant-related, we
-  // can skip parsing and transforming it.
-  if (!/invariant/i.test(code)) {
-    return code;
-  }
-
   const ast = reparse(code);
-  let addedDEV = false;
 
   recast.visit(ast, {
     visitCallExpression(path) {
@@ -198,62 +191,31 @@ function transform(code: string, relativeFilePath: string) {
         if (isDEVLogicalAnd(path.parent.node)) {
           return newNode;
         }
-        addedDEV = true;
         return b.logicalExpression('&&', makeDEVExpr(), newNode);
       }
     },
   });
 
-  if (addedDEV) {
-    // Make sure there's an import { __DEV__ } from "../utilities/globals" or
-    // similar declaration in any module where we injected __DEV__.
-    let foundExistingImportDecl = false;
-
+  if (!['utilities/globals/index.js', 'config/jest/setup.js'].includes(relativeFilePath))
     recast.visit(ast, {
-      visitImportDeclaration(path) {
+      visitIdentifier(path) {
         this.traverse(path);
         const node = path.node;
-        const importedModuleId = node.source.value;
-
-        // Normalize node.source.value relative to the current file.
-        if (
-          typeof importedModuleId === 'string' &&
-          importedModuleId.startsWith('.')
-        ) {
-          const normalized = posix.normalize(
-            posix.join(posix.dirname(relativeFilePath), importedModuleId)
+        if (isDEVExpr(node)) {
+          return b.binaryExpression(
+            '!==',
+            b.memberExpression(
+              b.identifier('globalThis'),
+              b.identifier('__DEV__')
+            ),
+            b.literal(false)
           );
-          if (normalized === 'utilities/globals') {
-            foundExistingImportDecl = true;
-            if (
-              node.specifiers?.some((s) =>
-                isIdWithName(s.local || s.id, '__DEV__')
-              )
-            ) {
-              return false;
-            }
-            if (!node.specifiers) node.specifiers = [];
-            node.specifiers.push(b.importSpecifier(b.identifier('__DEV__')));
-            return false;
-          }
         }
+
+        return node;
       },
     });
 
-    if (!foundExistingImportDecl) {
-      // We could modify the AST to include a new import declaration, but since
-      // this code is running at build time, we can simplify things by throwing
-      // here, because we expect invariant and InvariantError to be imported
-      // from the utilities/globals subpackage.
-      throw new Error(
-        `Missing import from "${posix.relative(
-          posix.dirname(relativeFilePath),
-          'utilities/globals'
-        )} in ${relativeFilePath}`
-      );
-    }
-  }
-
   return reprint(ast);
 }
 

config/rollup.config.js

@@ -90,7 +90,7 @@ function prepareCJSMinified(input) {
         compress: {
           toplevel: true,
           global_defs: {
-            '@__DEV__': 'false',
+            '@globalThis.__DEV__': 'false',
           },
         },
       }),

src/cache/inmemory/__tests__/roundtrip.ts

@@ -1,4 +1,3 @@
-import { __DEV__ } from "../../../utilities/globals";
 import { DocumentNode } from 'graphql';
 import gql from 'graphql-tag';
 

src/cache/inmemory/object-canon.ts

@@ -1,4 +1,3 @@
-import { __DEV__ } from "../../utilities/globals";
 import { Trie } from "@wry/trie";
 import {
   canUseWeakMap,

src/cache/inmemory/policies.ts

@@ -1,4 +1,4 @@
-import { invariant, newInvariantError, __DEV__ } from '../../utilities/globals';
+import { invariant, newInvariantError } from '../../utilities/globals';
 
 import type {
   InlineFragmentNode,

src/cache/inmemory/readFromStore.ts

@@ -1,4 +1,4 @@
-import { invariant, newInvariantError, __DEV__ } from '../../utilities/globals';
+import { invariant, newInvariantError } from '../../utilities/globals';
 
 import type {
   DocumentNode,

src/cache/inmemory/writeToStore.ts

@@ -1,4 +1,4 @@
-import { invariant, newInvariantError, __DEV__ } from '../../utilities/globals';
+import { invariant, newInvariantError } from '../../utilities/globals';
 import { equal } from '@wry/equality';
 import { Trie } from '@wry/trie';
 import type {

src/core/ApolloClient.ts

@@ -1,4 +1,4 @@
-import { invariant, newInvariantError, __DEV__ } from '../utilities/globals';
+import { invariant, newInvariantError } from '../utilities/globals';
 
 import type { ExecutionResult, DocumentNode } from 'graphql';
 

src/core/ObservableQuery.ts

@@ -1,4 +1,4 @@
-import { invariant, __DEV__ } from '../utilities/globals';
+import { invariant } from '../utilities/globals';
 import type { DocumentNode } from 'graphql';
 import { equal } from '@wry/equality';
 

src/core/QueryManager.ts

@@ -1,4 +1,4 @@
-import { invariant, newInvariantError, __DEV__ } from '../utilities/globals';
+import { invariant, newInvariantError } from '../utilities/globals';
 
 import type { DocumentNode } from 'graphql';
 // TODO(brian): A hack until this issue is resolved (https://github.com/graphql/graphql-js/issues/3356)

src/core/index.ts

@@ -1,7 +1,5 @@
 /* Core */
 
-import { __DEV__ } from '../utilities/globals';
-
 export {
   ApolloClient,
   ApolloClientOptions,

src/link/http/createHttpLink.ts

@@ -1,4 +1,4 @@
-import { __DEV__, invariant } from '../../utilities/globals';
+import { invariant } from '../../utilities/globals';
 
 import type { DefinitionNode } from 'graphql';
 

src/react/hoc/__tests__/queries/recomposeWithState.ts

@@ -2,7 +2,6 @@
 // to avoid incurring an indirect dependency on ua-parser-js via fbjs.
 
 import React, { createFactory, Component } from "react";
-import { __DEV__ } from "../../../../utilities/globals";
 
 const setStatic =
   (key: string, value: string) => (BaseComponent: React.ComponentClass) => {

src/react/hooks/useSuspenseQuery.ts

@@ -1,4 +1,4 @@
-import { invariant, __DEV__ } from '../../utilities/globals';
+import { invariant } from '../../utilities/globals';
 import { useRef, useCallback, useMemo, useEffect, useState } from 'react';
 import type {
   ApolloClient,

src/react/hooks/useSyncExternalStore.ts

@@ -1,4 +1,4 @@
-import { invariant, __DEV__ } from '../../utilities/globals';
+import { invariant } from '../../utilities/globals';
 import * as React from 'react';
 
 import { canUseLayoutEffect } from '../../utilities';
@@ -33,7 +33,7 @@ export const useSyncExternalStore: RealUseSESHookType = realHook || ((
   // always synchronous.
   const value = getSnapshot();
   if (
-    // DEVIATION: Using our own __DEV__ polyfill (from ../../utilities/globals).
+    // DEVIATION: Using __DEV__
     __DEV__ &&
     !didWarnUncachedGetSnapshot &&
     // DEVIATION: Not using Object.is because we know our snapshots will never