[ 
https://issues.apache.org/jira/browse/TINKERPOP-2143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129503#comment-17129503
 ] 

ASF GitHub Bot commented on TINKERPOP-2143:
-------------------------------------------

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:
[email protected]


> JavaScript GLV: Support browsers
> --------------------------------
>
>                 Key: TINKERPOP-2143
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2143
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: javascript
>            Reporter: Jorge Bay
>            Priority: Minor
>         Attachments: query-editor.jpg
>
>
> Currently the JavaScript GLV is designed to work on Node.js runtime.
> We could consider supporting browser runtimes.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to