Skip to content

Commit

Permalink
roll back __DEV__ handling
Browse files Browse the repository at this point in the history
reverts #10521
Simplify __DEV__ polyfill to use imports instead of global scope
  • Loading branch information
phryneas committed May 26, 2023
1 parent 31f2023 commit 04c59a4
Show file tree
Hide file tree
Showing 23 changed files with 96 additions and 96 deletions.
5 changes: 0 additions & 5 deletions .changeset/lazy-teachers-sell.md

This file was deleted.

25 changes: 25 additions & 0 deletions config/distSpotFixes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as fs from "fs";
import { EOL } from "os";
import { distDir } from './helpers';

export function applyDistSpotFixes() {
sanitizeDEV();
}

function sanitizeDEV() {
const globalDTsPath = `${distDir}/utilities/globals/global.d.ts`;
const globalDTs = fs.readFileSync(globalDTsPath, "utf8");
// The global.d.ts types are useful within the @apollo/client codebase to
// provide a type for the global __DEV__ constant, but actually shipping that
// declaration as a globally-declared type runs too much risk of conflict with
// other __DEV__ declarations attempting to achieve the same thing, most
// notably the one in @types/react-native/index.d.ts. We preserve the default
// export so that index.d.ts can remain unchanged, but otherwise we remove all
// traces of __DEV__ from global.d.ts.
if (/__DEV__/.test(globalDTs)) {
fs.writeFileSync(globalDTsPath, [
"declare const _default: typeof globalThis;",
"export default _default;",
].join(EOL));
}
}
3 changes: 3 additions & 0 deletions config/postprocessDist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import * as path from "path";
import resolve from "resolve";
import { distDir, eachFile, reparse, reprint } from './helpers';

import { applyDistSpotFixes } from "./distSpotFixes";
applyDistSpotFixes();

// 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
Expand Down
52 changes: 0 additions & 52 deletions config/processInvariants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ function transform(code: string, relativeFilePath: string) {
}

const ast = reparse(code);
let addedDEV = false;

recast.visit(ast, {
visitCallExpression(path) {
Expand Down Expand Up @@ -192,62 +191,11 @@ 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;

recast.visit(ast, {
visitImportDeclaration(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 (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;
}
}
},
});

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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/__snapshots__/exports.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ exports[`exports of public entry points @apollo/client/utilities/globals 1`] = `
Array [
"DEV",
"InvariantError",
"__DEV__",
"checkDEV",
"global",
"invariant",
"maybe",
Expand Down
1 change: 0 additions & 1 deletion src/cache/inmemory/__tests__/roundtrip.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { __DEV__ } from "../../../utilities/globals";
import { DocumentNode } from 'graphql';
import gql from 'graphql-tag';

Expand Down
3 changes: 2 additions & 1 deletion src/cache/inmemory/object-canon.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { __DEV__ } from "../../utilities/globals";
import "../../utilities/globals";

import { Trie } from "@wry/trie";
import {
canUseWeakMap,
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { invariant, newInvariantError, __DEV__ } from '../../utilities/globals';
import { invariant, newInvariantError } from '../../utilities/globals';

import type {
InlineFragmentNode,
Expand Down
4 changes: 2 additions & 2 deletions src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { invariant, newInvariantError, __DEV__ } from '../../utilities/globals';
import { invariant, newInvariantError } from '../../utilities/globals';

import type {
DocumentNode,
Expand Down Expand Up @@ -522,7 +522,7 @@ function assertSelectionSetForIdValue(
invariant(
!isReference(value),
`Missing selection set for object of type %s returned for query field %s`,
getTypenameFromStoreObject(store, value),
getTypenameFromStoreObject(store, value),
field.name.value
);
Object.values(value).forEach(workSet.add, workSet);
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/core/ApolloClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { invariant, newInvariantError, __DEV__ } from '../utilities/globals';
import { invariant, newInvariantError } from '../utilities/globals';

import type { ExecutionResult, DocumentNode } from 'graphql';

Expand Down
2 changes: 1 addition & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
2 changes: 1 addition & 1 deletion src/core/QueryManager.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Core */

import { __DEV__ } from '../utilities/globals';
import { DEV } from '../utilities/globals';

export {
ApolloClient,
Expand Down Expand Up @@ -90,7 +90,7 @@ export {
// Note that all invariant.* logging is hidden in production.
import { setVerbosity } from "ts-invariant";
export { setVerbosity as setLogVerbosity }
setVerbosity(__DEV__ ? "log" : "silent");
setVerbosity(DEV ? "log" : "silent");

// Note that importing `gql` by itself, then destructuring
// additional properties separately before exporting, is intentional.
Expand Down
2 changes: 1 addition & 1 deletion src/link/http/createHttpLink.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { __DEV__, invariant } from '../../utilities/globals';
import { invariant } from '../../utilities/globals';

import type { DefinitionNode } from 'graphql';

Expand Down
2 changes: 1 addition & 1 deletion src/link/http/responseIterator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
isNodeReadableStream,
isReadableStream,
isStreamableBlob,
} from "../../utilities";
} from "../../utilities/common/responseIterator";

import asyncIterator from "./iterators/async";
import nodeStreamIterator from "./iterators/nodeStream";
Expand Down
2 changes: 1 addition & 1 deletion src/react/hoc/__tests__/queries/recomposeWithState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// to avoid incurring an indirect dependency on ua-parser-js via fbjs.

import React, { createFactory, Component } from "react";
import { __DEV__ } from "../../../../utilities/globals";
import '../../../../utilities/globals'; // For __DEV__

const setStatic =
(key: string, value: string) => (BaseComponent: React.ComponentClass) => {
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks/useSuspenseQuery.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks/useSyncExternalStore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { invariant, __DEV__ } from '../../utilities/globals';
import { invariant } from '../../utilities/globals';
import * as React from 'react';

import { canUseLayoutEffect } from '../../utilities';
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/common/maybeDeepFreeze.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { __DEV__ } from '../globals';
import '../globals'; // For __DEV__
import { isNonNullObject } from './objects';

function deepFreeze(value: any) {
Expand Down
41 changes: 25 additions & 16 deletions src/utilities/globals/DEV.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
import global from "./global";
import { maybe } from "./maybe";

export default (
"__DEV__" in global
// 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__)
// To keep string-based find/replace minifiers from messing with __DEV__ inside
// string literals or properties like global.__DEV__, we construct the "__DEV__"
// string in a roundabout way that won't be altered by find/replace strategies.
const __ = "__";
const GLOBAL_KEY = [__, __].join("DEV");

// In a buildless browser environment, maybe(() => process.env.NODE_ENV)
// evaluates to undefined, so __DEV__ becomes true by default, but can be
// initialized to false instead by a script/module that runs earlier.
//
// 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"
);
function getDEV() {
try {
return Boolean(__DEV__);
} catch {
Object.defineProperty(global, GLOBAL_KEY, {
// In a buildless browser environment, maybe(() => process.env.NODE_ENV)
// evaluates as undefined, so __DEV__ becomes true by default, but can be
// initialized to false instead by a script/module that runs earlier.
value: maybe(() => process.env.NODE_ENV) !== "production",
enumerable: false,
configurable: true,
writable: true,
});
// Using computed property access rather than global.__DEV__ here prevents
// string-based find/replace strategies from munging this to global.false:
return (global as any)[GLOBAL_KEY];
}
}

export default getDEV();
20 changes: 17 additions & 3 deletions src/utilities/globals/global.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import { maybe } from "./maybe";

declare global {
interface Window {
__DEV__?: boolean;
}
// Despite our attempts to reuse the React Native __DEV__ constant instead of
// inventing something new and Apollo-specific, declaring a useful type for
// __DEV__ unfortunately conflicts (TS2451) with the global declaration in
// @types/react-native/index.d.ts.
//
// To hide that harmless conflict, we @ts-ignore this line, which should
// continue to provide a type for __DEV__ elsewhere in the Apollo Client
// codebase, even when @types/react-native is not in use.
//
// However, because TypeScript drops @ts-ignore comments when generating .d.ts
// files (https://github.com/microsoft/TypeScript/issues/38628), we also
// sanitize the dist/utilities/globals/global.d.ts file to avoid declaring
// __DEV__ globally altogether when @apollo/client is installed in the
// node_modules directory of an application.
//
// @ts-ignore
const __DEV__: boolean | undefined;
}

export default (
Expand Down
10 changes: 8 additions & 2 deletions src/utilities/globals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import { invariant, newInvariantError, InvariantError } from "./invariantWrapper
// Just in case the graphql package switches from process.env.NODE_ENV to
// __DEV__, make sure __DEV__ is polyfilled before importing graphql.
import DEV from "./DEV";
export { DEV };
export const __DEV__ = DEV;
export { DEV }
export function checkDEV() {
invariant("boolean" === typeof DEV, "%s", DEV);
}

// Import graphql/jsutils/instanceOf safely, working around its unchecked usage
// of process.env.NODE_ENV and https://github.com/graphql/graphql-js/pull/2894.
Expand All @@ -17,3 +19,7 @@ removeTemporaryGlobals();
export { maybe } from "./maybe";
export { default as global } from "./global";
export { invariant, newInvariantError, InvariantError }

// Ensure __DEV__ was properly initialized, and prevent tree-shaking bundlers
// from mistakenly pruning the ./DEV module (see issue #8674).
checkDEV();

0 comments on commit 04c59a4

Please sign in to comment.