next.js: Dynamic "import" creates a race condition with initial load requests

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

This issue is not related to the precise Next version but rather to the with-msw example code.

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000
Binaries:
  Node: 16.13.2
  npm: 8.1.2
  Yarn: 1.22.17
  pnpm: 7.4.0
Relevant packages:
  next: 12.3.4
  eslint-config-next: N/A
  react: 17.0.2
  react-dom: 17.0.2

Which example does this report relate to?

with-msw

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

I believe due to #25607, MSW example has migrated from require() to dynamic imports import() in order to tree-shake MSW from production builds. That, however, introduced a different problem:

  • Since await import() is async, and there’s no top-level await to wait for the initMocks() function, there’s a race condition created between the client-side code and the mocks being imported.

This race condition manifests upon initial page requests as those may be ignored by MSW since the code is not done resolving await import() when those requests happen.

Expected Behavior

  1. The example imports/requires MSW setup synchronously.
  2. Alternatively, the example ensures that the app doesn’t run until MSW is imported. This import is not a costly operation, it’s just a matter of asynchronicity and awaiting it.
  3. Side-effect: MSW must not be included in the production bundle as an indirect result of this fix/change.

To Reproduce

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Reactions: 10
  • Comments: 16 (13 by maintainers)

Most upvoted comments

I raised a different problem with Next 13 beta version here but it was closed down as being the same problem defined here, without any investigation or discussion. MSW works fine with the pages folder but fails with the app folder.

Having integration with a tool like msw is going to be important to demonstrate the new fetching features and closing down an investigation into why msw is not compatible with Next 13 appDir will only delay progress in testing those features.

@balazsorban44 please can you repoen that ticket relating to Next 13 until at least it is established these are the same problem. There is currently no msw+Next13 demo available and that issue could form the basis for it. You have closed down any possibility of that by closing the ticket.

If you cannot guarantee that fixing the issue for Next 12 will fix issues for Next 13 beta specifically the app directory features with server side components then it is quite reasonable to have to open tickets relating to different major versions of Next.

I see.

Do you have any opinion on abstracting this setup someplace else? Are there perhaps other tools that have a complex setup and utilize a different pattern than including it in the template directly?

My main concern here is that this kind of setup is brittle due to its verbosity and internal detail awareness. Despite being a template, this is still the code that people will ship in their apps but they have, effectively, no authorship around this code. As a consequence, if there’s an issue with this setup, it makes it hard to fix for people as they cannot re-apply the template and would have to apply whichever fix is appropriate manually. It really feels like something we should distribute rather than inline.

I was thinking if adding a new entry to the webpack compilation wouldn’t be beneficial here. Something like:

// next.config.js
module.exports = {
  webpack(config) {
    config.entry.push('./msw.setup.js')
    // Use a webpack plugin to create "msw.setup.js" module
    // in the in-memory file system.
    return config
  }
}

Then, on the consumer’s side:

// next.config.js
import { withMSW } from '@mswjs/webpack-plugin'

module.exports = withMSW()

The plugin would ensure the correct module import and initialization, and the with-msw template would still be relevant to show the directory structure and have a quick start.

The plugin is, indeed, limiting in terms of its dependency on the MSW setup structure on the consumer’s side as it needs to know where the handlers.js are, etc. We can parametrize this though. Still a lesser price to pay in terms of maintenance.

It would be great to hear the official Next team stance on the alternatives to the _app.js in terms of logic with their new Next 13 release. As far as I can tell, there are no alternatives, which causes this very problem.

The @philwolstenholme 's stance is correct. At the moment, I don’t see Next.js providing library authors with the alternative to _app.js with the new App router release (Next 13). That is the main reason why MSW example stops working in the next version of Next. I do hope the Next team considers this as there are quite a number of Next+MSW users out there, and it’d be great to have the means to execute browser/server-side code once before the app loads (to defer its rendering as we’re doing with the older versions of Next). This is not something we can do on the MSW side, this must be provided by the framework.

@philwolstenholme, regarding the module preloading, what you’re suggesting will work for Node.js but in Node.js things are already synchronous and working well. It is the browser integration that suffers from this race condition between:

  • The msw module being imported (which is an async import).
  • The service worker registering by MSW;
  • Async requests on initial page load that don’t await the previous two points.

That being said, you can give the preloading a try! If it works, it may be a reasonable workaround until a better solution is found.

I’ve just spotted https://nextjs.org/docs/app/building-your-application/optimizing/instrumentation – does this give a potential entry point for using MSW with the Next app router?

If you export a function named register from a instrumentation.ts (or .js) file in the root directory of your project (or inside the src folder if using one), we will call that function whenever a new Next.js server instance is bootstrapped.

Good to know

  • This feature is experimental. To use it, you must explicitly opt in by defining experimental.instrumentationHook = true; in your next.config.js.
  • The instrumentation file should be in the root of your project and not inside the app or pages directory. If you’re using the src folder, then place the file inside src alongside pages and app.
  • If you use the pagesExtension config option to add a suffix, you will also need to update the instrumentation filename to match.
  • We have created a basic with-opentelemetry example that you can use.

When your register function is deployed, it will be called on each cold boot (but exactly once in each environment).

Sometimes, it may be useful to import a file in your code because of the side effects it will cause. For example, you might import a file that defines a set of global variables, but never explicitly use the imported file in your code. You would still have access to the global variables the package has declared.

I was able to fix the race condition in next v13.1.6 by using dynamic imports in _app.tsx with a top level await:

if(process.env.NEXT_PUBLIC_API_MOCKING === 'enabled') {
  const { initMocks } = await import('path/to/your/mocks');
  await initMocks();
}

In order to do this you will need to update next.config.js and its webpack config so it can support topLevelAwait, see https://github.com/vercel/next.js/discussions/11185

I think the official line is that the MSW maintainer will wait until the app router is stable before investigating this issue (it’s caused by how the Next example is using MSW, not caused by MSW itself), and the Next maintainers will wait a bit longer to update the Next examples directory to use the app router. So, unless the community can figure it out themselves, we will need to wait.

I don’t have enough Node knowledge to know if this will work, but I was wondering about using this trick https://jake.tl/notes/2021-04-04-nextjs-preload-hack#using-preloading to ‘preload’ MSW in dev mode with a require call inside it to avoid the dynamic import, but also not add MSW to my production bundles. I’m not sure when I’ll have a chance to try it, so if anyone else would like to give it a go then please do!

  1. I am thinking that useEffect in general should not be used to trigger data fetching but it might be a different discussion. Loading a Service/Web worker for MSW is not different than any other case from the Next.js perspective, so I do believe it’s the correct solution here. A loading state could probably be added to be able to determine if it’s ready yet. Will ask about useLayoutEffect.
  2. Actually, useEffect never executes server-side and is the safe place to use browser APIs for example.