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

Enforce that __DEV__ is polyfilled in every entry point that uses it #8689

Merged
merged 4 commits into from Aug 24, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,9 @@
- Fix unhandled `Promise` rejection warnings/errors whose message is `Observable cancelled prematurely`. <br/>
[@benjamn](https://github.com/benjamn) in [#8676](https://github.com/apollographql/apollo-client/pull/8676)

- Enforce that `__DEV__` is polyfilled by every `@apollo/client/*` entry point that uses it. This build step considers not only explicit `__DEV__` usage but also `__DEV__` references injected near `invariant(...)` and `new InvariantError(...)` expressions. <br/>
[@benjamn](https://github.com/benjamn) in [#8689](https://github.com/apollographql/apollo-client/pull/8689)

## Apollo Client 3.4.8

### Bug Fixes
Expand Down
54 changes: 54 additions & 0 deletions config/checkDEV.ts
@@ -0,0 +1,54 @@
import * as path from "path";
import { readFileSync, promises as fs } from "fs";
import { eachFile, distDir } from "./helpers";

const entryPoints = require("./entryPoints.js");

const filesWithDEV = new Set<string>();

eachFile(distDir, async file => {
const source = await fs.readFile(file, "utf8");
if (/\b__DEV__\b/.test(source)) {
filesWithDEV.add(file);
}
}).then(() => {
const filesByEntryPoint = new Map<string, {
indexPath: string;
source: string;
files: Set<string>;
}>();

entryPoints.forEach(({ dirs }: { dirs: string[] }) => {
const relIndexPath = path.join(...dirs, "index.js");
const absIndexPath = path.join(distDir, relIndexPath);
filesByEntryPoint.set(dirs.join("/"), {
indexPath: relIndexPath,
source: readFileSync(absIndexPath, "utf8"),
files: new Set<string>(),
});
});

filesWithDEV.forEach(file => {
const entryPointDir = entryPoints.getEntryPointDirectory(file);
const info = filesByEntryPoint.get(entryPointDir);
const absEntryPointDir = path.join(distDir, entryPointDir);
const relPath = "./" + path.relative(absEntryPointDir, file);
if (info) {
info.files.add(relPath);
}
});

filesByEntryPoint.forEach(({ indexPath, source, files }, entryPointDir) => {
if (!files.size || source.indexOf("checkDEV()") >= 0) {
return;
}
const entryPointId = `@apollo/client/${entryPointDir}`;
throw new Error(`Entry point ${
entryPointId
}/index.js does not call checkDEV(), but ${
entryPointId
} contains the following files that use __DEV__: ${
Array.from(files).join(", ")
}`);
});
});
6 changes: 6 additions & 0 deletions config/entryPoints.js
Expand Up @@ -92,6 +92,12 @@ function partsAfterDist(id) {
}
}

exports.getEntryPointDirectory = function (file) {
const parts = partsAfterDist(file) || file.split(path.sep);
const len = lengthOfLongestEntryPoint(parts);
if (len >= 0) return parts.slice(0, len).join(path.sep);
};

function lengthOfLongestEntryPoint(parts) {
let node = lookupTrie;
let longest = -1;
Expand Down
3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -31,14 +31,15 @@
"scripts": {
"prebuild": "npm run clean",
"build": "tsc",
"postbuild": "npm run update-version && npm run invariants && npm run sourcemaps && npm run rollup && npm run prepdist && npm run resolve && npm run verify-version",
"postbuild": "npm run update-version && npm run invariants && npm run sourcemaps && npm run rollup && npm run prepdist && npm run resolve && npm run check-DEV && npm run verify-version",
"update-version": "node config/version.js update",
"verify-version": "node config/version.js verify",
"invariants": "ts-node-script config/processInvariants.ts",
"sourcemaps": "ts-node-script config/rewriteSourceMaps.ts",
"rollup": "rollup -c ./config/rollup.config.js",
"prepdist": "node ./config/prepareDist.js",
"resolve": "ts-node-script config/resolveModuleIds.ts",
"check-DEV": "ts-node-script config/checkDEV.ts",
"clean": "rimraf -r dist coverage lib",
"test": "jest --config ./config/jest.config.js",
"test:debug": "BABEL_ENV=server node --inspect-brk node_modules/.bin/jest --config ./config/jest.config.js --runInBand",
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/exports.ts.snap
Expand Up @@ -331,6 +331,7 @@ Array [
"buildQueryFromSelectionSet",
"canUseWeakMap",
"canUseWeakSet",
"checkDEV",
"checkDocument",
"cloneDeep",
"compact",
Expand Down
5 changes: 2 additions & 3 deletions src/cache/index.ts
@@ -1,6 +1,5 @@
import { invariant } from "ts-invariant";
import { DEV } from "../utilities";
invariant("boolean" === typeof DEV, DEV);
import { checkDEV } from "../utilities";
checkDEV();

export { Transaction, ApolloCache } from './core/cache';
export { Cache } from './core/types/Cache';
Expand Down
3 changes: 2 additions & 1 deletion src/core/index.ts
@@ -1,6 +1,7 @@
/* Core */

import { DEV } from "../utilities";
import { DEV, checkDEV } from "../utilities";
checkDEV();

export {
ApolloClient,
Expand Down
5 changes: 2 additions & 3 deletions src/errors/index.ts
@@ -1,6 +1,5 @@
import { invariant } from "ts-invariant";
import { DEV } from "../utilities";
invariant("boolean" === typeof DEV, DEV);
import { checkDEV } from "../utilities";
checkDEV();

import { GraphQLError } from 'graphql';

Expand Down
5 changes: 2 additions & 3 deletions src/link/core/index.ts
@@ -1,6 +1,5 @@
import { invariant } from "ts-invariant";
import { DEV } from "../../utilities";
invariant("boolean" === typeof DEV, DEV);
import { checkDEV } from "../../utilities";
checkDEV();

export { empty } from './empty';
export { from } from './from';
Expand Down
3 changes: 3 additions & 0 deletions src/link/http/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export {
parseAndCheckHttpResponse,
ServerParseError
Expand Down
3 changes: 3 additions & 0 deletions src/link/persisted-queries/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();
Comment on lines +1 to +2
Copy link
Member Author

Choose a reason for hiding this comment

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

It turns out a number of entry points that weren't obviously using __DEV__ actually do need it, because they use invariant or InvariantError, which we transform to an expression containing __DEV__ to enable stripping long error messages in production.


import { print } from 'graphql';
import {
DocumentNode,
Expand Down
3 changes: 3 additions & 0 deletions src/link/utils/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export { fromError } from './fromError';
export { toPromise } from './toPromise';
export { fromPromise } from './fromPromise';
Expand Down
3 changes: 3 additions & 0 deletions src/react/context/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export * from './ApolloConsumer';
export * from './ApolloContext';
export * from './ApolloProvider';
3 changes: 3 additions & 0 deletions src/react/data/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export { SubscriptionData } from './SubscriptionData';
export { OperationData } from './OperationData';
export { MutationData } from './MutationData';
Expand Down
3 changes: 3 additions & 0 deletions src/react/hoc/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export { graphql } from './graphql';

export { withQuery } from './query-hoc';
Expand Down
3 changes: 3 additions & 0 deletions src/react/hooks/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

export * from './useApolloClient';
export * from './useLazyQuery';
export * from './useMutation';
Expand Down
5 changes: 2 additions & 3 deletions src/react/index.ts
@@ -1,6 +1,5 @@
import { invariant } from "ts-invariant";
import { DEV } from "../utilities";
invariant("boolean" === typeof DEV, DEV);
import { checkDEV } from "../utilities";
checkDEV();

export {
ApolloProvider,
Expand Down
3 changes: 3 additions & 0 deletions src/react/parser/index.ts
@@ -1,3 +1,6 @@
import { checkDEV } from "../../utilities";
checkDEV();

import {
DocumentNode,
DefinitionNode,
Expand Down
6 changes: 3 additions & 3 deletions src/testing/index.ts
@@ -1,4 +1,4 @@
import { invariant } from "ts-invariant";
import { DEV } from "../utilities";
invariant("boolean" === typeof DEV, DEV);
import { checkDEV } from "../utilities";
checkDEV();

export * from '../utilities/testing';
5 changes: 5 additions & 0 deletions src/utilities/globals/index.ts
@@ -1,7 +1,12 @@
import { invariant } from "ts-invariant";

// 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 function checkDEV() {
invariant("boolean" === typeof DEV, DEV);
Comment on lines +7 to +8
Copy link
Member Author

Choose a reason for hiding this comment

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

This checkDEV function not only checks that the DEV.ts module successfully exported a boolean value (DEV), but also indirectly ensures __DEV__ actually works, because this invariant(...) expression is transformed to __DEV__ ? invariant(...) : invariant(...) during build.

}

// 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 Down
7 changes: 3 additions & 4 deletions src/utilities/index.ts
@@ -1,7 +1,6 @@
import { invariant } from "ts-invariant";
import { DEV } from "./globals";
invariant("boolean" === typeof DEV, DEV);
export { DEV }
import { DEV, checkDEV } from "./globals";
export { DEV, checkDEV }
checkDEV();

export {
DirectiveInfo,
Expand Down