Skip to content

Commit

Permalink
fix: remove globalThis check and align with what bundlers can accept (
Browse files Browse the repository at this point in the history
graphql#4022)

As surfaced in
[Discord](https://discord.com/channels/625400653321076807/862957336082645006/1206980831915282532)
this currently is a breaking change in the 16.x.x release line which is
preventing folks from upgrading towards a security fix. This PR should
result in a patch release on the 16 release line.

This change was originally introduced to support CFW and browser
environments which should still be supported with the `typeof` check CC
@n1ru4l

This also adds a check whether `.env` is present as in the DOM using
`id="process"` defines that as a global which we don't want to access on
accident. as shown in graphql#4017

Bundles also target `process.env.NODE_ENV` specifically which fails when
it replaces `globalThis.process.env.NODE_ENV` as this becomes
`globalThis."production"` which is invalid syntax.

Fixes graphql#3978
Fixes graphql#3918
Fixes graphql#3928
Fixes graphql#3758
Fixes graphql#3934

This purposefully does not account for
graphql#3925 as we can't address
this without breaking CF/plain browsers so the small byte-size increase
will be expected for bundled browser environments. As a middle ground we
did optimise the performance here. We can revisit this for v17.

Most bundlers will be able to tree-shake this with a little help, in
graphql#4075 (comment)
you can find a conclusion with a repo where we discuss a few.

- Next.JS by default replaces
[`process.env.NODE_ENV`](https://github.com/vercel/next.js/blob/b0ab0fe85fe8c93792051b058e060724ff373cc2/packages/next/webpack.config.js#L182)
you can add `typeof process` linearly
- Vite allows you to specify
[`config.define`](https://vitejs.dev/config/shared-options.html#define)
- ESBuild by default will replace `process.env.NODE_ENV` but does not
support replacing `typeof process`
- Rollup has a plugin for this
https://www.npmjs.com/package/@rollup/plugin-replace

Supersedes graphql#4021
Supersedes graphql#4019
Supersedes graphql#3927

> This now also adds a documentation page on how to remove all of these
  • Loading branch information
JoviDeCroock authored and yaacovCR committed Aug 19, 2024
1 parent 9bba83f commit 4db691e
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 1 deletion.
7 changes: 7 additions & 0 deletions cspell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ overrides:
words:
- clsx
- infima
- noopener
- Vite
- craco
- esbuild
- swcrc
- noreferrer
- xlink

validateDirectives: true
ignoreRegExpList:
Expand Down
8 changes: 7 additions & 1 deletion src/jsutils/instanceOf.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { inspect } from './inspect.js';

/* c8 ignore next 3 */
const isProduction =
globalThis.process != null &&
// eslint-disable-next-line no-undef
process.env.NODE_ENV === 'production';

/**
* A replacement for instanceof which includes an error warning when multi-realm
* constructors are detected.
Expand All @@ -9,7 +15,7 @@ import { inspect } from './inspect.js';
export const instanceOf: (value: unknown, constructor: Constructor) => boolean =
/* c8 ignore next 6 */
// FIXME: https://github.com/graphql/graphql-js/issues/2317
globalThis.process != null && globalThis.process.env.NODE_ENV === 'production'
isProduction
? function instanceOf(value: unknown, constructor: Constructor): boolean {
return value instanceof constructor;
}
Expand Down
127 changes: 127 additions & 0 deletions website/docs/tutorials/going-to-production.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
---
title: Going to production
category: FAQ
---

GraphQL.JS contains a few development checks which in production will cause slower performance and
an increase in bundle-size. Every bundler goes about these changes different, in here we'll list
out the most popular ones.

## Bundler-specific configuration

Here are some bundler-specific suggestions for configuring your bundler to remove `globalThis.process` and `process.env.NODE_ENV` on build time.

### Vite

```js
export default defineConfig({
// ...
define: {
'globalThis.process': JSON.stringify(true),
'process.env.NODE_ENV': JSON.stringify('production'),
},
});
```

### Next.js

```js
// ...
/** @type {import('next').NextConfig} */
const nextConfig = {
webpack(config, { webpack }) {
config.plugins.push(
new webpack.DefinePlugin({
'globalThis.process': JSON.stringify(true),
'process.env.NODE_ENV': JSON.stringify('production'),
}),
);
return config;
},
};

module.exports = nextConfig;
```

### create-react-app

With `create-react-app`, you need to use a third-party package like [`craco`](https://craco.js.org/) to modify the bundler configuration.

```js
const webpack = require('webpack');
module.exports = {
webpack: {
plugins: [
new webpack.DefinePlugin({
'globalThis.process': JSON.stringify(true),
'process.env.NODE_ENV': JSON.stringify('production'),
}),
],
},
};
```

### esbuild

```json
{
"define": {
"globalThis.process": true,
"process.env.NODE_ENV": "production"
}
}
```

### Webpack

```js
config.plugins.push(
new webpack.DefinePlugin({
'globalThis.process': JSON.stringify(true),
'process.env.NODE_ENV': JSON.stringify('production'),
}),
);
```

### Rollup

```js
export default [
{
// ... input, output, etc.
plugins: [
minify({
mangle: {
toplevel: true,
},
compress: {
toplevel: true,
global_defs: {
'@globalThis.process': JSON.stringify(true),
'@process.env.NODE_ENV': JSON.stringify('production'),
},
},
}),
],
},
];
```

### SWC

```json title=".swcrc"
{
"jsc": {
"transform": {
"optimizer": {
"globals": {
"vars": {
"globalThis.process": true,
"process.env.NODE_ENV": "production"
}
}
}
}
}
}
```
5 changes: 5 additions & 0 deletions website/sidebars.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ module.exports = {
label: 'Advanced',
items: ['tutorials/constructing-types'],
},
{
type: 'category',
label: 'FAQ',
items: ['tutorials/going-to-production'],
},
'tutorials/express-graphql',
'tutorials/defer-stream',
],
Expand Down

0 comments on commit 4db691e

Please sign in to comment.