Skip to content

Commit

Permalink
Experiment: use __DEV__ instead of process.env.NODE_ENV.
Browse files Browse the repository at this point in the history
See PR #8347 for full explanation.
  • Loading branch information
benjamn committed Jun 7, 2021
1 parent 9b522cf commit d7dbe39
Show file tree
Hide file tree
Showing 24 changed files with 154 additions and 142 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@
- Fix polling when used with `skip`. <br/>
[@brainkim](https://github.com/brainkim) in [#8346](https://github.com/apollographql/apollo-client/pull/8346)

### 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. <br/>
[@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). <br/>
[@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)). <br/>
[@devrelm](https://github.com/devrelm) in [#7742](https://github.com/apollographql/apollo-client/pull/7742)

Expand Down
45 changes: 17 additions & 28 deletions config/processInvariants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,50 +71,50 @@ 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;
}

const newArgs = node.arguments.slice(0, 1);
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);
}
},

visitNewExpression(path) {
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,
);
}
}
Expand All @@ -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__");
}
23 changes: 1 addition & 22 deletions config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function prepareCJSMinified(input) {
compress: {
toplevel: true,
global_defs: {
'@process.env.NODE_ENV': JSON.stringify('production'),
'@__DEV__': 'false',
},
},
}),
Expand All @@ -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(),
Expand Down
5 changes: 5 additions & 0 deletions src/__tests__/__snapshots__/exports.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Array [
"NetworkStatus",
"Observable",
"ObservableQuery",
"__DEV__",
"applyNextFetchPolicy",
"checkFetcher",
"concat",
Expand Down Expand Up @@ -68,6 +69,7 @@ Array [
"InMemoryCache",
"MissingFieldError",
"Policies",
"__DEV__",
"cacheSlot",
"canonicalStringify",
"defaultDataIdFromObject",
Expand All @@ -91,6 +93,7 @@ Array [
"NetworkStatus",
"Observable",
"ObservableQuery",
"__DEV__",
"applyNextFetchPolicy",
"checkFetcher",
"concat",
Expand Down Expand Up @@ -226,6 +229,7 @@ Array [
"ApolloConsumer",
"ApolloProvider",
"DocumentType",
"__DEV__",
"getApolloContext",
"operationName",
"parser",
Expand Down Expand Up @@ -322,6 +326,7 @@ Array [
"Concast",
"DeepMerger",
"Observable",
"__DEV__",
"addTypenameToDocument",
"argumentsObjectFromField",
"asyncMap",
Expand Down
2 changes: 2 additions & 0 deletions src/cache/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/__tests__/roundtrip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions src/cache/inmemory/object-canon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ export class StoreReader {
}), i);
}

if (process.env.NODE_ENV !== 'production') {
if (__DEV__) {
assertSelectionSetForIdValue(context.store, field, item);
}

Expand Down
4 changes: 2 additions & 2 deletions src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();
Expand Down Expand Up @@ -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)) {
Expand Down
4 changes: 2 additions & 2 deletions src/core/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export class ApolloClient<TCacheShape> 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,
Expand Down Expand Up @@ -205,7 +205,7 @@ export class ApolloClient<TCacheShape> 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' &&
Expand Down
2 changes: 1 addition & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ export class QueryManager<TStore> {
});
}

if (process.env.NODE_ENV !== "production" && queryNamesAndDocs.size) {
if (__DEV__ && queryNamesAndDocs.size) {
queryNamesAndDocs.forEach((included, nameOrDoc) => {
if (!included) {
invariant.warn(`Unknown query ${
Expand Down Expand Up @@ -1343,7 +1343,7 @@ export class QueryManager<TStore> {
) => {
const data = diff.result;

if (process.env.NODE_ENV !== 'production' &&
if (__DEV__ &&
isNonEmptyArray(diff.missing) &&
!equal(data, {}) &&
!returnPartialData) {
Expand Down
2 changes: 2 additions & 0 deletions src/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* Core */

export { __DEV__ } from "../utilities";

export {
ApolloClient,
ApolloClientOptions,
Expand Down
2 changes: 2 additions & 0 deletions src/react/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
export { __DEV__ } from "../utilities";

export {
ApolloProvider,
ApolloConsumer,
Expand Down
58 changes: 0 additions & 58 deletions src/utilities/common/__tests__/environment.ts

This file was deleted.

20 changes: 0 additions & 20 deletions src/utilities/common/environment.ts

This file was deleted.

0 comments on commit d7dbe39

Please sign in to comment.