mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.97k stars 519 forks source link

fix: import `graphql` lazily #2250

Closed kettanaito closed 2 months ago

kettanaito commented 2 months ago

Changes

MSW depends on graphql only when using the graphql request handlers, and only to import its parse() method (other imports are type imports and are not present on runtime, neither can they affect it).

In a long term, we should drop http/graphql handlers from the root entry and keep them only in msw/core/http and msw/core/graphql.

kettanaito commented 2 months ago

I can confirm this fixes the issue on a fresh Vitest project.

Lalem001 commented 2 months ago

I can confirm this fixes the issue on a fresh Vitest project.

I can confirm it works in my project as well

kettanaito commented 2 months ago

Thanks for verifying this, @Lalem001 👍 Waiting for the final build and then releasing this.

kettanaito commented 2 months ago

Released: v2.4.1 🎉

This has been released in v2.4.1!

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.

calebevans-modeln commented 2 months ago

@kettanaito Just wanted to say thank you so much for this fix! I installed msw 2.4.0 yesterday and ran into this exact issue. Was in the middle of creating a reproduction repository this morning when I discovered I couldn't reproduce the issue anymore (because v2.4.1 was published two hours ago). All that to say, I very much appreciate the fix!

kettanaito commented 2 months ago

@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.

jgosmann commented 2 months ago

Unfortunately, this didn't fix the issue for us. Running the TypeScript compiler tsc for type checking, presents us with these errors:

node_modules/msw/lib/core/graphql.d.ts:1:30 - error TS2307: Cannot find module 'graphql' or its corresponding type declarations.

1 import { DocumentNode } from 'graphql';
                               ~~~~~~~~~

node_modules/msw/lib/core/GraphQLHandler-Cu4Xvg4S.d.ts:1:63 - error TS2307: Cannot find module 'graphql' or its corresponding type declarations.

1 import { OperationTypeNode, DocumentNode, GraphQLError } from 'graphql';
                                                                ~~~~~~~~~

Found 2 errors in 2 files.

Errors  Files
     1  node_modules/msw/lib/core/graphql.d.ts:1
     1  node_modules/msw/lib/core/GraphQLHandler-Cu4Xvg4S.d.ts:1

I'd try fine-grained imports as recommended here, but I cannot figure out from which file to import HttpResponse.

kettanaito commented 2 months ago

@jgosmann, does setting skipLibCheck: true in your tsconfig.json help?

kettanaito commented 2 months ago

Jest patch

If anybody's having this issue still, try https://github.com/mswjs/msw/pull/2258. It ships require('graphql') for CJS but keeps a dynamic import for ESM.

flaviodelgrosso commented 2 months ago

Hello, it seems the fix is not working for me. I tried granular import and it's working but when I try to import HttpResponse from 'msw/core/HttpResponse' tsc complains about not finding corresponding type declarations.

kettanaito commented 2 months ago

@flaviodelgrosso, we don't export HttpResponse as a standalone export right now so TypeScript errors on that. We should discover why the fixes aren't working.

maxdavidson commented 2 months ago

This broke our Webpack setup, since it can't resolve the graphql dependency during compilation if it's not installed.

We needed to manually add a resolution fallback of false in our Webpack config to make it work:

resolve: {
  fallback: {
    graphql: false
  }
}
jgosmann commented 2 months ago

@jgosmann, does setting skipLibCheck: true in your tsconfig.json help?

Yes, the error disappears with that setting, but I'd see this rather as a workaround than a fix.

kettanaito commented 2 months ago

@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?

grebenyuksv commented 1 month ago

We've managed to solve the following error in Storybook+Vite by downgrading to 2.4.0 (the one right before this PR):

    error: [MSW] Failed to parse a GraphQL query: cannot import the "graphql" module. Please make sure you install it if you wish to intercept GraphQL requests. See the original import error below.

    error: TypeError: Failed to fetch dynamically imported module: http://127.0.0.1:6006/node_modules/.cache/storybook/1c3385a5d25e538d10b518b310c74d3ca2690b6aaffeadccd74da79736171f86/sb-vite/deps/graphql-JJE5APMP.js?v=50c46e56
    TypeError: Failed to fetch dynamically imported module: http://127.0.0.1:6006/node_modules/.cache/storybook/1c3385a5d25e538d10b518b310c74d3ca2690b6aaffeadccd74da79736171f86/sb-vite/deps/graphql-JJE5APMP.js?v=50c46e56

Seems like the series of the recent attempts to make the graphql package truly optional has led to this. This error might totally have been caused by a misconfiguration on our side, I'm just sharing the knowledge. Narrowing the problem down to this has already taken many hours, so I can't dive deeper right now.

kettanaito commented 1 month ago

@grebenyuksv, this has been rolled back on the latest release of MSW. Please update to the latest. Thanks.

grebenyuksv commented 1 month ago

Wow, thanks for responding so fast, @kettanaito! Rolled back or re-worked?

In the currently latest 2.4.9, I'm seeing graphql imported in a slightly different way than this PR suggests but still dynamically: https://github.com/mswjs/msw/blame/452686d27cc84c1a18262e9d0b18cf64b13aa71d/src/core/utils/internal/parseGraphQLRequest.ts#L50

async function parseQuery(query: string): Promise<ParsedGraphQLQuery | Error> {
  ...
  const { parse } = require('graphql')
  ...
}

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.

kettanaito commented 1 month ago

@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?

grebenyuksv commented 1 month ago

Ok, @kettanaito, will do within 48 hours.

grebenyuksv commented 1 month ago

It doesn't let me request your review, @kettanaito 🤔 https://github.com/mswjs/msw/pull/2298