Skip to content

feat(interval): option to unref sleep interval #378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2025

Conversation

jeengbe
Copy link
Contributor

@jeengbe jeengbe commented Jan 18, 2025

Normally, this is not a problem, however when using bufferCountOrTime() in unit tests, Jest tends to hang due to the interval used in sleep().

Demo:

test('x', async () => {
  // Use a longer interval than the Jest timeout to demonstrate "Jest did not exit one second after the test run has completed."
  await from([]).pipe(bufferCountOrTime(0, 10000)).forEach(console.log);
});

This PR adds an optional unref: boolean parameters that default to the current behaviour. I think it makes sense to unref only in bufferCountOrTime() and timeout().

There are currently not really tests for any of the affected files. I would defer coverage of more operators for another PR, if that is okay.

@jeengbe
Copy link
Contributor Author

jeengbe commented Jan 19, 2025

The UMD code this gives looks very suspicious, specifically the h.Fa() which should stay h.unref(). (How) is this intended?

function vb(a, b, c) {
    c = void 0 === c ? !1 : c;
    return new Promise(function (e, d) {
        function f() {
            clearTimeout(h);
            d(new R());
        }
        b && b.aborted && d(new R());
        var h = setTimeout(function () {
            if (b && (b.removeEventListener("abort", f), b.aborted)) {
                f();
                return;
            }
            e();
        }, a);
        c && "unref" in h && h.Fa();
        b && b.addEventListener("abort", f, { once: !0 });
    });
}

@trxcllnt
Copy link
Member

id.unref() is getting mangled by closure compiler, change it to id['unref']() and it won't get mangled.

@jeengbe
Copy link
Contributor Author

jeengbe commented Jan 19, 2025

Aha, that's the reason for all the

const { ['signal']: signal, ['thisArg']: thisArg, ['predicate']: predicate } = options

Strange that it does that, but thanks, makes sense. Fixed the PR.

@trxcllnt trxcllnt merged commit 9d45d5b into ReactiveX:master May 5, 2025
jeengbe added a commit to jeengbe/IxJS that referenced this pull request May 11, 2025
feat: add new `bufferCountWithDebounce` instead

chore: add more tests

feat(interval): option to unref sleep interval (ReactiveX#378)

fix: use sleep helper method
@jeengbe jeengbe deleted the je-interval-unref branch May 11, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants