[RAF] Remove FPS limiting to consider other possible features

[titoBouzout]

Off the top of my head, I could think that what almost any fancy requestAnimationFrame need is:

• targetFPS, run at most N FPS
• tell at how many FPS we're running, to provide feedback, maybe only for development, but nice to have
• tell the delta value, how much time elapsed since the last run

What most people need is none of the above, but that's not the point. Others would find these values to be passed down useful.

I'm proposing to start over, because this is a fresh and breaking change, and having people to use it, will make them to then have to change the way it's used in the future, and nobody likes that. The proposal made in #78 considered the above, but what got merged somehow is not considering it.

The targetFPS callback that got implemented was very clever, but how do we add additional features that won't get repeated around and won't require changing signatures.

[tomByrer]

Note: rAF is often used for animation. Perfectly fine to animate 12 or 24 FPS ("Into the Spiderverse" was mostly 12 FPS, sometimes 8FPS). I've done testing on smaller things like switches & found 15FPS worked nicely.

Lowering FPS saves on power, & can help free up CPU/GPU for other more important tasks.

[thetarnav]

I get what you're saying, there are probably plenty of other features the raf package could be providing. The delta time would definitely be helpful. But I don't get how removing the targetFPS primitive right now would move us towards that goal.
We can use this thread to figure out an API that would cover all this things AND allow users to compose their own solutions. Or create a new issue/discussion dedicated to that.
I like the cleanness of current API, it is composable, users can write their own targetFPS functions if they have a slightly different needs and just swap one for another.
It would also allow to add the delta time in the same way:

createRAF(captureDelta(delta => {...}))

If you're interesting in rewriting the API though, we could have a raf-next branch where the new design could take form. But on the main I would stick to existing one for now, and maybe add more helpers if necessary.

[titoBouzout]

I would have preferred an options object, as this is dealing mostly with data that we already had in the loop. Makes it also easier to use and see how you get the information.

I tried to think how the composable approach would work. I'm having a hard time. It is something like this?

createRAF(
	targetFPS(timeStamp => {
		countFrames(frames => {
			captureDelta(delta => {
				callback(timeStamp, frames, delta)
			})
		})
	}, 15),
)

[thetarnav]

Composition can sometimes lead to more boilerplate, but you get things like control, explicitness and replaceability in exchange. It is also more tied to the ideal of this library, of shipping small, composable primitives.
Not saying that what's currently is perfect, but it's just something that we should keep im mind in designing the new version.
Options are fine, and clearly have worked for the fps throttle, but if it can be easily extracted as a separate primitive, we should do it.
Options also have a problem with things that modify the types/return values. You can see that in solid itself, all the primitives that accept options have the same type definition regardless of the options picked. It's only for toggling some internal behaviour.
I imagine your example could we written in this way: (don't know yet how this functions would exactly work, so just guessing)

let lastTimestamp = 0;
createRAF(
  targetFPS((timeStamp) => {
    const delta = calcDelta(lastTimestamp, timeStamp);
    const frames = calcFPS(delta, 15);
    callback(timeStamp, frames, delta);
  }, 15)
);

This way we are also keeping the interface as close to the browser api as possible. But if there are some improvements to be made then be should explore that. For example how often do you use the delta to calculate values for animation? It sounds like something you should always be doing, but I can image that a lot of people are not using it this way. (I wasn't doing it previously either) So should it be a part of the base createRAF primitive because it is a good practice, or left for users to decide? I'm fine with putting it into the createRAF because it seams essential, while the countFrames thing would be completely separate.
How does this look?

createRAF((timestamp, delta) => {
    const frames = calcFPS(delta);
    callback(timestamp, frames, delta);
});
// or with fps throttling
createRAF(
  targetFPS((timestamp, delta) => {
    const frames = calcFPS(delta);
    callback(timestamp, frames, delta);
  }, 15)
);

[titoBouzout]

The delta time is a useful value, for more than animations. This is a great post about considerations if used in animations https://stackoverflow.com/a/46346441

I understand the gains of compostability, I just imagined this to be differently. the callback could have had an extra parameter with any information needed, like callback(timeStamp, {fps:60, delta:16, etc:bla}). I also do not like the idea of calling so many functions and declaring so many objects.

Anyway, if you are happy with it, feel free to close this, thanks for your time.

[davedbase]

Perhaps this is just a matter of exporting createTargetRAF()? Hides the need to call the extra method. We shouldn't just ship this implementation as createRAF because it's not a standard/expected implementation.

Edit: I misunderstood the original thread. The issue is composability itself being difficult/tricky. Sorry for the confusion haha. Let's just keep in mind that the primitive should balance performance, ease and base functionality. Too much composability makes this cumbersome to use. So striking a balance where the base primitive is packaged with a few extras (maybe delta) and then compose targetFPS and countframes on top of that.

[titoBouzout]

I want to clarify something. This pull request is about not introducing more breaking changes till consensus could be reached about how to handle or implement the extra features. The state of it, is already a breaking change, what I am trying to do here is to not break more than needed, as this may have not been adopted yet (been merged yesterday). The thing is how we move forwards, considering the additional expected features.

As I said before, there are not many features that can be given by this. Roughly speaking, we may want to do the following:

1. being able to tell at how many frames we are running(our callback function)
2. maybe being able to tell at how many frames the screen is refreshing(without considering our callback function)
3. being able to tell how much time passed since the last run(elapsed time)
4. provide a timeStamp (have you seen a HTML5 game that cannot be played because the game runs a lot faster?) here is an example https://www.lessmilk.com/almost-pong/ on my main screen the game runs at 300fps and it's unplayable, because the game is frame based or they are not using this time. If I move the chrome tab to my secondary screen and refresh, I can play it (as that screen refresh rate is 60fps)
5. being able to drop frames (the targetFPS thing, this is possibly a poor name now that I think). This is differently than point 4, as point 4 is not dropping frames, they're drawing a lot more but moving little, is still updating the screen at your frame rate, it's doing the computations considering a time frame. While this will just drop frames till elapsed > interval instead.

I'm not convinced that there's a need to provide compostability to deal with this, I think an options object would suffice. But that's just me, obviously. I favour performance on this case over anything else, this is probably one of the hottest function of the whole library. I will never use it if this calls 1k functions every second.

Maybe @trusktr was right, and this pull request should be merged, so to each their own about how to deal with the features.

[titoBouzout]

Imma close it and if there's anything to contribute I will open a new one!, Thanks!