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

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

Closed
4 tasks done
bartektelec opened this issue Jul 6, 2023 · 21 comments · Fixed by #1754
Closed
4 tasks done

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

bartektelec opened this issue Jul 6, 2023 · 21 comments · Fixed by #1754
Assignees
Labels
api:graphql bug Something isn't working scope:browser Related to MSW running in a browser

Comments

@bartektelec
Copy link
Contributor

bartektelec commented Jul 6, 2023

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 14 or higher

Browsers

Chromium (Chrome, Brave, etc.)

Reproduction repository

CodeSandbox link

Reproduction steps

Install any kind of msw version - i have only tested versions newer than 1.0.1

  1. Go to the codesandbox repro
  2. See the page isnt rendering
  3. Open up a codesandbox terminal and make sure deps are installed and run yarn dev
  4. Open up the hosted server in a separate tab and look into the devtools
  5. Chrome will throw about Unexpected string and firefox throws about missing name after . operator

Screenshot:
image

It looks like the graphql maintainers already know about this issue as there is a FIXME comment with an issue linked to it on the exact place where it breaks

Current behavior

It took me some digging. since graphql introduced some breaking changes when bumping up the minor version and msw depends on
"graphql": "^15.0.0 || ^16.0.0",

Without a lock file, npm will by default install 16.7.1 as it's peer dependency. Both graphql versions 16.7.0 and 16.7.1 will cause this problem.

If you have a lock file that points to an older version of graphql as msw's dependency like 16.6.0 it will work just fine.

Expected behavior

MSW should lock its graphql dependency to not include the 16.7.x versions until stable

@bartektelec bartektelec added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser labels Jul 6, 2023
@bartektelec
Copy link
Contributor Author

bartektelec commented Jul 10, 2023

issue with graphql seems to be affecting other tools as well graphql/graphql-js#3928 and graphql/graphql-js#3925

@bartektelec bartektelec changed the title GraphQL version >16.7.0 throws exceptions about Unexpected string GraphQL version >=16.7.0 throws exceptions about Unexpected string Jul 10, 2023
@Dygerydoo
Copy link

Any updates on this?

@bartektelec
Copy link
Contributor Author

I opened a PR with a fix that pins the graphql dependency to last working version before the breaking change.
Sadly doesn't seem to get any attention from the maintainers

@Dygerydoo
Copy link

Sad to hear that, let's hope this get merged soon

@kettanaito
Copy link
Member

I left a comment on the pnpm version mismatch that prevents me from merging this fix. Please see. Thanks.

@kettanaito kettanaito added api:graphql and removed needs:triage Issues that have not been investigated yet. labels Jul 26, 2023
@kettanaito
Copy link
Member

I genuinely wonder why you aren't getting the latest graphql given the dependency range on MSW's side is rather permissive (^16.0.0 will match 16.6.0). Have you tried force re-installing the dependencies on your side / bumping graphql to the version that ships the fix?

@Dygerydoo
Copy link

Tried it, but I'm starting to suspect that is some kind of issue with NX (I just moved the project to a monorepo). So will keep looking for a solution.

@kettanaito
Copy link
Member

Sometimes a package manager will skip installing a more recent version if the local one matches the semver range. Deleting node_modules usually helps.

@bartektelec
Copy link
Contributor Author

bartektelec commented Jul 26, 2023

I genuinely wonder why you aren't getting the latest graphql given the dependency range on MSW's side is rather permissive (^16.0.0 will match 16.6.0). Have you tried force re-installing the dependencies on your side / bumping graphql to the version that ships the fix?

yeah that's where the issue was, with ^16.0.0 it'd also include the 16.7.0 and 16.7.1 that introduced that problem, so I ended up locking the peer dep graphql version to 16.6.0 manually in my package-lock.json.

that's been 3 weeks ago, don't know if graphql fixed the issue on their end since then but by looking at the issues i linked in my previous comment i assume the issue is still present

@Dygerydoo
Copy link

Dygerydoo commented Jul 26, 2023

@bartektelec Just tried with resolutions option in package json and with 16.6.0 it worked!. Cannot say the same about 16.7.0 and 16.7.1 :(

@kettanaito Thanks both for help!

@bartektelec
Copy link
Contributor Author

I left a comment on the pnpm version mismatch that prevents me from merging this fix. Please see. Thanks.

@kettanaito should be good now

@kettanaito
Copy link
Member

@bartektelec,

that's been 3 weeks ago, don't know if graphql fixed the issue on their end since then but by looking at the issues i linked in my previous comment i assume the issue is still present

Yes, the GraphQL maintainers have fixed the issue since. There was a bug in the compiler, as I understand, that evaluated the correct source code incorrectly, shipping an expression that would break on runtime when consumed.

I don't mind explicitly pinning the graphql package version to the upper range so no MSW users would get a broken graphql version.

@bartektelec
Copy link
Contributor Author

@bartektelec Just tried with resolutions option in package json and with 16.6.0 it worked!. Cannot say the same about 16.7.0 and 16.7.1 :(

@kettanaito Thanks both for help!

is your project using a bundler like vite or rollup? Could you check what version you are running?

I think I narrowed down the issue to be solved when using vite version higher or equal 4.2.1

@andyjessop
Copy link

I'm getting the same error today, fresh install, don't have graphql installed separately. Looking in package-lock shows msw has the graphql dependency as:

"graphql": "^15.0.0 || ^16.0.0",

And I have 16.8.0 resolved:

"node_modules/graphql": {
      "version": "16.8.0",
      "resolved": "https://registry.npmjs.org/graphql/-/graphql-16.8.0.tgz",
      "integrity": "sha512-0oKGaR+y3qcS5mCu1vb7KG+a89vjn06C7Ihq/dDl3jA+A8B3TKomvi3CiEcVLJQGalbu8F52LxkOym7U5sSfbg==",
      "dev": true,
      "engines": {
        "node": "^12.22.0 || ^14.16.0 || ^16.0.0 || >=17.0.0"
      }
    },

@andyjessop
Copy link

If this is a problem with >16.7.0, why isn't msw handling this by setting this as the upper limit in peerDependencies?

@bartektelec
Copy link
Contributor Author

Hey @andyjessop
if you take a look at previous comments I managed to narrow down the issue to only occur when using vite@<4.2.1 so if you can, try updating it.

If for some reason you are bound to a lower version of vite, you can change the package-lock file to not download graphql higher than 16.7.0

@asantTMC
Copy link

Hey @andyjessop if you take a look at previous comments I managed to narrow down the issue to only occur when using vite@<4.2.1 so if you can, try updating it.

If for some reason you are bound to a lower version of vite, you can change the package-lock file to not download graphql higher than 16.7.0

Updating to vite >=4.2 solves the issue however I need Vite to be 4.1. Is there a way to lock the upper graphql version resolved by msw from the package.json file instead of manually editing the lock file?

@bartektelec
Copy link
Contributor Author

@asantTMC
Allthough I have not tested it, there is a feature in npm that you can use that may help you override what dependency is being downloaded for a specific package, can you give it a try and shout out if it helped so others facing the same issue can follow the same instructions

https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides

@asantTMC
Copy link

Can confirm that using ovverides or resolutions prevent msw to depend on the graphql 16.8 version thus solving the issue

@kettanaito
Copy link
Member

Released: v1.3.2 🎉

This has been released in v1.3.2!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@fracergu
Copy link

fracergu commented Nov 8, 2023

I recently upgraded msw to version 2 and had the same problem with vite 3.2.3.
I have updated vite to the latest version and everything works correctly now.

npm i -D msw@latest and npm i -D vite@latest did the trick for me.
Be careful if you have msw versions lower than 2 as many things have changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api:graphql bug Something isn't working scope:browser Related to MSW running in a browser
Projects
None yet
6 participants