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

eventStream cleanup function is not being called in vite dev mode #317

Closed
unbiased-dev opened this issue Mar 2, 2024 · 11 comments
Closed

Comments

@unbiased-dev
Copy link

Describe the bug

When running vite:dev the eventStream return value which is the cleanup callback is not being triggered, probably because of HMR?..

It works fine as soon as you build and run your app that way.

Your Example Website or App

https://stackblitz.com/edit/remix-run-remix-fd7jtu?file=app%2Froutes%2F_index.tsx

Steps to Reproduce the Bug or Issue

Go to https://stackblitz.com/edit/remix-run-remix-fd7jtu?file=app%2Froutes%2F_index.tsx
Refresh the inpage web browser.
Note the user count going up instead of staying at 1.
Check terminal.
Note 'add called' being logged but not 'remove called'.

Expected behavior

Cleanup function to work in dev mode aswell, as it's kinda important.

Screenshots or Videos

No response

Platform

  • OS: wsl2
  • Browser: chrome
  • Version: pretty sure it doesnt matter

Additional context

No response

@philhradecs
Copy link

philhradecs commented Mar 2, 2024

Thanks for creating the issue, I see the same problem after migrating to Vite.
My setup involves an event queue and since the cleanup is never called, it causes my event listeners to pile up.

Remix: v2.8.0
Utils: v7.5.0
Vite: v5.1.4

@matt-aitken
Copy link

We're also experiencing this issue during development.

We are running Remix 2.1.0 not with Vite. We're using Express with a custom server with manual mode.

"dev": "cross-env PORT=3030 remix dev -c \"node ./build/server.js\"",

Our server file: https://github.com/triggerdotdev/trigger.dev/blob/main/apps/webapp/server.ts

@vidjuheffex
Copy link

Also experiencing this issue

@sergiodxa
Copy link
Owner

sergiodxa commented Jun 26, 2024

This is a problem with Vite dev server, it's not aborting the AbortSignal attached to the request so your event stream cleanup function will never run.

It has to be fixed on their side or you have to use a custom dev server that does it, the eventStream function can only rely on request.signal to be aborted

@vidjuheffex
Copy link

vidjuheffex commented Jun 26, 2024

I got it!

import { EventEmitter } from "events";

let emitter;

if (process.env.NODE_ENV === "production") {
  emitter = new EventEmitter();
  emitter.setMaxListeners(100);
} else {
  if (!global.__emitter) {
    global.__emitter = new EventEmitter();
    global.__emitter.setMaxListeners(100);
  }
  emitter = global.__emitter;
}

export { emitter };

exporting my emitter like this did the trick! (this is from the jokes example app)

@cjoecker
Copy link
Contributor

cjoecker commented Aug 9, 2024

@sergiodxa is there a way to recognize that the route using the event stream is still active? I want to build a workaround to kill the interval when it is not used.

@pawelblaszczyk5
Copy link

I think this one will fix that:

remix-run/remix#9976

@sergiodxa
Copy link
Owner

The fix was released on the latest version of Remix.

@jvaill
Copy link

jvaill commented Sep 23, 2024

This still doesn't seem fully fixed.

The cleanup function gets called most of the time now, but not always after a few messages are sent and the request is closed.

@sergiodxa
Copy link
Owner

@jvaill even if it still happens it's not something that can be fixed on Remix Utils' side, it has to be fixed by the HTTP server as eventStream only receives the AbortSignal instance and listen to the abort event.

@jvaill
Copy link

jvaill commented Sep 23, 2024

@jvaill even if it still happens it's not something that can be fixed on Remix Utils' side, it has to be fixed by the HTTP server as eventStream only receives the AbortSignal instance and listen to the abort event.

Makes sense, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants