Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Commit

Permalink
Do not assume process.env.NODE_ENV is globally defined/replaced.
Browse files Browse the repository at this point in the history
Currently, because of the naked reference to process.env.NODE_ENV in
src/jsutils/instanceOf.ts, the `graphql` package must either be loaded
in an environment that supports/polyfills process.env, or minified in a
way that replaces process.env.NODE_ENV expressions with string literals
at build time.

As a concrete example of how this problem can manifest, consider running
the following command in your browser console:

  await import("https://cdn.jsdelivr.net/npm/graphql@16.1.0/jsutils/instanceOf.mjs")

Even though instanceOf.mjs is otherwise a valid ECMAScript module,
importing it in a browser throws the following exception:

  instanceOf.mjs:13 Uncaught ReferenceError: process is not defined
    at instanceOf.mjs:12:3

To make matters worse, polyfilling globalThis.process is not a safe
default option in general, because many programs use the presence of the
globalThis.process object to determine if they're running in Node.js.

In order to allow the `@apollo/client` package to run natively (without
a build step or globalThis.process polyfill) in browsers, we had to come
up with an extensive workaround that temporarily polyfills
globalThis.process.env while importing the `graphql` package, and then
immediately removes the polyfill (if it was previously undefined):
apollographql/apollo-client#8347

This PR is my attempt to make those hacks no longer necessary, by making
the `graphql` package more defensive in the way it detects
process.env.NODE_ENV, but also without interrupting various existing
ways in which process.env.NODE_ENV is exposed/handled/transformed in a
typical web application. In short, I use maybe(() =>
process.env.NODE_ENV) in place of just process.env.NODE_ENV, and
maybe(...) makes things safe.

The maybe(() => ...) utility is a convenient way to evaluate an
arbitrary expression like process.env.NODE_ENV safely in any
environment, even if not all parts of the expression are defined:

1. If process.env.NODE_ENV is defined, the maybe(() =>
   process.env.NODE_ENV) expression will correctly infer its type
   (string | undefined) and return a value of that type when called.

2. If evaluating process.env.NODE_ENV throws an exception (because
   either process or process.env are not defined), the maybe(...)
   expression will evaluate safely to undefined.

3. If a build tool replaces process.env.NODE_ENV expressions with string
   literals, the resulting expressions maybe(() => "production") or
   maybe(() => "development") will work as expected.

Other solutions like `typeof process === "object"` and
process?.env?.NODE_ENV chaining fail to satisfy one or more of these
requirements (especially the last one, given the variety of different
build tools used), and tend to require more code.
  • Loading branch information
benjamn committed Dec 13, 2021
1 parent c8bbb0a commit f9579f0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/jsutils/Maybe.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
/** Conveniently represents flow's "Maybe" type https://flow.org/en/docs/types/maybe/ */
export type Maybe<T> = null | undefined | T;

/** Useful for safely evaluating expressions like process.env.NODE_ENV in any environment */
export function maybe<T>(thunk: () => T): Maybe<T> {
try {
return thunk();
// eslint-disable-next-line no-empty
} catch {}
}
3 changes: 2 additions & 1 deletion src/jsutils/instanceOf.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { maybe } from './Maybe';
import { inspect } from './inspect';

/**
Expand All @@ -9,7 +10,7 @@ import { inspect } from './inspect';
export const instanceOf: (value: unknown, constructor: Constructor) => boolean =
/* c8 ignore next 5 */
// FIXME: https://github.com/graphql/graphql-js/issues/2317
process.env.NODE_ENV === 'production'
maybe(() => process.env.NODE_ENV) === 'production'
? function instanceOf(value: unknown, constructor: Constructor): boolean {
return value instanceof constructor;
}
Expand Down

0 comments on commit f9579f0

Please sign in to comment.