vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.53k stars 26.82k forks source link

Next App Router (14.2.3) - Inconsistent Singleton #65350

Open vaneenige opened 5 months ago

vaneenige commented 5 months ago

Link to the code that reproduces this issue

https://github.com/vaneenige/next-app-router-singleton

To Reproduce

  1. Install dependencies
  2. Run the build - npm run build
  3. See the log of Create random multiple times

Current vs. Expected behavior

Current behavior:

Expected behavior:

In the reproduction repository you can find a few different test cases:


After quite a bit of research I found multiple sources claiming that this pattern does work with the pages directory, but also multiple sources that can't get it to work with the app directory.


Having a proper singleton could benefit a lot of use cases such as database connections or separate worker scripts.

I've seen the issue being discussed on Reddit and GitHub with regards to MongoDB (or other persistent API connections) too.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:34 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T8103
  Available memory (MB): 16384
  Available CPU cores: 8
Binaries:
  Node: 18.17.1
  npm: 9.6.7
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.2.3 // Latest available version is detected (14.2.3).
  eslint-config-next: 14.2.3
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Module Resolution

Which stage(s) are affected? (Select all that apply)

next build (local)

Additional context

No response

bitttttten commented 5 months ago

Are you sure it's not becaues of the Webpack Build Worker? This can split your build over a few different workers. This means that webpack is run X times over your app and therefore you may see X amount of logs called.

You can opt out of it: https://nextjs.org/docs/messages/webpack-build-worker-opt-out#webpack-build-worker-opt-out although you will most likely see a degrade in build times.

vaneenige commented 5 months ago

I thought this could be it (as I was already thinking it's running separate builds in parallel), but it doesn't seem to change anything. When I add the following it still runs the initialization multiple times (1 time for every ~20 pages):

const nextConfig = {
  experimental: {
    webpackBuildWorker: false
  }
}
hungcung2409 commented 3 months ago

I have the same problem. I tried to use globalThis in dev mode as Prisma's suggestion, but it does not work.

ioulian commented 3 months ago

Same here, doing exactly like this file: https://github.com/vercel/next.js/blob/canary/examples/with-mongodb/lib/mongodb.ts but it's recreated multiple times (twice at the start of the application, 1 other time when calling inside a server action from client, third party component).

This happens only on production build

cyril-marblism commented 3 months ago

Same problem. I have the same observations using both a singleton and globalThis.

Singleton

// test.ts
class Singleton {
  static instance = { value: 0 }
}

export default Singleton.instance

globalThis

// test.ts
if (!globalThis.test) {
  globalThis.test = { value: 0 }
  console.log('INITIALIZED')
} else {
  console.log('ALREADY INITIALIZED')
}

export default globalThis.test

Server

I created a TRPC procedure to expose the value and increment it on each request.

import test from "./test"
...
query(() => {
    if (Utility.isNull(test)) {
      return { test: -1 }
    }

    test.value += 1

    return { test: test.value }
  })
...

Frontend

Add button to trigger query on click.

Display test.value to check that it increments correctly.

Observation

The value is correctly incremented

however

The logs for the globalThis version constantly show INITIALIZED, which means that an instance of test.ts is constantly being created on each request.

Conclusion

This is very confusing, because according to the logs, it's broken. But according to the QA tests, it works.

I'm guessing that even though singletons and using globalThis both work, the NextJS server isn't optimized for it and will still spend time and resources initializing it on every request, which can be a huge problem if some libraries require time-consuming and expensive initialization.

Is there an explanation for this scenario, or is it a black box?

MichaelSitter commented 3 months ago

I have been researching this at some length, coming at this from a slightly different angle but I think the core issue is the same. The reason you see your initialization logic occurring more than once is due to Next internally using two different module systems, CJS for the server context, and webpack for the browser context. You can see in the Next server there are actually a number of different mechanisms for importing modules.

What was useful in my case to understand this was using console.trace to debug the module initialization, because you'll be able to see the initialization context this code is getting run in. Here is what the server CJS imports look like:

[build:node-service]     at Object.<anonymous> (.../packages/server-core/lib/test.js:15:9)
[build:node-service]     at Module._compile (node:internal/modules/cjs/loader:1369:14)
[build:node-service]     at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
[build:node-service]     at Module.load (node:internal/modules/cjs/loader:1206:32)
[build:node-service]     at Module._load (node:internal/modules/cjs/loader:1022:12)
[build:node-service]     at Module.require (node:internal/modules/cjs/loader:1231:19)
[build:node-service]     at require (node:internal/modules/helpers:179:18)
[build:node-service]     at Object.<anonymous> (../server-core/lib/index.js:38:14)
[build:node-service]     at Module._compile (node:internal/modules/cjs/loader:1369:14)
[build:node-service]     at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)

And here is what the RSC imports look like:

[build:node-service]     at eval (webpack-internal:///(rsc)/../../packages/server-core/lib/test.js:18:9)
[build:node-service]     at (rsc)/../../packages/server-core/lib/test.js (../services/nextjs-demo-service/dist/server/app/page.js:602:1)
[build:node-service]     at __webpack_require__ (../services/nextjs-demo-service/dist/server/webpack-runtime.js:33:42)
[build:node-service]     at eval (webpack-internal:///(rsc)/../../packages/server-core/lib/index.js:50:14)
[build:node-service]     at (rsc)/../../packages/server-core/lib/index.js (../services/nextjs-demo-service/dist/server/app/page.js:426:1)
[build:node-service]     at __webpack_require__ (../services/nextjs-demo-service/dist/server/webpack-runtime.js:33:42)
[build:node-service]     at eval (webpack-internal:///(rsc)/./src/app/page.tsx:10:79)
[build:node-service]     at (rsc)/./src/app/page.tsx (../services/nextjs-demo-service/dist/server/app/page.js:651:1)
[build:node-service]     at Function.__webpack_require__ (../services/nextjs-demo-service/dist/server/webpack-runtime.js:33:42)

This is a problem for a lot of other libs which rely on singletons, or otherwise need to use the state of objects which are loaded into the RCS context. In my case this completely breaks prometheus metrics which are used in your component code. But this seems like a pretty fundamentally hard problem to overcome, I'm not sure how you'd avoid re-importing something that needs to exist in multiple module formats.

khuezy commented 2 months ago

@MichaelSitter thanks for digging into that. I've also notice the different module importing issue causing singletons not to work, now I know why. Is this intended or a bug/bad design?

Fwiw, using instrumentation.ts properly runs code there once.

wesbos commented 1 month ago

Also hitting this, I have a socket with a single connection, but it seems a client component that calls a server action and a server component that calls a server action create different bundles in development, thus globalThis is not shared?

samcx commented 1 month ago

Hi everyone!—

I have discussed this with the :nextjs: team and will be sharing some thoughts on this here.

First, during build, by default we use multiple workers. These are distinct child processes so unique copies of node that each load the module system and build some subset of pages. So for each worker spawned during the build you will see a copy of singleton initialization. We don't optimize for singletons because it would force us to use only 1 cpu core (JS is single-threaded) when for many machines more are available and thus builds can be faster if we parallelize.

Second, cjs vs webpack module loading seems like a bug/misconfiguration. In the first trace the module is being loaded by node which means it was at some point treated as an external. The second trace shows the same module being loaded by webpack so it wasn't treated as an external. This might be because the module is being used in RSC and client and we intentionally fork these modules so you can have one implementation for RSC and one for the client. In the future we may actually render the RSC part in it's own process which again would force there to be two different module instances.

The pattern of using a singleton during build or render to have a global view of the state of the server is just something that we are currently not optimizing for. It is maybe possible to mark something as "must be external" which means that it would again be a single instance across RSC and client (on the server).

Let me know if that clarifies!

khuezy commented 1 month ago

Thanks for the info @samcx . The first makes sense and is expected, but the 2nd can be confusing and cause unintended bugs. Please let us know when the "must be external" flag is implemented.

souporserious commented 1 month ago

Thank you for bringing this up with the team, @samcx! I've faced this issue in plugin development where I need one instance of the TypeScript compiler and Shiki syntax highlighter which can be very expensive to run. For now, I've opted to have people run a separate process that I can control better to avoid this since it can cause OOM issues, but breaks down the DX. Since this problem seems very similar to the instrumentation hook, it would be great to expose that in the next.js configuration rather than a file so plugin authors could make use of this as well.

Something like the following in next config would be great if possible:

export default {
  instrumentation: ['init-singleton-here.ts']
}
VanCoding commented 4 days ago

There issues seem to be related: #55885 #49309