diff --git a/CHANGELOG.md b/CHANGELOG.md index 250c686ddbd..62e60d7d7fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,14 @@ - Make sure the `MockedResponse` `ResultFunction` type is re-exported.
[@hwillson](https://github.com/hwillson) in [#8315](https://github.com/apollographql/apollo-client/pull/8315) -### Potentially breaking changes +### Potentially disruptive changes - To avoid retaining sensitive information from mutation root field arguments, Apollo Client v3.4 automatically clears any `ROOT_MUTATION` fields from the cache after each mutation finishes. If you need this information to remain in the cache, you can prevent the removal by passing the `keepRootFields: true` option to `client.mutate`. `ROOT_MUTATION` result data are also passed to the mutation `update` function, so we recommend obtaining the results that way, rather than using `keepRootFields: true`, if possible.
[@benjamn](https://github.com/benjamn) in [#8280](https://github.com/apollographql/apollo-client/pull/8280) +- Internally, Apollo Client now controls the execution of development-only code using the `__DEV__` global variable, rather than `process.env.NODE_ENV`. While this change should not cause any visible differences in behavior, it will increase your minified+gzip bundle size by more than 3.5kB, unless you configure your minifier to replace `__DEV__` with a `true` or `false` constant, the same way you already replace `process.env.NODE_ENV` with a string literal like `"development"` or `"production"`. For an example of configuring a Create React App project without ejecting, see this pull request for our [React Apollo reproduction template](https://github.com/apollographql/react-apollo-error-template/pull/51).
+ [@benjamn](https://github.com/benjamn) in [#8347](https://github.com/apollographql/apollo-client/pull/8347) + - Internally, Apollo Client now uses namespace syntax (e.g. `import * as React from "react"`) for imports whose types are re-exported (and thus may appear in `.d.ts` files). This change should remove any need to configure `esModuleInterop` or `allowSyntheticDefaultImports` in `tsconfig.json`, but might require updating bundler configurations that specify named exports of the `react` and `prop-types` packages, to include exports like `createContext` and `createElement` ([example](https://github.com/apollographql/apollo-client/commit/16b08e1af9ba9934041298496e167aafb128c15d)).
[@devrelm](https://github.com/devrelm) in [#7742](https://github.com/apollographql/apollo-client/pull/7742) diff --git a/config/processInvariants.ts b/config/processInvariants.ts index c719d5b71bf..0a26a01e9ee 100644 --- a/config/processInvariants.ts +++ b/config/processInvariants.ts @@ -71,7 +71,7 @@ function transform(code: string, file: string) { const node = path.node; if (isCallWithLength(node, "invariant", 1)) { - if (isNodeEnvConditional(path.parent.node)) { + if (isDEVConditional(path.parent.node)) { return; } @@ -79,22 +79,22 @@ function transform(code: string, file: string) { newArgs.push(getErrorCode(file, node)); return b.conditionalExpression( - makeNodeEnvTest(), + makeDEVExpr(), + node, b.callExpression.from({ ...node, arguments: newArgs, }), - node, ); } if (node.callee.type === "MemberExpression" && isIdWithName(node.callee.object, "invariant") && isIdWithName(node.callee.property, "log", "warn", "error")) { - if (isNodeEnvLogicalOr(path.parent.node)) { + if (isDEVLogicalAnd(path.parent.node)) { return; } - return b.logicalExpression("||", makeNodeEnvTest(), node); + return b.logicalExpression("&&", makeDEVExpr(), node); } }, @@ -102,19 +102,19 @@ function transform(code: string, file: string) { this.traverse(path); const node = path.node; if (isCallWithLength(node, "InvariantError", 0)) { - if (isNodeEnvConditional(path.parent.node)) { + if (isDEVConditional(path.parent.node)) { return; } const newArgs = [getErrorCode(file, node)]; return b.conditionalExpression( - makeNodeEnvTest(), + makeDEVExpr(), + node, b.newExpression.from({ ...node, arguments: newArgs, }), - node, ); } } @@ -137,32 +137,21 @@ function isCallWithLength( node.arguments.length > length; } -function isNodeEnvConditional(node: Node) { +function isDEVConditional(node: Node) { return n.ConditionalExpression.check(node) && - isNodeEnvExpr(node.test); + isDEVExpr(node.test); } -function isNodeEnvLogicalOr(node: Node) { +function isDEVLogicalAnd(node: Node) { return n.LogicalExpression.check(node) && - node.operator === "||" && - isNodeEnvExpr(node.left); + node.operator === "&&" && + isDEVExpr(node.left); } -function makeNodeEnvTest() { - return b.binaryExpression( - "===", - b.memberExpression( - b.memberExpression( - b.identifier("process"), - b.identifier("env") - ), - b.identifier("NODE_ENV"), - ), - b.stringLiteral("production"), - ); +function makeDEVExpr() { + return b.identifier("__DEV__"); } -const referenceNodeEnvExpr = makeNodeEnvTest(); -function isNodeEnvExpr(node: Node) { - return recast.types.astNodesAreEquivalent(node, referenceNodeEnvExpr); +function isDEVExpr(node: Node) { + return isIdWithName(node, "__DEV__"); } diff --git a/config/rollup.config.js b/config/rollup.config.js index abe73fabcd2..511c832acfc 100644 --- a/config/rollup.config.js +++ b/config/rollup.config.js @@ -71,7 +71,7 @@ function prepareCJSMinified(input) { compress: { toplevel: true, global_defs: { - '@process.env.NODE_ENV': JSON.stringify('production'), + '@__DEV__': 'false', }, }, }), @@ -97,27 +97,6 @@ function prepareBundle({ exports: 'named', interop: 'esModule', externalLiveBindings: false, - // In Node.js, where these CommonJS bundles are most commonly used, - // the expression process.env.NODE_ENV can be very expensive to - // evaluate, because process.env is a wrapper for the actual OS - // environment, and lookups are not cached. We need to preserve the - // syntax of process.env.NODE_ENV expressions for dead code - // elimination to work properly, but we can apply our own caching by - // shadowing the global process variable with a stripped-down object - // that saves a snapshot of process.env.NODE_ENV when the bundle is - // first evaluated. If we ever need other process properties, we can - // add more stubs here. - intro: '!(function (process) {', - outro: [ - '}).call(this, {', - ' env: {', - ' NODE_ENV: typeof process === "object"', - ' && process.env', - ' && process.env.NODE_ENV', - ' || "development"', - ' }', - '});', - ].join('\n'), }, plugins: [ extensions ? nodeResolve({ extensions }) : nodeResolve(), diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index f5acde2fc1e..2f5236a1156 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -16,6 +16,7 @@ Array [ "NetworkStatus", "Observable", "ObservableQuery", + "__DEV__", "checkFetcher", "concat", "createHttpLink", @@ -67,6 +68,7 @@ Array [ "InMemoryCache", "MissingFieldError", "Policies", + "__DEV__", "cacheSlot", "canonicalStringify", "defaultDataIdFromObject", @@ -90,6 +92,7 @@ Array [ "NetworkStatus", "Observable", "ObservableQuery", + "__DEV__", "checkFetcher", "concat", "createHttpLink", @@ -224,6 +227,7 @@ Array [ "ApolloConsumer", "ApolloProvider", "DocumentType", + "__DEV__", "getApolloContext", "operationName", "parser", @@ -320,6 +324,7 @@ Array [ "Concast", "DeepMerger", "Observable", + "__DEV__", "addTypenameToDocument", "argumentsObjectFromField", "asyncMap", diff --git a/src/cache/index.ts b/src/cache/index.ts index 5e3f221a44f..220bfdf378a 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -1,3 +1,5 @@ +export { __DEV__ } from "../utilities"; + export { Transaction, ApolloCache } from './core/cache'; export { Cache } from './core/types/Cache'; export { DataProxy } from './core/types/DataProxy'; diff --git a/src/cache/inmemory/__tests__/roundtrip.ts b/src/cache/inmemory/__tests__/roundtrip.ts index 8facc6cbbf5..8604a09a883 100644 --- a/src/cache/inmemory/__tests__/roundtrip.ts +++ b/src/cache/inmemory/__tests__/roundtrip.ts @@ -57,7 +57,7 @@ function storeRoundtrip(query: DocumentNode, result: any, variables = {}) { const immutableResult = readQueryFromStore(reader, readOptions); expect(immutableResult).toEqual(reconstructedResult); expect(readQueryFromStore(reader, readOptions)).toBe(immutableResult); - if (process.env.NODE_ENV !== 'production') { + if (__DEV__) { try { // Note: this illegal assignment will only throw in strict mode, but that's // safe to assume because this test file is a module. diff --git a/src/cache/inmemory/object-canon.ts b/src/cache/inmemory/object-canon.ts index e436d74d7dd..22f39b4243d 100644 --- a/src/cache/inmemory/object-canon.ts +++ b/src/cache/inmemory/object-canon.ts @@ -118,7 +118,7 @@ export class ObjectCanon { // Since canonical arrays may be shared widely between // unrelated consumers, it's important to regard them as // immutable, even if they are not frozen in production. - if (process.env.NODE_ENV !== "production") { + if (__DEV__) { Object.freeze(array); } } @@ -154,7 +154,7 @@ export class ObjectCanon { // Since canonical objects may be shared widely between // unrelated consumers, it's important to regard them as // immutable, even if they are not frozen in production. - if (process.env.NODE_ENV !== "production") { + if (__DEV__) { Object.freeze(obj); } } diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index dbce333b190..7fcfa59b343 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -477,7 +477,7 @@ export class StoreReader { }), i); } - if (process.env.NODE_ENV !== 'production') { + if (__DEV__) { assertSelectionSetForIdValue(context.store, field, item); } diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index d50f54ecdec..4317aae944e 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -269,7 +269,7 @@ export class StoreWriter { incomingFields = this.applyMerges(mergeTree, entityRef, incomingFields, context); } - if (process.env.NODE_ENV !== "production" && !context.overwrite) { + if (__DEV__ && !context.overwrite) { const hasSelectionSet = (storeFieldName: string) => fieldsWithSelectionSets.has(fieldNameFromStoreName(storeFieldName)); const fieldsWithSelectionSets = new Set(); @@ -319,7 +319,7 @@ export class StoreWriter { // In development, we need to clone scalar values so that they can be // safely frozen with maybeDeepFreeze in readFromStore.ts. In production, // it's cheaper to store the scalar values directly in the cache. - return process.env.NODE_ENV === 'production' ? value : cloneDeep(value); + return __DEV__ ? cloneDeep(value) : value; } if (Array.isArray(value)) { diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 9c63094c9bd..17e068ed8b2 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -151,7 +151,7 @@ export class ApolloClient implements DataProxy { // devtools, but disable them by default in production. typeof window === 'object' && !(window as any).__APOLLO_CLIENT__ && - process.env.NODE_ENV !== 'production', + __DEV__, queryDeduplication = true, defaultOptions, assumeImmutableResults = false, @@ -205,7 +205,7 @@ export class ApolloClient implements DataProxy { /** * Suggest installing the devtools for developers who don't have them */ - if (!hasSuggestedDevtools && process.env.NODE_ENV !== 'production') { + if (!hasSuggestedDevtools && __DEV__) { hasSuggestedDevtools = true; if ( typeof window !== 'undefined' && diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 998cbb9577a..cbc9b964106 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -306,7 +306,7 @@ export class ObservableQuery< const { updateQuery } = fetchMoreOptions; if (updateQuery) { - if (process.env.NODE_ENV !== "production" && + if (__DEV__ && !warnedAboutUpdateQuery) { invariant.warn( `The updateQuery callback for fetchMore is deprecated, and will be removed diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 8e56d630f5d..ddb8170754d 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -802,7 +802,7 @@ export class QueryManager { }); } - if (process.env.NODE_ENV !== "production" && queryNamesAndDocs.size) { + if (__DEV__ && queryNamesAndDocs.size) { queryNamesAndDocs.forEach((included, nameOrDoc) => { if (!included) { invariant.warn(`Unknown query ${ @@ -1364,7 +1364,7 @@ export class QueryManager { ) => { const data = diff.result; - if (process.env.NODE_ENV !== 'production' && + if (__DEV__ && isNonEmptyArray(diff.missing) && !equal(data, {}) && !returnPartialData) { diff --git a/src/core/index.ts b/src/core/index.ts index c5c14217944..8e01d030b4c 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -1,5 +1,7 @@ /* Core */ +export { __DEV__ } from "../utilities"; + export { ApolloClient, ApolloClientOptions, diff --git a/src/react/index.ts b/src/react/index.ts index a423c3c75a1..3d9c5583b9e 100644 --- a/src/react/index.ts +++ b/src/react/index.ts @@ -1,3 +1,5 @@ +export { __DEV__ } from "../utilities"; + export { ApolloProvider, ApolloConsumer, diff --git a/src/utilities/common/__tests__/environment.ts b/src/utilities/common/__tests__/environment.ts deleted file mode 100644 index f9fe25362db..00000000000 --- a/src/utilities/common/__tests__/environment.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { isEnv, isDevelopment, isTest } from '../environment'; - -describe('environment', () => { - let keepEnv: string | undefined; - - beforeEach(() => { - // save the NODE_ENV - keepEnv = process.env.NODE_ENV; - }); - - afterEach(() => { - // restore the NODE_ENV - process.env.NODE_ENV = keepEnv; - }); - - describe('isEnv', () => { - it(`should match when there's a value`, () => { - ['production', 'development', 'test'].forEach(env => { - process.env.NODE_ENV = env; - expect(isEnv(env)).toBe(true); - }); - }); - - it(`should treat no proces.env.NODE_ENV as it'd be in development`, () => { - delete process.env.NODE_ENV; - expect(isEnv('development')).toBe(true); - }); - }); - - describe('isTest', () => { - it('should return true if in test', () => { - process.env.NODE_ENV = 'test'; - expect(isTest()).toBe(true); - }); - - it('should return true if not in test', () => { - process.env.NODE_ENV = 'development'; - expect(!isTest()).toBe(true); - }); - }); - - describe('isDevelopment', () => { - it('should return true if in development', () => { - process.env.NODE_ENV = 'development'; - expect(isDevelopment()).toBe(true); - }); - - it('should return true if not in development and environment is defined', () => { - process.env.NODE_ENV = 'test'; - expect(!isDevelopment()).toBe(true); - }); - - it('should make development as the default environment', () => { - delete process.env.NODE_ENV; - expect(isDevelopment()).toBe(true); - }); - }); -}); diff --git a/src/utilities/common/environment.ts b/src/utilities/common/environment.ts deleted file mode 100644 index 21d61f5f122..00000000000 --- a/src/utilities/common/environment.ts +++ /dev/null @@ -1,20 +0,0 @@ -export function getEnv(): string | undefined { - if (typeof process !== 'undefined' && process.env.NODE_ENV) { - return process.env.NODE_ENV; - } - - // default environment - return 'development'; -} - -export function isEnv(env: string): boolean { - return getEnv() === env; -} - -export function isDevelopment(): boolean { - return isEnv('development') === true; -} - -export function isTest(): boolean { - return isEnv('test') === true; -} diff --git a/src/utilities/common/global.ts b/src/utilities/common/global.ts new file mode 100644 index 00000000000..6464e27a3ca --- /dev/null +++ b/src/utilities/common/global.ts @@ -0,0 +1,15 @@ +import { maybe } from "./maybe"; + +declare global { + const __DEV__: boolean | undefined; +} + +export default ( + maybe(() => globalThis) || + maybe(() => window) || + maybe(() => self) || + maybe(() => global) || + maybe(() => Function("return this")()) +) as typeof globalThis & { + __DEV__: typeof __DEV__; +}; diff --git a/src/utilities/common/maybe.ts b/src/utilities/common/maybe.ts new file mode 100644 index 00000000000..dd9d4c90d80 --- /dev/null +++ b/src/utilities/common/maybe.ts @@ -0,0 +1,3 @@ +export function maybe(thunk: () => T): T | undefined { + try { return thunk() } catch {} +} diff --git a/src/utilities/common/maybeDeepFreeze.ts b/src/utilities/common/maybeDeepFreeze.ts index f8e9d483d1b..7cac2539b87 100644 --- a/src/utilities/common/maybeDeepFreeze.ts +++ b/src/utilities/common/maybeDeepFreeze.ts @@ -1,4 +1,4 @@ -import { isDevelopment, isTest } from './environment'; +import '../fixes'; // For __DEV__ import { isNonNullObject } from './objects'; function deepFreeze(value: any) { @@ -15,7 +15,7 @@ function deepFreeze(value: any) { } export function maybeDeepFreeze(obj: T): T { - if (process.env.NODE_ENV !== "production" && (isDevelopment() || isTest())) { + if (__DEV__) { deepFreeze(obj); } return obj; diff --git a/src/utilities/fixes/__DEV__.ts b/src/utilities/fixes/__DEV__.ts new file mode 100644 index 00000000000..2ff0f7aedfc --- /dev/null +++ b/src/utilities/fixes/__DEV__.ts @@ -0,0 +1,21 @@ +import global from "../common/global"; +import { maybe } from "../common/maybe"; + +function getDEV() { + try { + return Boolean(__DEV__); + } catch { + Object.defineProperty(global, "__DEV__", { + // 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, + }); + return global.__DEV__; + } +} + +export default getDEV(); diff --git a/src/utilities/fixes/graphql.ts b/src/utilities/fixes/graphql.ts new file mode 100644 index 00000000000..14acd0d021d --- /dev/null +++ b/src/utilities/fixes/graphql.ts @@ -0,0 +1,14 @@ +// The ordering of these imports is important, because it ensures the temporary +// process.env.NODE_ENV polyfill is defined globally (if necessary) before we +// import { isType } from 'graphql'. The instanceOf function that we really care +// about (the one that uses process.env.NODE_ENV) is not exported from the +// top-level graphql package, but isType uses instanceOf, and is exported. +import { undo } from './process'; +import { isType } from 'graphql'; + +export function applyFixes() { + // Calling isType here just to make sure it won't be tree-shaken away, + // provided applyFixes is called elsewhere. + isType(null); + return undo(); +} diff --git a/src/utilities/fixes/index.ts b/src/utilities/fixes/index.ts new file mode 100644 index 00000000000..426dfd68a9f --- /dev/null +++ b/src/utilities/fixes/index.ts @@ -0,0 +1,12 @@ +// Just in case the graphql package switches from process.env.NODE_ENV to +// __DEV__, make sure __DEV__ is polyfilled before importing graphql. +import { default as __DEV__ } from "./__DEV__"; +export { __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. +import { applyFixes } from "./graphql"; + +// Synchronously undo the global process.env.NODE_ENV polyfill that we created +// temporarily while importing the offending graphql/jsutils/instanceOf module. +applyFixes(); diff --git a/src/utilities/fixes/process.ts b/src/utilities/fixes/process.ts new file mode 100644 index 00000000000..40a31373693 --- /dev/null +++ b/src/utilities/fixes/process.ts @@ -0,0 +1,41 @@ +import { maybe } from "../common/maybe"; +import global from "../common/global"; + +let needToUndo = false; + +if (global && maybe(() => process.env.NODE_ENV) === void 0) { + // Inherit from process and process.env, just in case they are defined. + const stub = Object.create(maybe(() => process) || null); + stub.env = Object.create(stub.env || null, { + NODE_ENV: { + // This default needs to be "production" instead of "development", to + // avoid the problem https://github.com/graphql/graphql-js/pull/2894 + // will eventually solve, once merged. + value: "production", + }, + }); + + try { + Object.defineProperty(global, "process", { + value: stub, + // Let anyone else change global.process as they see fit, but hide it from + // Object.keys(global) enumeration. + configurable: true, + enumerable: false, + writable: true, + }); + } catch { + // If the global object is immutable, then we're out of luck, but we + // shouldn't crash the application just because of that. + } + + // We expect this to be true now. + needToUndo = "process" in global; +} + +export function undo() { + if (needToUndo) { + delete (global as any).process; + needToUndo = false; + } +} diff --git a/src/utilities/index.ts b/src/utilities/index.ts index b5d8b3a51db..87e28b1e9d3 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -1,3 +1,5 @@ +export { __DEV__ } from "./fixes"; + export { DirectiveInfo, InclusionDirectives,