shefalijoshi / telemetry-client Goto Github PK

The Open MCT tutorial server is needed in order to run this application. The existing Open MCT tutorial instructions are aimed at integrating telemetry into Open MCT, and do not address simply running the tutorial server. Would you mind adding some basic instructions for installing and running the Open MCT tutorial server.

In this file there is a mixture of async/await and promise chaining. There are two main things I'd do, only use async/await, and also get rid of explicit return statements for the arrow functions. So I'd write this as:

https://github.com/shefalijoshi/telemetry-client/blob/master/src/apis/data.js

export const get_data = async (points, start, end) => {
  const urls = getUrlsForPoints(HISTORY_API, points, start, end);

  const promises = urls.map(async url => await fetch(url, {method: 'get', mode: 'cors'}).json())
  const dataList = await Promise.all(promises)

  return dataList.reduce((acc, data) => acc.concat(data), [])
}

The above is a little bit terse, but it is basically just fetching and transformation so not too hard to read.

getUrlsForPoints(): Might want to consider encodeURIComomponent. Also can lose the explicit returns on the arrow functions. 4 params is also getting pretty long, might want to consider a named object for input instead.

sortPoints(): I see a comment about performance there. In general it is better to perform sorting on the back end. The sorting can't really be batched because you need the entire list in order to be able to sort it.

addRow(): These dom manipulation methods feel a little out of place with the more pure JS helper methods. I'd recommend probably splitting this file into utils/lib.js and utils.dom.js. Also addRow(), addCell(), removeRow() seem very specific to some particular use case, and likely would be better colocated with the UI components that need them.

The readme file references the command npm start, but a start script has not been defined in package.json. Instead, the correct command appears to be npm run build.

Right now running this client code involves modifying the tutorial telemetry server to add CORS headers. This is time consuming and fiddly for users who are not developers (eg. testers!).

The webpack-dev-server supports reverse proxying. The webpack config in this project could be modified to defined a reverse proxy for the openmct tutorial server running on the same port as the application, which would negate the need to add CORS heads to the tutorial server for this particular application. See the openmct-yamcs project for an example of reverse-proxying with webpack-dev-server.

For a pubsub event handler like this, I'd recommend using dependency injection for the event bus dispatch. So it would look something like:

const connect = (bus) => {
  conn = newWebscoket(REALTIME_API);
  
  conn.onmessage = (evt) => {
    let event = new CustomEvent(RECEIVED_EVENT, { detail: evt.data });
    bus.dispatch(event);
  };
}

That way you don't rely on the global window, and can pass in a mock bus object for testing.

On thing to think about here as well, is what happens if someone calls connect() more than once without closing?