Skip to content
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

Module imported in different files re-evaluated #49309

Open
1 task done
jesinmat opened this issue May 5, 2023 · 18 comments
Open
1 task done

Module imported in different files re-evaluated #49309

jesinmat opened this issue May 5, 2023 · 18 comments
Labels
bug Issue was opened via the bug report template.

Comments

@jesinmat
Copy link

jesinmat commented May 5, 2023

Verify canary release

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

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Wed Mar 2 00:30:59 UTC 2022
    Binaries:
      Node: 17.9.0
      npm: 8.19.3
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 13.4.1-canary.2
      eslint-config-next: 13.4.1
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://github.com/jesinmat/next-reimport

To Reproduce

Build and run the project. Open the app in your browser. You should see the following:

Current ID: 1
<Change ID>

In the server log, you will see that a cache has been created (by importing lib/cache.ts):

Creating a cache
Loading data from database
Returning data 1

Now, click on the "Change ID" button. You should see the following in the server log:

Creating a cache
API request received
Setting data to 2

Now refresh the page. Client will still see 1 as the ID as there are now two caches, since it was imported twice and evaluated upon each import. This should not happen.

Describe the Bug

According to ECMAScript specification, importing a module from different places should only evaluate it once.
I wanted to use this fact to create a "cache", but it seems like the imported file is re-evaluated during imports from different files,
which makes it impossible to use as a singleton.

The file in question is located at lib/cache.ts and it's imported in app/page.tsx and pages/api/change/index.ts. Importing from these two files causes Cache constructor to be called twice, effectively exporting two completely separate instances of the class.

Expected Behavior

File lib/cache.ts is only evaluated once and the result is exported and shared between all places.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

@jesinmat jesinmat added the bug Issue was opened via the bug report template. label May 5, 2023
@niemal
Copy link

niemal commented Jun 2, 2023

Exactly what I am losing my hair on. I tried attaching my instance singleton on the global variable, didn't do it. @jesinmat did you find a work-around or did you just abandon?

@jesinmat
Copy link
Author

jesinmat commented Jun 2, 2023

I did not manage to find a work-around, this is an existing issue that's still bothering me.

@niemal
Copy link

niemal commented Jun 2, 2023

I did not manage to find a work-around, this is an existing issue that's still bothering me.

This is how I am exporting my instance if you are interested in trying:

const globalRoomsHandler = global as typeof global & {
  roomsHandler?: RoomsHandler;
};

if (!globalRoomsHandler.roomsHandler) {
  globalRoomsHandler.roomsHandler = new RoomsHandler();
}

export const roomsHandler = globalRoomsHandler.roomsHandler;

@CarlosBalladares
Copy link

im having the same issue on 13.4.9. The workaround doesn't seem to work

@CarlosBalladares
Copy link

CarlosBalladares commented Jul 12, 2023

Actually it doesn't work when running dev, however when you build and start it it doesnt get reevaluated (with the workaround)

@MangoMarcus
Copy link

This issue is getting on for a year old, is there any update for this?

I've noticed modules being executed multiple times both during dev and during the build step.

Eg. if I have a module /src/lib/module.ts which includes a console.log() at the module root (not in an export), and that module is imported in many places throughout my app, the log prints multiple times. Not as many times as the module is imported, but still a handful.

I'd expect it to only print once during the build step, unless I'm missing something?

It's breaking my caching solution in the app router at the moment

@etrepum
Copy link

etrepum commented Apr 25, 2024

This can also break packages such as yjs and lexical which expect their modules to be evaluated at most once and rely on instanceof checks

bsorrentino added a commit to visual-agent-studio/event-broker that referenced this issue May 9, 2024
Module imported in different files re-evaluated: vercel/next.js#49309 (comment)
@saisantosh

This comment has been minimized.

@dedesite
Copy link

dedesite commented May 28, 2024

So I'm not crazy, module are re-evaluated across import !
I spend hours trying to implement a singleton mecanism with Next.js without success (at least in dev mode) !
I want to store in-memory data related to websockets connections and be able to share them across modules and to want to setup something like redis for that...

EDIT : using global variable like done in the above workaround did the trick for me. Thanks @bsorrentino

@DanqingLuan
Copy link

It seems that I am not the only one who has encountered such a problem. I created and exported a Session class to save global Session information and perform related operations on it through a Server action class. However, I found that when using Server action in different Pages, two different copies of the Session class were created. This is crazy. FUCK Next.js!

@nigelwtf
Copy link

It seems wild to me that Next.js can silently modify behaviour defined in TC39 and ignore an issue the community has raised for over a year (in-fact it's worse—the organisation I work for is a paying Vercel customer, so we're essentially paying for broken or outright bad software).

The fact that so much of React's future has been entrusted to Vercel does not bode well for anybody.

@BitOfaLegend
Copy link

I am running into this problem as well on the current latest release 14.2.5

@VanCoding
Copy link

VanCoding commented Oct 3, 2024

Oh my god, I've spent hours trying to figure out why a variable isn't properly shared between API routes until I found out they don't even share the same module, but instead two separate copies. This is bad. Does this mean we need a separate database like redis running alongside the next process to share even the tinyest amounts of data between requests?

The Issue still exists on Next 15 btw..

@aq1018
Copy link

aq1018 commented Oct 22, 2024

How does NextJs, supposedly the latest and the greatest web framework is letting this going on for a year and half without even a work around. This is insane.

@sagan
Copy link

sagan commented Nov 13, 2024

I think this is a intentional design, to make the nextjs app behavior the same way between a node.js server and a "serverless" runtime (aka. Vercel).

In nextjs app, each page (page/**/*.tsx , page/api/*.tsx, app/**/page.tsx) will be compiled to separated endpoint, independent of each other. You can confirm it by checking the log of npm build.

This makes nextjs app suitable for running in a serverless runtime. If you deploy nextjs app to vercel, each endpoint will be deployed as a vercel function. Functions are runned (spawned) on-demand and multiple instances of one function may be running at the same time. Function instances do not share memory between each other.

So in the nextjs way, the recommended practice is do not use any "global state" variable. All state which has a lifecycle larger than a single http request should be put in external source (e.g. Redis or database).

@VanCoding
Copy link

@sagan I think it's clear that trying to share memory between functions in multi-instance deployments is not a good idea. And serverless setups are typically multi-instance deployments.

But there are also a lot of single-instance deployments where it's just completely overkill use redis. I think it's important to support this use-case as well. If multiple instances become necessary, state can always be moved out of memory and into redis. But it shouldn't be required to do this from the beginning even for the simplest apps.

Also, for local development there are other use-cases as well, we, for example, locally expose a "testing"-api that allows integeration tests to bring the system in a certain state, or mock http requests, before executing the tests. But if memory is not shared, that's surely not going to work, because the whole test-setup get's discared again for each request.

@etrepum
Copy link

etrepum commented Nov 13, 2024

It's also simply not implemented correctly if the goal is to isolate requests in that way. If they did it right, this side-effect would not be observable.

@canastro
Copy link

canastro commented Jan 6, 2025

I have this in a /lib/runtime.ts file:

const globalForRuntime = global as unknown as { runtime: AppRuntime | undefined };

export const appRuntime =
	globalForRuntime.runtime || createAppRuntime();

if (process.env.NODE_ENV !== "production") globalForRuntime.runtime = appRuntime;

I see this createAppRuntime called a few times in production... and I assume this is one of the reasons why I have a too many clients connected issues with my postgres database.

In my dev environment I see this function called multiple times, and when I log global it gets constantly set to undefined!

How do others usually handle this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests