Skip to content

Commit

Permalink
Merge pull request #10521 from apollographql/attempt-to-simplify-__DEV__
Browse files Browse the repository at this point in the history
Simplify `__DEV__` strategy to use internal imports instead of global scope.
  • Loading branch information
benjamn authored Feb 7, 2023
2 parents a8c0c5a + fbf7294 commit 3a1369c
Show file tree
Hide file tree
Showing 28 changed files with 116 additions and 112 deletions.
5 changes: 5 additions & 0 deletions .changeset/lazy-teachers-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': minor
---

Simplify `__DEV__` polyfill to use imports instead of global scope
2 changes: 1 addition & 1 deletion config/bundlesize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { join } from "path";
import { gzipSync } from "zlib";
import bytes from "bytes";

const gzipBundleByteLengthLimit = bytes("33.58KB");
const gzipBundleByteLengthLimit = bytes("33.7KB");
const minFile = join("dist", "apollo-client.min.cjs");
const minPath = join(__dirname, "..", minFile);
const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength;
Expand Down
25 changes: 0 additions & 25 deletions config/distSpotFixes.ts

This file was deleted.

3 changes: 0 additions & 3 deletions config/postprocessDist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ 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
65 changes: 58 additions & 7 deletions config/processInvariants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as fs from "fs";
import * as path from "path";
import { posix, join as osPathJoin } from "path";
import { distDir, eachFile, reparse, reprint } from './helpers';

eachFile(distDir, (file, relPath) => {
Expand All @@ -10,7 +10,7 @@ eachFile(distDir, (file, relPath) => {
}
}).then(() => {
fs.writeFileSync(
path.join(distDir, "invariantErrorCodes.js"),
osPathJoin(distDir, "invariantErrorCodes.js"),
recast.print(errorCodeManifest, {
tabWidth: 2,
}).code + "\n",
Expand Down Expand Up @@ -56,14 +56,15 @@ function getErrorCode(
return numLit;
}

function transform(code: string, file: string) {
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) {
Expand All @@ -76,8 +77,9 @@ function transform(code: string, file: string) {
}

const newArgs = node.arguments.slice(0, 1);
newArgs.push(getErrorCode(file, node));
newArgs.push(getErrorCode(relativeFilePath, node));

addedDEV = true;
return b.conditionalExpression(
makeDEVExpr(),
node,
Expand All @@ -94,6 +96,7 @@ function transform(code: string, file: string) {
if (isDEVLogicalAnd(path.parent.node)) {
return;
}
addedDEV = true;
return b.logicalExpression("&&", makeDEVExpr(), node);
}
},
Expand All @@ -106,8 +109,9 @@ function transform(code: string, file: string) {
return;
}

const newArgs = [getErrorCode(file, node)];
const newArgs = [getErrorCode(relativeFilePath, node)];

addedDEV = true;
return b.conditionalExpression(
makeDEVExpr(),
node,
Expand All @@ -120,11 +124,58 @@ function transform(code: string, file: string) {
}
});

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

function isIdWithName(node: Node, ...names: string[]) {
return n.Identifier.check(node) &&
function isIdWithName(node: Node | null | undefined, ...names: string[]) {
return node && n.Identifier.check(node) &&
names.some(name => name === node.name);
}

Expand Down
12 changes: 11 additions & 1 deletion src/__tests__/__snapshots__/exports.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -391,19 +391,29 @@ Array [
"hasAnyDirectives",
"hasClientExports",
"hasDirectives",
"isAsyncIterableIterator",
"isBlob",
"isDocumentNode",
"isExecutionPatchIncrementalResult",
"isExecutionPatchInitialResult",
"isExecutionPatchResult",
"isField",
"isInlineFragment",
"isNodeReadableStream",
"isNodeResponse",
"isNonEmptyArray",
"isNonNullObject",
"isReadableStream",
"isReference",
"isStreamableBlob",
"iterateObserversSafely",
"makeReference",
"makeUniqueId",
"maybe",
"maybeDeepFreeze",
"mergeDeep",
"mergeDeepArray",
"mergeIncrementalData",
"mergeOptions",
"offsetLimitPagination",
"relayStylePagination",
Expand All @@ -424,7 +434,7 @@ exports[`exports of public entry points @apollo/client/utilities/globals 1`] = `
Array [
"DEV",
"InvariantError",
"checkDEV",
"__DEV__",
"global",
"invariant",
"maybe",
Expand Down
1 change: 1 addition & 0 deletions src/cache/inmemory/__tests__/roundtrip.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { __DEV__ } from "../../../utilities/globals";
import { DocumentNode } from 'graphql';
import gql from 'graphql-tag';

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

import { __DEV__ } from "../../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, InvariantError } from '../../utilities/globals';
import { invariant, InvariantError, __DEV__ } from '../../utilities/globals';

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

import {
DocumentNode,
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, InvariantError } from '../../utilities/globals';
import { invariant, InvariantError, __DEV__ } from '../../utilities/globals';
import { equal } from '@wry/equality';
import { Trie } from '@wry/trie';
import {
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, InvariantError } from '../utilities/globals';
import { invariant, InvariantError, __DEV__ } from '../utilities/globals';

import { 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 } from '../utilities/globals';
import { invariant, __DEV__ } from '../utilities/globals';
import { DocumentNode } from 'graphql';
import { equal } from '@wry/equality';

Expand Down
2 changes: 1 addition & 1 deletion src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { equal } from "@wry/equality";

import { Cache, ApolloCache } from '../cache';
import { DeepMerger } from "../utilities"
import { mergeIncrementalData } from '../utilities/common/incrementalResult';
import { mergeIncrementalData } from '../utilities';
import { WatchQueryOptions, ErrorPolicy } from './watchQueryOptions';
import { ObservableQuery, reobserveCacheFirst } from './ObservableQuery';
import { QueryListener } from './types';
Expand Down
4 changes: 2 additions & 2 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { invariant, InvariantError } from '../utilities/globals';
import { invariant, InvariantError, __DEV__ } from '../utilities/globals';

import { DocumentNode } from 'graphql';
// TODO(brian): A hack until this issue is resolved (https://github.com/graphql/graphql-js/issues/3356)
Expand All @@ -9,7 +9,7 @@ import { ApolloLink, execute, FetchResult } from '../link/core';
import {
isExecutionPatchIncrementalResult,
isExecutionPatchResult,
} from '../utilities/common/incrementalResult';
} from '../utilities';
import { Cache, ApolloCache, canonicalStringify } from '../cache';

import {
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 '../../utilities/globals';
import { __DEV__ } from '../../utilities/globals';

import { visit, DefinitionNode, VariableDefinitionNode } 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/common/responseIterator";
} from "../../utilities";

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 '../../../../utilities/globals'; // For __DEV__
import { __DEV__ } from "../../../../utilities/globals";

const setStatic =
(key: string, value: string) => (BaseComponent: React.ComponentClass) => {
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks/internal/useIsomorphicLayoutEffect.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useLayoutEffect, useEffect } from 'react';
import { canUseLayoutEffect } from '../../../utilities/common/canUse';
import { canUseLayoutEffect } from '../../../utilities';

export const useIsomorphicLayoutEffect = canUseLayoutEffect
? useLayoutEffect
Expand Down
3 changes: 1 addition & 2 deletions src/react/hooks/useSubscription.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import '../../utilities/globals';
import { invariant } from '../../utilities/globals';
import { useState, useRef, useEffect } from 'react';
import { DocumentNode } from 'graphql';
import { TypedDocumentNode } from '@graphql-typed-document-node/core';
import { invariant } from '../../utilities/globals'
import { equal } from '@wry/equality';

import { DocumentType, verifyDocumentType } from '../parser';
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks/useSuspenseQuery.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { invariant, __DEV__ } from '../../utilities/globals';
import { useRef, useEffect, useCallback, useMemo, useState } from 'react';
import { equal } from '@wry/equality';
import {
Expand All @@ -11,7 +12,6 @@ import {
WatchQueryOptions,
WatchQueryFetchPolicy,
} from '../../core';
import { invariant } from '../../utilities/globals';
import {
compact,
Concast,
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 } from '../../utilities/globals';
import { invariant, __DEV__ } 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 '../globals'; // For __DEV__
import { __DEV__ } from '../globals';
import { isNonNullObject } from './objects';

function deepFreeze(value: any) {
Expand Down
Loading

0 comments on commit 3a1369c

Please sign in to comment.