socketio / socket.io

Realtime application framework (Node.JS server)
https://socket.io
MIT License
60.97k stars 10.1k forks source link

Minor: Event handler of 'offline' event must be added on the initial evaluation of worker script on Service Worker #5125

Open wanfahmi11 opened 7 months ago

wanfahmi11 commented 7 months ago

Describe the bug Getting a warning when including socket.io client to service worker:

image

Event handler of 'offline' event must be added on the initial evaluation of worker script on Service Worker

To Reproduce

Socket.IO server version: 4.7.4

Server

import { Server } from "socket.io";

const io = new Server(3000, {});

io.on("connection", (socket) => {
  console.log(`connect ${socket.id}`);

  socket.on("disconnect", () => {
    console.log(`disconnect ${socket.id}`);
  });
});

Socket.IO client version: 4.7.4

Client

import { io } from "socket.io-client";

const socket = io("ws://localhost:3000/", {});

socket.on("connect", () => {
  console.log(`connect ${socket.id}`);
});

socket.on("disconnect", () => {
  console.log("disconnect");
});

Expected behavior Probably can skip adding "offline" event listener for service worker, since it's already skipped it for offline, I don't see any harm skipping it for service worker.

Platform:

Additional context I would like to contribute myself but I can't find the exact line to modify. I can find the codes in dist/socket.io.js but I'm not sure if that's the right file.

darrachequesne commented 7 months ago

Hi!

The "offline" event listener is added here: https://github.com/socketio/engine.io-client/blob/fa479164251dd1f283dd163143b546582161339a/lib/socket.ts#L397

FMaz008 commented 2 weeks ago

Is there a milestone for this fix? I just installed socket.io (current version being 4.7.5) and I'm encountering this very same warning.

darrachequesne commented 1 week ago

@FMaz008 out of curiosity, could you please explain your use case? Why use a ServiceWorker and not a SharedWorker?

FMaz008 commented 1 week ago

@FMaz008 out of curiosity, could you please explain your use case? Why use a ServiceWorker and not a SharedWorker?

Honestly everything works fine with the Service Worker coupled with an Alarm. That and I had never heard of a Shared Worker before today. So it seems I'm just outdated a bit on my knowledge here.

(Not to side track the conversation, but I tried implementing a SharedWorker, but I keep getting a security error. But even assuming I could sort out this problem, it would be a major refactor of the existing code if it worked.)