itaylor / redux-socket.io

An opinionated connector between socket.io and redux
410 stars 53 forks source link

Added way to handle disconnect event. #27

Closed singerxt closed 7 years ago

singerxt commented 7 years ago

What did you implement:

Hello! Thank you for all your hard work. I had problems with handling case when socket is closing in the middle of process. I noticed that when socket disconnecting action is a string but in our case action should be object to fit redux requirement. I didn't find a way to send a message in right format before server crash so I decided to create this small PR.

With this code user is able to define action name what will be send when server crashes. Just add actionName to options.

How did you implement it:

How can we verify it:

var http = require('http'); var server = http.createServer(); var socket_io = require('socket.io'); server.listen(3000); var io = socket_io(); io.attach(server); io.on('connection', function(socket){ console.log("Socket connected: " + socket.id); socket.on('action', (action) => { if(action.type === 'server/hello'){ console.log('Got hello data!', action.data); socket.emit('action', {type:'message', data:'good day!'}); } }); });



* run server notice that message is incoming.
* shut down server
* notice that 'disconnect' action was fired
singerxt commented 7 years ago

Is this repo still in active development @itaylor ? :-)

itaylor commented 7 years ago

Hi @singerxt, thanks for the PR.

Is the project under active development? Not really. The codebase is pretty mature and reasonably widely used in production. It's very narrow in scope, and seems to do the thing it aims to do pretty well. I definitely plan on fixing bugs, keeping it working, and perhaps do some refactoring to eliminate reliance on babel as ES6 features become more mainstream, but I see its feature set as relatively complete, and I don't see a lot of need for a lot of active development.

As for adding the socket disconnect handling... I don't think it belongs in this library.

Many apps don't need it, or need some different version of it. Socket.io's disconnect/reconnect/retry logic is fairly complicated. Just search for 'reconnect' and 'disconnect' on this page https://socket.io/docs/client-api/ and it's immediately clear that there's a lot of nuance and subtlety that you need to understand and set configuration for before trying to "do the right thing" by default for socket disconnect/reconnect.

If we look at the client-side example from the readme.md, it seems like what you're adding here could just be one extra line near the bottom after the store is created. socket.on('disconnect', () => store.dispatch({ type: 'disconnect' }));

import { createStore, applyMiddleware } from 'redux';
import createSocketIoMiddleware from 'redux-socket.io';
import io from 'socket.io-client';
let socket = io('http://localhost:3000');
let socketIoMiddleware = createSocketIoMiddleware(socket, "server/");
function reducer(state = {}, action){
  switch(action.type){
    case 'message':
      return Object.assign({}, {message:action.data});
    case 'disconnect': 
      return Object.assign({}, { isConnected: false });
    default:
      return state;
  }
}
let store = applyMiddleware(socketIoMiddleware)(createStore)(reducer);
socket.on('disconnect', () => store.dispatch({ type: 'disconnect' }));
store.subscribe(()=>{
  console.log('new client state', store.getState());
});
store.dispatch({type:'server/hello', data:'Hello!'});
singerxt commented 7 years ago

That make sense. Thanks let me close PR then :-)