Gregivy / simpleddp

An easy to use DDP client library
MIT License
165 stars 19 forks source link

server.on not working #9

Closed joaopiopedreira closed 5 years ago

joaopiopedreira commented 5 years ago

Hi @Gregivy , great work in this package, thank you.

I have this setup:

// meteor.js
import SimpleDdp from 'simpleddp';
import ws from 'isomorphic-ws';

const {simpleDDPLogin} = require('simpleddp-plugin-login');
const SERVER_URL = 'wss://xxx.sssssss.com/websocket';
const opts = {
  SocketConstructor: ws,
  autoReconnect: true,
  endpoint: SERVER_URL,
  reconnectInterval: 5000,
};

const server = new SimpleDdp(opts,[simpleDDPLogin]);

export default {
  SERVER_URL,
  SERVER_URL_HTTP: SERVER_URL
    .replace('ws://','http://')
    .replace('wss://','https://')
    .replace('/websocket',''),
  server,
};

and invoke the server on the main component like below. Everything works as expected, except the "server.on" events.

import simpleddp from "simpleddp";
import {config} from './meteor'

class App extends Component<IProps,IState> {
  private server: simpleddp;  

  constructor(props) {
    super(props);
    this.server = config.server;
  }

  public async componentDidMount()  {
    this.server.connect().then(() => {
        console.log('this gets logged!');
    }

    this.server.on('connected',()=>{
      console.log('this does not get logged');
    });

  }
  (...)
}

I can't get any of the "server.on" events to work. Am I doing something wrong? Cheers

Gregivy commented 5 years ago

Hello @joaopiopedreira, thank you for your response!

Have you tried something like this?

    setTimeout(() => {
      this.server.connect().then(() => {
          console.log('this gets logged!');
      }
    },1000);

    this.server.on('connected',()=>{
      console.log('this does not get logged');
    });

Will the both messages appear?

joaopiopedreira commented 5 years ago

Hi @Gregivy, thank you for the quick reply. That didn't work, but the below did. I'll fork your project this evening and will try to debug why the core ddp emitter is not working. If I find anything, I'll post it here or send you a PR.

import simpleddp from "simpleddp";
import {config} from './meteor'

class App extends Component<IProps,IState> {
  private server: simpleddp;  

  constructor(props) {
    super(props);
    this.server = config.server;
  }

  public async componentDidMount()  {
    this.server.connect().then(() => {
        console.log('this gets logged!');
        this.server.ddpConnection.emit('connected',{});  // <-------
    }

    this.server.on('connected',()=>{
      console.log('this does not get logged');
    });

  }
  (...)
}

On another note, are you interested in making this package typescript compatible? I'm writing a ...d.ts file as I move along and will soon be able to either submit a PR to DefinitelyTyped or directly to you and you include it in your source, as you prefer.

joaopiopedreira commented 5 years ago

Hi again @Gregivy , maybe you should emit a connected event here, something like

connect () {
        this.autoReconnect = this.autoReconnectUserValue;
        this.socket.open();
        this.emit('connected',{...}) //  <---------
    }
Gregivy commented 5 years ago

Hi @joaopiopedreira, its great idea to support typescript definitions! I can merge your PR if you'd like or you can submit to DefinitelyType as you like more. It's up to you :)

What concerns the error you faced I think it may be due to the fact that connected message comes from server too quick, quicker than you start listening for connected events. Try to switch positions in code:

    this.server.on('connected',()=>{
      console.log('this does not get logged');
    });

    this.server.connect().then(() => {
        console.log('this gets logged!');
    }

If it won't help, I need to see your code example that reproduces this error (test repo suits perfect).

P.S. You can't emit connected after this.socket.open(); because it doesn't match DDP protocol specification (https://github.com/meteor/meteor/blob/devel/packages/ddp/DDP.md).

joaopiopedreira commented 5 years ago

Hi @Gregivy, setting autoConnect:false when instantiating simpleddp did the trick (so you were probably right when you said that the server was responding too fast, because the connection was being establish on class instantiation).

A separate PR will follow with the typescript definition file.