fix: Revert "fix(scheduler): prevent unwanted clearInterval (#3044)"

[kwonoj]

This reverts commit 7d722d4.

Description:
fixing regressions.
Related issue (if exists):

• closes 5.5.3 EmptyError: no elements in sequence #3151

[kwonoj]

@cartant I'm reverting back PR #3044 as it brakes existing behavior with ng zones, possibly others.

[rxjs-bot]

	Messages
📖

CJS: 1383.5KB, global: 752.4KB (gzipped: 120.7KB), min: 145.3KB (gzipped: 31.4KB)

Generated by 🚫 dangerJS

[cartant]

@kwonoj Thanks. I should have some time to look at this on Tuesday. If the change breaks zones, I would expect that it exposes a bug someplace else, as the change simply avoids tearing down and re-creating a setInterval for each emission. Hardly ideal behaviour. I will have a closer look in a few days.

[coveralls]

Coverage decreased (-0.01%) to 97.079% when pulling 64f9285 on kwonoj:fix-interval into fdfeef2 on ReactiveX:master.

[kwonoj]

@cartant I think this is kind of hard thing catch when make changes, especially it is hard to say what is root cause for this. Just sharing as fyi, as we have to take back your contribution.

[cartant]

@kwonoj No worries. When I say I'll have a look at it, I really mean that I'll have a look at Angular.

[cartant]

@kwonoj Apparently, zone.js auto-cancels setInterval calls. See this pending PR: angular/zone.js#935

[jpmckearin]

@cartant in relation to #3152 (comment), I have an Angular project that is producing this error. I can throw it up somewhere for you to investigate if needed.

[cartant]

@jpmckearin Thanks. If it's a small repro, that would be great. If it's a large project that just happens to manifest the error, I'd need detailed, step-by-step instructions on its reproduction.

I'd like to confirm that the zone.js PR I referenced above fixes the issue.

[jpmckearin]

@cartant https://github.com/jpmckearin/ng5-rxjs-issue. See the README for repro steps (super simple, small project)

[Cito]

@cartant see also the Plunk mentioned in Angular issue #20752 for a minimal reproduction of the problem in Angular

[benlesh]

@kwonoj this targeted the wrong branch lol.

[kwonoj]

@benlesh nah, check #3154 cherry picks into stable.

[cartant]

@jpmckearin and @Cito Thanks for the repros. With the merging of angular/zone.js#935, I've finally got around to looking into this.

@kwonoj and @benlesh FYI, the problem with 5.5.3 is unrelated to PR #3044 and it was not the reversion of that PR that solved the problem of Angular's routing module effecting an EmptyError.

Instead, the problem appears to have been the way in which the .js files in 5.5.3 were generated.

I can reproduce the effected error by taking the util/EmptyError.js file from 5.5.3 and overwriting the file in the 5.5.2 distribution.

In fact, if you look at the files that are incorporated into the Plunk, you will see that scheduler/AsyncAction.js does not even form part of the build.

The 5.5.2 EmptyError.js file looks like this:

"use strict";
var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
/**
 * An error thrown when an Observable or a sequence was queried but has no
 * elements.
 *
 * @see {@link first}
 * @see {@link last}
 * @see {@link single}
 *
 * @class EmptyError
 */
var EmptyError = (function (_super) {
    __extends(EmptyError, _super);
    function EmptyError() {
        var err = _super.call(this, 'no elements in sequence');
        this.name = err.name = 'EmptyError';
        this.stack = err.stack;
        this.message = err.message;
    }
    return EmptyError;
}(Error));
exports.EmptyError = EmptyError;--
//# sourceMappingURL=EmptyError.js.map

Whereas the 5.5.3 EmptyError.js file looks like this:

"use strict";
var __extends = (this && this.__extends) || (function () {
    var extendStatics = Object.setPrototypeOf ||
        ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
        function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
    return function (d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();
Object.defineProperty(exports, "__esModule", { value: true });
/**
 * An error thrown when an Observable or a sequence was queried but has no
 * elements.
 *
 * @see {@link first}
 * @see {@link last}
 * @see {@link single}
 *
 * @class EmptyError
 */
var EmptyError = /** @class */ (function (_super) {
    __extends(EmptyError, _super);
    function EmptyError() {
        var _this = this;
        var err = _this = _super.call(this, 'no elements in sequence') || this;
        _this.name = err.name = 'EmptyError';
        _this.stack = err.stack;
        _this.message = err.message;
        return _this;
    }
    return EmptyError;
}(Error));
exports.EmptyError = EmptyError;
//# sourceMappingURL=EmptyError.js.map

The EmptyError implementation in 5.5.3 seems to mess with this instanceof test in Angular's router.

Why the two generated files differ, I have no idea. The EmptyError.js generated for 5.5.4 is identical to that generated for 5.5.2, yet there appear to be no configuration changes between 5.5.3 and 5.5.4, so I can only presume that the build environment for 5.5.3 was not what it should have been.

The Zone.js bug - in the PR that I referenced above - does not effect the EmptyError. The bug is related to my PR, as there was a (far more subtle) problem with Zone.js and setInterval, but that has been fixed with the release of version 0.8.19 of Zone.js.

Given that the reported problem was unrelated to the reverted PR and that the PR fixes a problem with intervals drifting (see this issue and this SO question) I think the commit should be reinstated. If not into stable, then definitely into master.

P.S. @Brooooooklyn I noticed a comment of yours asking why the reversion was made. This comment should explain it.

[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.