elysiajs / stream

Plugin for Elysia for streaming response and Server Sent Event
MIT License
15 stars 2 forks source link

Should `Stream` be really deprecated? #14

Open wladpaiva opened 2 months ago

wladpaiva commented 2 months ago

Nothing wrong with the generator function but doesn't work on every possible case.

If you are sending data from a event emitter like this you won't be able to use the generator

import { Elysia } from "elysia";
import { Stream } from "@elysiajs/stream";
import { EventEmitter } from "node:events";

/**
 * The event emitter for the whole application.
 */
export let emitter = new EventEmitter<{
  x: [];
}>();

const app = new Elysia()
  .get(
    "/",
    () =>
      new Stream(async (stream) => {
        console.log("Started streaming");

        emitter.on("x", () => {
          stream.send(new Date().toISOString());
        });
      })
  )
  .listen(3000);

setInterval(() => {
  console.info("Emitting transaction status changed");
  emitter.emit("x");
}, 30_000);

console.log(
  `🦊 Elysia is running at http://${app.server?.hostname}:${app.server?.port}`
);
raduconst06 commented 2 months ago

I believe the approach from below should work exactly as you need:

import { Elysia } from "elysia"; import { EventEmitter } from "node:events";

/**

const app = new Elysia() .get( "/", async function* () { console.log("Started streaming");

  try {
    while (true) {
      // Wait for the next "x" event
      await new Promise((resolve) => {
        emitter.once("x", resolve);
      });

      // Yield the data
      yield new Date().toISOString();
    }
  } finally {
    // Cleanup code if necessary
    console.log("Client disconnected, stopping stream.");
  }
}

) .listen(3000);

setInterval(() => { console.info("Emitting transaction status changed"); emitter.emit("x"); }, 3_000);

console.log( 🦊 Elysia is running at http://${app.server?.hostname}:${app.server?.port} );

wladpaiva commented 2 months ago

Screenshot 2024-09-17 at 11 33 54

Yea, not sure what's advantage and still couldn't make the stream to wait for the event.

Maybe is just the use case tho. The generator function seems like a clear winner for streams in general but for SSE, the Stream package seems more robust

guyutongxue commented 2 months ago

The generator handler still does have a critical issue: it doesn't implement text/event-stream protocol, thus yielding object / strings might not be recognizable for SSE frontends. (elysiajs/elysia#741)

A migration guide is use yield `event: message\ndata: ${JSON.stringify(object)}\n\n`; to replace the previous stream.send(object), we should do this until elysiajs/elysia#743 merged