Skip to content
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

Status of timers enroll/unenroll/active #896

Closed
piscisaureus opened this issue Feb 19, 2015 · 11 comments
Closed

Status of timers enroll/unenroll/active #896

piscisaureus opened this issue Feb 19, 2015 · 11 comments
Assignees
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@piscisaureus
Copy link
Contributor

The timers module exposes some functions (enroll, unenroll, active) that allow for the implementation of efficient idle timers and are used for this purpose by the builtin http module.

I'd like to make some changes to these APIs in a semver-minor patch, but the status of these APIs is currently unclear.
These functions aren't documented and their use outside of core seems very limited.
My proposal is to define them as being private and renaming them to '_enroll', '_unenroll' etc. as part of the next change.

Comments? @iojs/tc

@trevnorris
Copy link
Contributor

If it's in heavy use by user-land then I'm not for changing them. Otherwise +1 on the idea.

@mikeal
Copy link
Contributor

mikeal commented Feb 19, 2015

I've been working on a tool that could tell us how many modules in npm use a particular core module, either themselves or through one of their dependencies. It sounds like it would be useful here :)

@bnoordhuis
Copy link
Member

FWIW, a quick if non-exhaustive GH code search doesn't turn up any projects that use enroll/unenroll.

@mikeal
Copy link
Contributor

mikeal commented Feb 19, 2015

wow... if GH code search finds nothing then I'm pretty confident nobody is actually using this :)

@cjihrig
Copy link
Contributor

cjihrig commented Feb 19, 2015

I'm also +1 if it's not in use. I'd be surprised if it was. I recently updated enroll() and it was only used like twice in core IIRC.

@mikeal
Copy link
Contributor

mikeal commented Feb 19, 2015

I was able to find a few, looks like mostly people using or building polyfills for browserify though https://github.com/search?l=javascript&q=timers.enroll&ref=searchresults&type=Code&utf8=%E2%9C%93

@brendanashworth brendanashworth added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Feb 22, 2015
@Fishrock123
Copy link
Contributor

@chrisdickinson maybe you could run this against your packages thing?

@Fishrock123
Copy link
Contributor

@piscisaureus what changes were you planning to do to them?

@Fishrock123
Copy link
Contributor

See: http://logs.libuv.org/io.js/2015-06-24#23:15:56.833

Bert's request was (basically) to make them always act like _unrefActive.

@Trott
Copy link
Member

Trott commented Mar 11, 2016

@Fishrock123 Is the plan to first deprecate active(), enroll(), and unenroll() and then make them private at some semver-major point in time? Or something else?

Would it be useful to see if @ChALkeR can pull up some rough usage stats for userland module?

@Fishrock123
Copy link
Contributor

@Trott I think we should probably actually keep them, undocumented.

They are non-problematic and unlikely to ever require significant changes imo.

Closing this, re-open if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

No branches or pull requests

8 participants