graphql-js icon indicating copy to clipboard operation
graphql-js copied to clipboard

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

Open srmagura opened this issue 1 year ago • 18 comments

Minimal repro: https://github.com/srmagura/graphql-repro

The project was created with:

npm create-vite@latest graphql-repro

Then I chose "Vanilla" and "TypeScript" at the prompts.

Install the graphql npm package and add the import

import * as graphql from "graphql";

It produces an error like:

Uncaught SyntaxError: Unexpected string (at graphql.js?v=4b27e75c:1248:113)

If you click on the error in the console, you'll see that process.env.NODE_ENV has been replaced with "development".

image

Let me know if you think this is a Vite issue instead, and I will report it there.

srmagura avatar Jun 21 '23 17:06 srmagura

This got us too -- I think the regexp here is not working quite right: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/clientInjections.ts#L88-L95 . I think this is a bonified vite bug but because both libraries are so popular this error is going to cause a lot of pain. Worth yanking, or changing the transpilation to work fine with vite out of the box?

airhorns avatar Jun 21 '23 18:06 airhorns

Just adding more info - the $ in _globalThis$process seen here is what's not accounted for in the regexp - https://diff.intrinsic.com/graphql/16.6.0/16.7.0#file-jsutils__instanceOf.mjs

So _globalThis$process.env.NODE_ENV gets changed to _globalThis$"development" by Vite

bburr avatar Jun 21 '23 18:06 bburr

@srmagura It's an issue with Vite internals, but I added a workaround for it in 16.7.1 Can you please confirm that it is fixed?

IvanGoncharov avatar Jun 22 '23 17:06 IvanGoncharov

@IvanGoncharov I was having the same problem and it was solved. Thank you.

KangMiSun17 avatar Jun 23 '23 05:06 KangMiSun17

i still have the problem, using the 16.7.1 and vite 2.9.16.
when i undo your fix, it's working.

michaelLoeffelmann avatar Jun 23 '23 07:06 michaelLoeffelmann

@IvanGoncharov I believe that the issue is still present, even with your change applied:

Screenshot 2023-06-23 at 2 46 14 PM

I believe a more common way of checking whether you are in Node is to do typeof process === 'object', like this: https://github.com/graphql/graphql-js/compare/16.x.x...lemonmade:graphql-js:fix-process-env-replacement?expand=1. @rollup/plugin-replace, which is also affected by this issue, actually has special handling for this code pattern: https://github.com/rollup/plugins/tree/master/packages/replace#objectguards.

lemonmade avatar Jun 23 '23 18:06 lemonmade

Why this happens I wrote in https://github.com/graphql/graphql-js/issues/3925

dimaMachina avatar Jun 24 '23 00:06 dimaMachina

@lemonmade did you change the default configuration of @rollup/plugin-replace?

According to the documentation on the delimiters option, the default value for that option should guard against deep property access.

Or are you using another bundler and only also mention that rollup plugin?

typeof process is problematic, as some other bundlers that actually look at an AST and don't just do blind string replacement (e.g. esbuild) are not able to replace that at all, as it is a full statement, not just a variable name/access.

phryneas avatar Jun 26 '23 06:06 phryneas

@phryneas thanks for the response. I checked again and it appears that this issue only affects the development build of Vite, and only up until version 4.2.0. Here's an example showing a slightly older Vite still getting bit by this bug. I can definitely update Vite to fix this for myself, though!

On typeof process — I think bundlers like ESBuild could still replace process with the literal undefined, couldn't they? I imagine that might not be a safe operation since it could conflict with bindings users create themselves, though.

lemonmade avatar Jun 26 '23 14:06 lemonmade

Error still present on Quasar Framework with Vite

 » Dev mode............... spa
 » Pkg quasar............. v2.12.0
 » Pkg @quasar/app-vite... v1.4.3
├─┬ @quasar/[email protected]
│ ├─┬ @quasar/[email protected]
│ │ ├── @vitejs/[email protected] deduped
│ │ ├── [email protected] deduped

ETA:

├── [email protected]

Rolling back to 16.6.0 for now but don't know how long that's gonna fly.

salcedo avatar Jun 27 '23 22:06 salcedo

Same problem here with Quasar + Vite with [email protected]

NGPixel avatar Jul 01 '23 05:07 NGPixel

I tried rolling back to 16.6.0 and I'm still having the same issue with Vite.

davidsims9t avatar Jul 01 '23 16:07 davidsims9t

@davidsims9t in that case, maybe clear your vite cache?

phryneas avatar Jul 01 '23 18:07 phryneas

Hitting this using Rollup as well as we're replacing the NODE_ENV key for React production builds as per this ancient technique: https://github.com/rollup/rollup/issues/208

The fix for me in this case was to add another replace option for graphql in rollup.config.js:

replace({
  "globalThis.process.env.NODE_ENV": JSON.stringify(process.env.NODE_ENV),
  "process.env.NODE_ENV": JSON.stringify(process.env.NODE_ENV),
  preventAssignment: true,
}),

jptaranto avatar Aug 24 '23 22:08 jptaranto

I tried rolling back to 16.6.0 and I'm still having the same issue with Vite.

16.5.0 will be fine in my case.

slhmy avatar Sep 05 '23 12:09 slhmy

I had to downgrade all the way down to 15.6.1 for it to work, for me it failed in graphql 16.6.0 with vite , suddenly out of nowhere while i was developing. After that I have not been bale to do anything to restore it, and I ended up having to test previous version, settled at 15.6.1

romanpastu avatar Apr 13 '24 16:04 romanpastu

Any updates?

unckleg avatar May 23 '24 08:05 unckleg

I hope to be merging https://github.com/graphql/graphql-js/pull/4022 after the next WG which should remove these issues... We have investigated more in https://github.com/graphql/graphql-js/issues/4075

A workaround would be to add globalThis.process and globalThis.process.env.NODE_ENV to your define config.

JoviDeCroock avatar May 23 '24 08:05 JoviDeCroock

This is fixed in https://github.com/graphql/graphql-js/pull/4022 and published in GraphQL 16.8.2, you can find the instructions for various bundlers here on how to remove it in production.

JoviDeCroock avatar Jul 11 '24 18:07 JoviDeCroock