feat(stream): add browser support

[SaladFork]

Resolves #55

• Add isomorphic-ws
• Fix browser-exclusive issues
  • [x] Fix issue with Axios
  • Fix issue with Timeout being Node-only
  • Fix issue with setImmediate being Node-only
  • Fix race condition with Queue
• Pass lint checks
• Confirm working in Node.js and browser

Was hoping to use this in the browser after really appreciating it in Node.js. I've added the latest version of isomorphic-ws as suggested in #55.

This doesn't seem to quite work yet (or might be a quirk of my setup), as I now get an error around how Axios is being included/used in the CensusClient's RestClient's constructor. It seems like the default import ends up being a string of the path to the cjs import, rather than an import matching the AxiosStatic interface.

Error

Uncaught TypeError: axios_1.default.create is not a function
    at new RestClient (rest.client.js:30:1)
    at new CensusClient (census.client.js:28:1)

[SaladFork]

Okay, that did end up being a quirk of my setup (Create-React-App not properly handling cjs-type imports). I've addressed it but am now failing on unref being undefined on the return value of a setTimeout, which feels less likely to be unique to me:

Uncaught TypeError: this.timeout.unref is not a function
    at new DecayingSet (decaying-set.js:17:1)
    at new DuplicateFilter (duplicate-filter.js:14:1)
    at new StreamManager (stream.manager.js:32:1)
    at new CensusClient (census.client.js:30:1)

ps2census/src/client/utils/decaying-set.ts

Line 22 in 465b4e6

this.timeout.unref();

Looks like Node.js's setInterval returns a Timeout while in the browser we get back a number.

[SaladFork]

Assuming unref usage is resolved, there's still an issue of setImmediate not being available in browsers. It's currently used 4 times in StreamHandler and once in StreamClient.

[microwavekonijn]

Nice job so far.

You can use an intersection type for Timeout, something like Timeout | number. When it comes to unref it is there to not keep the process alive if that is the only thing left. I would add something like this.timeout.unref && this.timeout.unref() to mitigate this.

I have actually swapped setImmediate for process.nextTick as that has the better behavior. You might want to add a utility function to utils called nextTick and use this snippet:

function nextTick(callback: Function, ...args: any[]): void {
  if (process && process.nextTick)
    process.nextTick(callback, ...args);
  else if (setImmediate)
    setImmediate(callback, ...args);
  else
    setTimeout(callback, 0, ...args);
}

setImmediate should work for some browsers, I think Firefox is one that it might not work with.

[microwavekonijn]

Ignore that run-test pipe, it is missing a service id because it is outside of the repo.

[SaladFork]

Thanks!

setImmediate should work for some browsers, I think Firefox is one that it might not work with.

Less support than you might expect 😞. This is one of the rare times I've seen IE be the early adopter.

You might want to add a utility function to utils called nextTick and use this snippet:

I got a TypeScript error with the proposed so had to tweak it a bit:

src/client/utils/next-tick.ts:7:18 - error TS2345: Argument of type 'Function' is not assignable to parameter of type '(...args: any[]) => void'.
  Type 'Function' provides no match for the signature '(...args: any[]): void'.

7     setImmediate(callback, ...args);
                   ~~~~~~~~

Seeing a new issue now, looks like a race condition (maybe as a result of setTimeout(…, 0)). I'm getting:

Uncaught Error: Queue is empty
    at Queue.dequeue (queue.js:13:1)
    at StreamClient.<anonymous> (command.handler.js:30:1)
    at StreamClient.emit (index.js:202:1)
    at stream.client.js:152:1

at

ps2census/src/client/utils/queue.ts

Line 11 in 465b4e6

if (!item) throw new Error('Queue is empty');

[microwavekonijn]

Websocket in browsers do not support the callback that is used in send on StreamClient, which is what break the code. Probably what can be done is just to make the send function synchronous, and then emit an error for ws if any occurs.

StreamClient

send(data: CensusCommand): void {
  if (!this.connection)
    throw new Error(`Connection not available`);

  this.connection.send(JSON.stringify(data), err => {
    if (err) this.emit('error', err);
  });
}

CommandHandler

  private sendCommand<T>(
    queue: Queue<CommandCallback<T>>,
    command: CensusCommand,
  ): Promise<T> {
    if (!this.stream.isReady)
      return Promise.reject(new StreamResponseException('Stream is closed'));

    return new Promise<T>((resolve, reject) => {
      this.stream.send(command);

      queue.enqueue({
        resolve,
        reject,
      });
    });
  }

[SaladFork]

Thanks again.

With the latest push -- this library is now working for me in both the browser and node without errors! 🎉 This includes calling destroy(). This does not include changes still in discussion above.

[SaladFork]

Probably want to squash (up to you) but I think this is ready!

[microwavekonijn]

Nice work. There are some small things I need to do, which will probably be later today. Once that is done I will create a new release.