lib: add requestAnimationFrame global

[jasnell]

Adds the browser API requestAnimationFrame() as an isomorphic
wrapper around setImmediate(). Marked Legacy with a clear
indication that code targeting Node.js should use setImmediate()
instead.

Signed-off-by: James M Snell jasnell@gmail.com

/cc @mcollina

[devsnek]

f

[mcollina]

lgtm

[timoxley]

This probably needs the corresponding cancelAnimationFrame global (clearImmediate) to make this compatible with code using requestAnimationFrame https://developer.mozilla.org/en-US/docs/Web/API/Window/cancelAnimationFrame

[bengl]

setImmediate is already a misleading name. Providing an alias (or almost alias) that's even more misleading seems like something that should be avoided. The documentation itself in this PR points out that it's misleading.

Also, is there any precedent for introducing an API and immediately marking it Legacy in the same commit/PR?

[tlhunter]

A global requestAnimationFrame function doesn't feel correct to me without the concept of a global renderer. Of course, in a browser window there has always been a global interface, but with Node.js we don't have one.

If a user was building an ncurses-based terminal application, I would expect requestAnimationFrame to dynamically match the speed at which the Node.js program is able to update the output. Maybe with xterm this is 500 times per second, maybe with iTerm2 this is 17 times per second. Likewise, if the program were drawing to an OpenGL window, I would then expect the output to be closer to 144fps on my desktop, 60fps on my laptop, or 30fps on my Pi.

Of course, a Node.js program could output to both a terminal window and an OpenGL window simultaneously. A global requestAnimationFrame makes even less sense in this case where the framerates shouldn't be matched. In such cases it makes much more sense to have something like ncurses.requestAnimationFrame and opengl.requestAnimationFrame, which is very contextual to the renderer and is something better handled by external modules.

[aduh95]

Also, is there any precedent for introducing an API and immediately marking it Legacy in the same commit/PR?

Yes, there is d1e2184.

[mscdex]

I disagree with adding new APIs like this (especially ones that are global) with a stability of legacy.

It seems like it would be better to just have a separate page in our documentation that matches browser APIs like this one (and atob()/btoa()) with their equivalent node API.

[jasnell]

Given the pushback on this, it's unlikely to land, to be honest. But it would be good to get some general feedback from @nodejs/collaborators and @nodejs/tsc before progressing one way or the way on this. Converting this to a draft PR while we sort out the discussion

[guybedford]

Aligning with browsers is always a plus, and unless there are genuine technical concerns with the implementation that can be worked through and discussed, I'm not sure why it should be blocked.

[vdeturckheim]

I am +1 on @guybedford 's argument here

[bmeck]

I could imagine the 60fps limitation and batching of RAF being actually somewhat useful particularly if this did have an embedder API. Do we know of anything except electron in heavy usage of a GUI and embedding Node?

[benjamingr]

We... don't have animation frames.

I get the want to align with browsers but I really think this is very misleading to developers for two reasons:

• The semantics here don't align with what requestAnimationFrame in browsers and users who run universal code are unlikely to benefit from node diverging from what RAF does by providing "an" async callback. This is unlike other web APIs like queueMicrotask, AbortController or atob where Node.js can provide a reasonable implementation (at the very least callbacks should be called together and not interleaved with timers - and likely have some sort of expiry mechanism for when X time is elapsed).
• We don't actually have animation frames or page rendering for that matter. This really confusing to embedders who do (like electron).

[benjamingr]

An easier avenue to tackle might be requestIdleCallback ?

[guybedford]

http://npmjs.org/package/canvas might be an obvious example of something that might benefit from this. Just because Node.js doesn't define a native renderer doesn't mean there might not be native renderers in play. Extending the native Node.js event loop for constructing a draw loop easily seems useful. The requestAnimationFrame specification doesn't reference canvas / drawing apart from in the example - the base semantics are fully abstracted.

requestIdleCallback seems useful as well certainly.

[benjamingr]

@guybedford

http://npmjs.org/package/canvas might be an obvious example of something that might benefit from this. Just because Node.js doesn't define a native renderer doesn't mean there might not be native renderers in play.

That's sort of my point - users using these libraries might use Node.js requestAnimationFrame and not the library's - without providing a hook for libraries into this (or have them define it rather than us) - this is just a footgun for developers.

requestIdleCallback seems useful as well certainly.

Worth mentioning when I mentioned adding these sort of APIs (and things like the web scheduling API) it got mostly a negative response. If the project's position changed - they certainly should be revisited.

[bmeck]

I think if we get feedback on this being a coordination primitive for libraries or embedders to use that might be an easier path towards meaningful discussion here. It might just be we need to setup a hook for those embedders outside of the global. It doesn't seem there is any complaint about this API if we had a draw loop, and certainly their are draw loops in the ecosystem; so, if we add this I think it likely would ease their behaviors but would be curious to get actual feedback from them.

[bmeck]

To be clear, I don't think we should do anything that makes it harder to coordinate a draw loop for Node.js developers. The RAF APIs have a clear semantics that might not be special cased for optimized use cases, but the interoperability with browsers isn't just shipping the API, but having a means for draw loops coordinate (be it from core or implemented in a library).

[guybedford]

When writing a draw routine, the goal of requestAnimationCallback is to saturate the CPU with drawing calls by getting the soonest callback after the current sync code and task queue.

Waiting on the last draw operation to finish is handled within the registered draw function itself, so no hooks back to the context are needed. Because requestAnimationFrame is based on re-registration for each call, you call the next frame draw operation after completing the last. The wait time is all about handling event loop stuff - input events etc.

I don't really think a render embedder hook comes into play here at all.

[jasnell]

Fyi .. I've already started looking at an implementation of requestIdleCallback. It's a bit more complicated because the Node.js event loop does not have the same notion of frame-based idle time.

[GeoffreyBooth]

I'm with @guybedford on this one. If providing this API means that more code that might not have been written for Node becomes able to run in Node without modification, that seems like a big win for users. 👍

[Qard]

It's worth noting that having this in core doesn't stop anyone from implementing their own equivalent. The libraries that are doing that already will likely just continue to do so. All having this in core does is enable code that doesn't currently work simply due to lack of a function to work basically correctly. Maybe the semantics aren't 100% the same because there's no concept of a "draw" in Node.js, but though the name may imply a deep connection to draws it's really more an expression of "run this as soon as you can without interrupting anything else" which basically correlates to setImmediate. As long as we are clearly documenting the caveats, which we are here, I think it's fine to include just for the sake of compatibility, with that express statement that it's not the same and you may want to use your own implementation if you have more specific needs for it. 🤷

[jasnell]

@mscdex:

It seems like it would be better to just have a separate page in our documentation that matches browser APIs like this one (and atob()/btoa()) with their equivalent node API.

FWIW, that runs counter to exactly what adding these kinds of apis is intended to address.

In another comment, electron was mentioned...

In general, it's important to keep in mind that Electron does not just use Node.js, it also embeds chromium and (unless things have changed recently) sets the _noBrowserGlobals flag which would cause the requestAnimationFrame() implementation to be pulled from chromium instead of Node.js. The key there is that the slightly different semantics here should have zero impact on electron environments.

That said, given the lack of a "render" step built in to node.js, it's legitmate to ask if there's any real world case where requestAnimationFrame() is going to be used outside of any use case that involves a rendering step.

@bengl:

For some prior art, Deno...

FWIW, this discussion really has nothing to do with Deno or any decisions they've made, so while it's interesting that Deno chose not to implement this, that's not fairly irrelevant to the decision making here. Questions about whether the semantic timing differences and lack of a renderer are relevant and worth discussion.

[mscdex]

FWIW, that runs counter to exactly what adding these kinds of apis is intended to address.

Adding a brand new API that is basically labeled as "don't use this" from the start is not something we should be doing just to appease Electron (or at all).

[jasnell]

@mscdex :

...we should be doing just to appease Electron...

Again, electron really has nothing to do with this. So long as electron is using _noBrowserGlobals, electron would never use this.

[bengl]

@jasnell

FWIW, this discussion really has nothing to do with Deno or any decisions they've made, so while it's interesting that Deno chose not to implement this, that's not fairly irrelevant to the decision making here.

Fair enough. Marking that one off-topic.

That said, given the lack of a "render" step built in to node.js, it's legitmate to ask if there's any real world case where requestAnimationFrame() is going to be used outside of any use case that involves a rendering step.

This is the crux of it for me. If Node.js does not include any renderer, then the function name makes no inherent sense in Node.js. If I'm not mistaken, every other Web API that's been added to Node.js isn't specific to rendering, or having a DOM, or anything to that effect.

Adding Web APIs for compatibility can certainly be useful and make a lot of sense, but there are already a multitude of ways of scheduling work for later execution in Node.js, many of which already have confusing names, some of which are also shared with the Web Platform. Adding another one with a name that doesn't make sense in the context of Node.js might not be doing newcomers any favours.

[jasnell]

Now that's a strong argument @bengl ;-) ... And really it's the only one I've found really compelling in this discussion. The comments about whether it's marked legacy or not out the gate are largely irrelevant for me, but whether the api makes sense in the Node.js context is another matter.

That said, the raf module does have over 4 million weekly downloads so someone certainly thinks it makes sense to use on the server side.

In either case, I'm personally satisfied with the argument you make about the api likely adding to the confusion of node developers who are unfamiliar with where it came from and what it's for. So given that, I'm inclined to close this. We can revisit again later if a good enough reason emerges.