You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:13:25 UTC

[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1291: TINKERPOP-2143 JavaScript GLV: Support browsers

jorgebay commented on a change in pull request #1291:
URL: https://github.com/apache/tinkerpop/pull/1291#discussion_r437294032



##########
File path: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##########
@@ -119,55 +120,56 @@ class Connection extends EventEmitter {
 
     this.emit('log', `ws open`);
 
-    this._ws = new WebSocket(this.url, {
-      headers: this.options.headers,
-      ca: this.options.ca,
-      cert: this.options.cert,
-      pfx: this.options.pfx,
-      rejectUnauthorized: this.options.rejectUnauthorized
-    });
+    this._ws = new WebSocket(this.url);
 
-    this._ws.on('message', (data) => this._handleMessage(data));
-    this._ws.on('error', (err) => this._handleError(err));
-    this._ws.on('close', (code, message) => this._handleClose(code, message));
+    this._ws.onmessage = (data => this._handleMessage(data));
+    this._ws.onerror = (err => this._handleError(err));
+    this._ws.onclose = (code, message) => this._handleClose(code, message);
 
-    this._ws.on('pong', () => {
+    this._ws.pong = (() => {
       this.emit('log', 'ws pong received');
-      if (this._pongTimeout) {
-        clearTimeout(this._pongTimeout);
-        this._pongTimeout = null;
-      }
-    });
-    this._ws.on('ping', () => {
+    if (this._pongTimeout) {
+      clearTimeout(this._pongTimeout);
+      this._pongTimeout = null;
+    }
+  });
+    this._ws.ping = (() => {
       this.emit('log', 'ws ping received');
-      this._ws.pong();
-    });
+    this._ws.pong();
+  });
 
     return this._openPromise = new Promise((resolve, reject) => {
-      this._ws.on('open', () => {
-        this.isOpen = true;
-        if (this._pingEnabled) {
-          this._pingHeartbeat();
-        }
-        resolve();
-      });
-    });
+      this._ws.onopen = (() => {
+      this.isOpen = true;
+    if (this._pingEnabled) {
+      this._pingHeartbeat();
+    }
+    resolve();
+  });
+  });
   }
 
   /** @override */
   submit(bytecode, op, args, requestId, processor) {
     return this.open().then(() => new Promise((resolve, reject) => {
       if (requestId === null || requestId === undefined) {
-        requestId = utils.getUuid();
-        this._responseHandlers[requestId] = {
-          callback: (err, result) => err ? reject(err) : resolve(result),
+      requestId = utils.getUuid();
+      this._responseHandlers[requestId] = {
+        callback: (err, result) => err ? reject(err) : resolve(result),
           result: null
-        };
-      }
+    };
+    }
 
-      const message = Buffer.from(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor)));
-      this._ws.send(message);
-    }));
+    //const message = Buffer.from(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor)));

Review comment:
       leftover.

##########
File path: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##########
@@ -119,55 +120,56 @@ class Connection extends EventEmitter {
 
     this.emit('log', `ws open`);
 
-    this._ws = new WebSocket(this.url, {
-      headers: this.options.headers,
-      ca: this.options.ca,
-      cert: this.options.cert,
-      pfx: this.options.pfx,
-      rejectUnauthorized: this.options.rejectUnauthorized
-    });
+    this._ws = new WebSocket(this.url);
 
-    this._ws.on('message', (data) => this._handleMessage(data));
-    this._ws.on('error', (err) => this._handleError(err));
-    this._ws.on('close', (code, message) => this._handleClose(code, message));
+    this._ws.onmessage = (data => this._handleMessage(data));
+    this._ws.onerror = (err => this._handleError(err));
+    this._ws.onclose = (code, message) => this._handleClose(code, message);
 
-    this._ws.on('pong', () => {
+    this._ws.pong = (() => {
       this.emit('log', 'ws pong received');
-      if (this._pongTimeout) {
-        clearTimeout(this._pongTimeout);
-        this._pongTimeout = null;
-      }
-    });
-    this._ws.on('ping', () => {
+    if (this._pongTimeout) {
+      clearTimeout(this._pongTimeout);
+      this._pongTimeout = null;
+    }
+  });
+    this._ws.ping = (() => {
       this.emit('log', 'ws ping received');
-      this._ws.pong();
-    });
+    this._ws.pong();
+  });
 
     return this._openPromise = new Promise((resolve, reject) => {
-      this._ws.on('open', () => {
-        this.isOpen = true;
-        if (this._pingEnabled) {
-          this._pingHeartbeat();
-        }
-        resolve();
-      });
-    });
+      this._ws.onopen = (() => {
+      this.isOpen = true;
+    if (this._pingEnabled) {
+      this._pingHeartbeat();
+    }
+    resolve();
+  });
+  });
   }
 
   /** @override */
   submit(bytecode, op, args, requestId, processor) {
     return this.open().then(() => new Promise((resolve, reject) => {
       if (requestId === null || requestId === undefined) {
-        requestId = utils.getUuid();
-        this._responseHandlers[requestId] = {
-          callback: (err, result) => err ? reject(err) : resolve(result),
+      requestId = utils.getUuid();
+      this._responseHandlers[requestId] = {
+        callback: (err, result) => err ? reject(err) : resolve(result),
           result: null
-        };
-      }
+    };
+    }
 
-      const message = Buffer.from(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor)));
-      this._ws.send(message);
-    }));
+    //const message = Buffer.from(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor)));
+     const message = this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor));
+      var buf = new ArrayBuffer(message.length); // 2 bytes for each char
+      var bufView = new Uint8Array(buf);
+      for (var i=0, strLen=message.length; i < strLen; i++) {

Review comment:
       Probably this needs a `TextEncoder` or something like that for fast access: https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder
   
   There are buffer libraries that use built-in `Buffer` in Node.js and polyfills for browsers, we should use one of those.

##########
File path: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##########
@@ -23,7 +23,8 @@
 'use strict';
 
 const EventEmitter = require('events');
-const WebSocket = require('ws');
+//const WebSocket = require('ws');

Review comment:
       This import should still be included.

##########
File path: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js
##########
@@ -119,55 +120,56 @@ class Connection extends EventEmitter {
 
     this.emit('log', `ws open`);
 
-    this._ws = new WebSocket(this.url, {
-      headers: this.options.headers,
-      ca: this.options.ca,
-      cert: this.options.cert,
-      pfx: this.options.pfx,
-      rejectUnauthorized: this.options.rejectUnauthorized
-    });
+    this._ws = new WebSocket(this.url);
 
-    this._ws.on('message', (data) => this._handleMessage(data));
-    this._ws.on('error', (err) => this._handleError(err));
-    this._ws.on('close', (code, message) => this._handleClose(code, message));
+    this._ws.onmessage = (data => this._handleMessage(data));
+    this._ws.onerror = (err => this._handleError(err));
+    this._ws.onclose = (code, message) => this._handleClose(code, message);
 
-    this._ws.on('pong', () => {
+    this._ws.pong = (() => {
       this.emit('log', 'ws pong received');
-      if (this._pongTimeout) {
-        clearTimeout(this._pongTimeout);
-        this._pongTimeout = null;
-      }
-    });
-    this._ws.on('ping', () => {
+    if (this._pongTimeout) {
+      clearTimeout(this._pongTimeout);
+      this._pongTimeout = null;
+    }
+  });
+    this._ws.ping = (() => {
       this.emit('log', 'ws ping received');
-      this._ws.pong();
-    });
+    this._ws.pong();
+  });
 
     return this._openPromise = new Promise((resolve, reject) => {
-      this._ws.on('open', () => {
-        this.isOpen = true;
-        if (this._pingEnabled) {
-          this._pingHeartbeat();
-        }
-        resolve();
-      });
-    });
+      this._ws.onopen = (() => {
+      this.isOpen = true;
+    if (this._pingEnabled) {
+      this._pingHeartbeat();
+    }
+    resolve();
+  });
+  });

Review comment:
       Indentation is wrong.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org