mswjs / msw-storybook-addon

Mock API requests in Storybook with Mock Service Worker.
https://msw-sb.vercel.app
MIT License
408 stars 39 forks source link

Require function is used in a way in which dependencies cannot be statically extracted #68

Closed will-stone closed 2 years ago

will-stone commented 2 years ago

Hi,

Firstly thank-you for all the hard work on msw, it really makes mocking end-points really easy. I have found that on latest Storybook (6.4.9), there's a warning in the console:

./node_modules/msw-storybook-addon/dist/mswDecorator.js
25:45-52 Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

It is referring to this line. I have found simply commenting it out directly in my node_modules resolves the issue. Is there a reason for assigning require to __non_webpack_require__? Doesn't this overwrite the point of using __non_webpack_require__ in this instance? i.e.

  1. globalThis.__non_webpack_require__ = require is replaced by webpack to globalThis.__non_webpack_require__ = __webpack__require__
  2. Meaning the next line reads: const { setupServer } = __webpack_require__('msw/node')

I'm going to see if I can get this repo up and running in a fork and replicate the issue...

will-stone commented 2 years ago

When I installed this project it resolved to Storybook 6.3.8 and the issue was still prevalent, so it's not isolated to just the most recent version. Would you like me to submit this as a PR?

Before:

image

After removing the globalThis.__non_webpack_require__ = require line.

image
yannbf commented 2 years ago

Hey @will-stone thanks for opening the issue and doing this investigation! The reason for that code is to support Webpack 5 codebases.

You can check here for more context. I'm not sure if there is a better way to support Webpack 5 and remove that warning, I'd love to hear some ideas and I appreciate your efforts!

kettanaito commented 2 years ago

It'd be great to find a better way, but the native require is stubbed by webpack, causing all sorts of issues when used in our scenario. Escaping that stub produces the warning you're mentioning.

This is not an issue, however, as static dependency analysis is primarily needed for production build optimization.

will-stone commented 2 years ago

Using the solution that is linked to in the comment seems to work:

const nodeVer = typeof process !== "undefined" && process.versions?.node;
    const nodeRequire = nodeVer
      ? typeof __webpack_require__ === "function"
        ? __non_webpack_require__
        : require
      : undefined;
kettanaito commented 2 years ago

That sounds like a great approach, @will-stone. Would you like to open a pull request so we could release it?

will-stone commented 2 years ago

Sure @kettanaito, will do 👍 Thanks.