dead code from `instanceOf.ts` is not tree shaked for browser due `globalThis.process` #3925

dimaMachina · 2023-06-23T11:12:34Z

after #3923 and #3501 dead code from instanceOf is no longer tree shaked for browsers

Bundlers incorrectly replace (probably RegExp) process.env.NODE_ENV with "development" resulting in:
(_globalThis$process = globalThis.process) === null ||
_globalThis$process === void 0
  ? void 0
  : _globalThis$"development";

but this happens only when using globalThis.process.env.NODE_ENV

The text was updated successfully, but these errors were encountered:

vdineva · 2023-09-28T18:30:58Z

Any update on this issue? This is a blocker for us to upgrade to newer versions of graphql and there's an advisory for the version we're currently on: GHSA-9pv7-vfvm-6vr7

radesai21 · 2023-10-05T17:52:11Z

Can we get an update on this? This is a security warning caused by this GHSA-9pv7-vfvm-6vr7

JoviDeCroock · 2024-05-11T05:39:16Z

For now you can use something like rollup-plugin-replace or the equivalent in your own bundler to do the following

{
  `globalThis.process`: JSON.stringify(true),
  `globalThis.process.env.NODE_ENV`: JSON.stringify("production"),
}

after #4022 you'll be able to look it up in the docs, see the files changed you can pick and choose your bundler.

@n1ru4l

#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 #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 #3978 Fixes #3918 Fixes #3928 Fixes #3758 Fixes #3934 This purposefully does not account for #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 #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 #4021 Supersedes #4019 Supersedes #3927 > This now also adds a documentation page on how to remove all of these

JoviDeCroock · 2024-07-12T07:28:15Z

Hey @dimaMachina #4022 is live in the latest 16.x.x line, does this solve your issue?

@n1ru4l

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

@n1ru4l

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

@n1ru4l

#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 #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 #3978 Fixes #3918 Fixes #3928 Fixes #3758 Fixes #3934 This purposefully does not account for #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 #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 #4021 Supersedes #4019 Supersedes #3927 > This now also adds a documentation page on how to remove all of these

This was referenced Jun 24, 2023

Regression: Importing the package fails in a brand new Vite project #3918

Closed

avoid using process.env with globalThis #3927

Closed

oidualc mentioned this issue Jun 24, 2023

Error in the browser console when -> import { setupWorker } from 'msw' mswjs/msw#1645

Closed

4 tasks

wreality mentioned this issue Jul 1, 2023

fix: downgrade dependency graphql/graphql-js to 16.6.0 MESH-Research/Pilcrow#1882

Merged

bartektelec mentioned this issue Jul 10, 2023

GraphQL version >=16.7.0 throws exceptions about Unexpected string mswjs/msw#1655

Closed

4 tasks

mattcosta7 mentioned this issue Nov 16, 2023

Syntax Error Prevents Code from Loading when packed in Vue/Vite app mswjs/msw#1866

Closed

4 tasks

JoviDeCroock mentioned this issue Feb 14, 2024

fix: remove globalThis check and align with what bundlers can accept #4022

Merged

dimaMachina closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dead code from `instanceOf.ts` is not tree shaked for browser due `globalThis.process` #3925

dead code from `instanceOf.ts` is not tree shaked for browser due `globalThis.process` #3925

dimaMachina commented Jun 23, 2023

vdineva commented Sep 28, 2023 •

edited

Loading

radesai21 commented Oct 5, 2023

JoviDeCroock commented May 11, 2024 •

edited

Loading

JoviDeCroock commented Jul 12, 2024

dead code from instanceOf.ts is not tree shaked for browser due globalThis.process #3925

dead code from instanceOf.ts is not tree shaked for browser due globalThis.process #3925

Comments

dimaMachina commented Jun 23, 2023

vdineva commented Sep 28, 2023 • edited Loading

radesai21 commented Oct 5, 2023

JoviDeCroock commented May 11, 2024 • edited Loading

JoviDeCroock commented Jul 12, 2024

dead code from `instanceOf.ts` is not tree shaked for browser due `globalThis.process` #3925

dead code from `instanceOf.ts` is not tree shaked for browser due `globalThis.process` #3925

vdineva commented Sep 28, 2023 •

edited

Loading

JoviDeCroock commented May 11, 2024 •

edited

Loading