Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use __DEV__ internally instead of process.env.NODE_ENV. #8347

Merged
merged 4 commits into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@
- `InMemoryCache` now coalesces `EntityStore` updates to guarantee only one `store.merge(id, fields)` call per `id` per cache write. <br/>
[@benjamn](https://github.com/benjamn) in [#8372](https://github.com/apollographql/apollo-client/pull/8372)

### 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',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our Filesize CI check is only passing because I went ahead and reconfigured the minifier to replace __DEV__ with false, simulating a production environment.

},
},
}),
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'),
Comment on lines -100 to -120
Copy link
Member Author

@benjamn benjamn Jun 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes PR #7627 obsolete, because we're no longer using process.env.NODE_ENV internally, even in Node.js (where process is defined globally, but process.env lookups are surprisingly expensive).

},
plugins: [
extensions ? nodeResolve({ extensions }) : nodeResolve(),
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.cjs.min.js",
"maxSize": "24.5 kB"
"maxSize": "24.7 kB"
}
],
"peerDependencies": {
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 @@ -109,7 +109,7 @@ export class StoreWriter {
fields = this.applyMerges(mergeTree, entityRef, fields, 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 @@ -379,7 +379,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";
benjamn marked this conversation as resolved.
Show resolved Hide resolved

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

This file was deleted.