Default parameter for .interval causes hard-to-track runtime error.

[jdetle]

RxJS version:
5.0.0-rc2

Code to reproduce:

const Rx = require('rxjs/Rx');
const jupyter = require('rx-jupyter');

const serverConfig = {
  endpoint: "http://127.0.0.1:8888",
  crossDomain: true,
};

const poll = (obs, interval) => {
  const mappedObs = Rx.Observable.from(obs)
    .catch((err) => {
      if (err.xhr) {
        return Rx.Observable.of(err.xhr);
      }
      throw err;
    })

  return Rx.Observable.merge(
    // Fire off the first API Call
    mappedObs,
    // Poll on an interval
    Rx.Observable.interval(interval)
                 .mergeMap(() => mappedObs)
  )
}
const kernel$ = poll(jupyter.kernels.list(serverConfig));

Expected behavior:
When given no default argument for interval, a sane number is picked as default.

Actual behavior:
When no default is given, the .interval() operator defaults to a waiting time of 0 (probably in an effort to behave like the default javascript interval. However, the case I showed above sends out thousands of AjaxObservables per second and will cause the browser to crash in a few seconds.

Additional information:
I recognize its nice to align with the original design of .interval in javascript, but I think it would be nice to at least log some sort of warning if no interval argument is given.

[jayphelps]

This sounds like a case of "dynamic typing sucks". Just for clarity, .interval() and .interval(undefined)behave the same by design. I'm not sure what we could do here.

We don't currently provide any warnings in any API, I'd prefer we didn't start. I generally take the stance that if it's bad enough to warn on, it should be an error instead, with some rare exceptions. Because otherwise if someone wants to rely on that behavior, they get an annoying warning message they can't get rid of. We'd also need to double check at runtime that console.warn exists, but that's not hard.

I'll let others chime in with their opinions. I totally understand how frustrating JavaScript can be sometimes when its dynamic nature leads to strange behavior.

I think if anything, I'd be ok with considering not allowing interval without a provided duration, so .interval() and .interval(undefined) would error. Others may have stronger opinions.

[jdetle]

Thats a very good point regarding errors versus warnings. I'm all for your route of erroring on undefined, I was just thinking there'd be someone who didn't like the divergence from how .setInterval behaves.

[jayphelps]

@jdetle yeah, right now I feel it should be left as-is, but open to the discussion.

[kwonoj]

I'll use #2153 as umbrella issue to discuss runtime validations.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.