-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
fix: import graphql
lazily
#2250
Conversation
@@ -40,7 +39,9 @@ export function parseDocumentNode(node: DocumentNode): ParsedGraphQLQuery { | |||
} | |||
} | |||
|
|||
function parseQuery(query: string): ParsedGraphQLQuery | Error { | |||
async function parseQuery(query: string): Promise<ParsedGraphQLQuery | Error> { | |||
const { parse } = await import('graphql') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import cache should help us here since every GraphQL handler calls parseQuery()
.
I can confirm this fixes the issue on a fresh Vitest project. |
I can confirm it works in my project as well |
Thanks for verifying this, @Lalem001 👍 Waiting for the final build and then releasing this. |
Released: v2.4.1 🎉This has been released in v2.4.1! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
@kettanaito Just wanted to say thank you so much for this fix! I installed |
@calebevans-modeln, my pleasure! It looks like some folks are still experiencing it. Glad to hear it was solved for you. Looking forward to more reproduction repos to hunt it down for good. |
Unfortunately, this didn't fix the issue for us. Running the TypeScript compiler
I'd try fine-grained imports as recommended here, but I cannot figure out from which file to import |
@jgosmann, does setting |
Jest patchIf anybody's having this issue still, try #2258. It ships |
Hello, it seems the fix is not working for me. I tried granular import and it's working but when I try to import |
@flaviodelgrosso, we don't export |
This broke our Webpack setup, since it can't resolve the We needed to manually add a resolution fallback of resolve: {
fallback: {
graphql: false
}
} |
Yes, the error disappears with that setting, but I'd see this rather as a workaround than a fix. |
@maxdavidson, when I was working with webpack last time in Next.js 14 support, I've encountered a weird bug that webpack extracts dynamic imports and evaluates them on the root scope of the module. I wonder if that's a behavior consistent to webpack in general, not just in Next.js. Can you please submit a reproduction repo with webpack for me to look at? |
We've managed to solve the following error in Storybook+Vite by downgrading to 2.4.0 (the one right before this PR):
Seems like the series of the recent attempts to make the |
@grebenyuksv, this has been rolled back on the latest release of MSW. Please update to the latest. Thanks. |
Wow, thanks for responding so fast, @kettanaito! Rolled back or re-worked? In the currently latest 2.4.9, I'm seeing
For our case, however, only the static import works. By no means am I saying the problem lies inside MSW. On the contrary, I'm inclined to think it's somewhere in the other parts of our Storybook + Vite + [apparently, Jest-based] @storybook/test-runner. I don't have time to dive deeper, so I just wanted to share the workaround in case anyone encounters this. |
@grebenyuksv, thanks for pointing that out. It looks like I've reverted that change poorly. It should be a top-level import like it was before. Would you like to open a PR to fix that? |
Ok, @kettanaito, will do within 48 hours. |
This reverts commit 1799e06.
It doesn't let me request your review, @kettanaito 🤔 #2298 |
GraphQLHandler
) #2247Changes
MSW depends on
graphql
only when using thegraphql
request handlers, and only to import itsparse()
method (other imports are type imports and are not present on runtime, neither can they affect it).graphql
a dynamic import where it's needed. This way, even though MSW exports{ graphql } from './graphql'
in the root, it will never result inimport X from 'graphql'
unless youimport { graphql } from 'msw'
on runtime.In a long term, we should drop http/graphql handlers from the root entry and keep them only in
msw/core/http
andmsw/core/graphql
.graphql
fails for any reason. This will remind the developer that they have to installgraphql
if they want to intercept GraphQL requests.