0
\$\begingroup\$

I want to arrange async server-client communication with websockets and vanilla flux architecture. There is an excellent article about "Async Server-Side Communication with the Flux Architecture". I gonna implement it analogue with SockJS and ES6 promises:

ActionCreator.js - it's the same but moved to ES6

import * as constants from ('../constants/constants'),  
import TodoService = require('../services/todo-service');

class ActionCreator {  

  addTodo(text) {
    var that = this;
    service.addTodo({text: text}).then(function(todo) {
        that.dispatch(constants.ADD_TODO, todo);
    });
  }
}

export default new ActionCreator();

services/todo-service.js - even shorter

import WebSocketWrapper from 'WebSocketWrapper';

class TodoService {

    addTodo(todo) {
        //maybe do something with todos before send them to the server
        return WebSocketWrapper.send(todo);
    }
}

export default new TodoService();

WebSocketWrapper.js

/**
 * Due fact that SockJS provide raw websocket API and do not implement pub/sub, we need to do wrapper, which will corellate request/response
 */

import SockJS from 'sockjs';
import selectn from 'selectn';

var reqId = 1; 
const responseDeffers = {};

class WebSocketWrapper {

    constructor(url) {
        this.ws = new SockJS(url);
        this.ws.onopen = this.onOpen;
        this.ws.onmessage = this.onMessage;
        this.ws.onclose = this.onClose;
        this.ws.onerror = this.onError;
    }, 

    onMessage(message) {
        let requestId = selectn('message.data.request_id');
        if (!requestId) return;
        //resolve or reject the promise which is kept in ActionCreator
        if (message.data.result === 'Ok') resolveRequest(requestId, message);
        else rejectRequest(requestId, message.error);
    },

    /**
     * Return Promise, which will be resolved/rejected on server response with the same requestId
     */
    send(request) {  
        return new Promise(function(resolve, reject) {
            //cache resolve/reject function to be able to invoke themn on server response
            responseDeffers[reqId++] = {
                resolve: res,
                reject: rej
            };
            //send message via websockets
            if (this.ws.readyState == this.ws.OPEN) {
                this.ws.send(JSON.stringify(request));
            } else {
                reject('Websocket connection is closed');
            }
        });
    },

    resolveRequest(requestId, message) {
        let response = JSON.parse(message.data);
        responseDeffers[request_id].resolve(response);
    },

    rejectRequest(requestId, message) {
        responseDeffers[requestId].reject(message);
    }

    onOpen() {},

    oClose() {},

    onError(e) {}
}

export default new WebSocketStore(ENV.props.url);
\$\endgroup\$
6
  • \$\begingroup\$ Please do not update the code in your question after receiving answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ Commented Sep 10, 2015 at 7:28
  • \$\begingroup\$ @Mast ok< I just want to share this reference for further code reviews. I'll accept Joseph's answer \$\endgroup\$ Commented Sep 10, 2015 at 7:30
  • \$\begingroup\$ If there are enough changes made, there's nothing wrong with posting your new code as a separate follow-up question. Adding links in the new and this question to each other would do what you want. Make sure you explain what you changed. \$\endgroup\$ Commented Sep 10, 2015 at 7:32
  • \$\begingroup\$ @Mast got it. Will do this way next time. This time I 'll add a comment above that current code is already changed due Joseph's comments \$\endgroup\$ Commented Sep 10, 2015 at 7:52
  • \$\begingroup\$ Again, do not change your code in the question. Not with a comment, not with anything. Leave the code in the question as-is. \$\endgroup\$ Commented Sep 10, 2015 at 9:30

1 Answer 1

2
\$\begingroup\$

ActionCreator.js

  • Why write a class for when you just export a single instance? When one makes a class, one expects to be able to make multiple instances. This doesn't. You can just roll with object literals. You could even just write functions and just export them.

  • service is undefined. Where is it?

todo-service.js

  • Same as above, why write a class?

WebSocketWrapper.js

  • Same as above, why write a class? Even more interesting is that you have "tracking variables" (reqId and responseDeffers) outside the class.

  • Not sure how you'd benefit with let in you code. You're declaring them at the very top of a function scope. var would have sufficed.

Just because JS has ES6 doesn't mean you have to use classes, let and all the new stuff.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.