w3c/compute-pressure

PressureObserver options object: constructor vs observe method

Elchi3 opened this issue · 3 comments

Currently, sampleInterval is an option of the PressureObserver constructor. If I want to create several pressure observers, I guess my code would look like this:

const cpuObserver = new PressureObserver(callback, { sampleInterval: 1500 });
await cpuObserver.observe("cpu");

const cpuObserverHighSampleRate = new PressureObserver(callback, { sampleInterval: 7000 });
await cpuObserverHighSampleRate.observe("cpu");

const gpuObserver = new PressureObserver(callback, { sampleInterval: 2500 });
await gpuObserver.observe("gpu");

In other APIs, for example for the PerformanceObserver, the options are on the observe method. If that would be the case for this API, I could write code like this:

const observer = new PressureObserver(callback);
await observer.observe("cpu", { sampleInterval: 1500 });
await observer.observe("cpu", { sampleInterval: 7000 });
await observer.observe("gpu", { sampleInterval: 2500 });

Not sure if having multiple pressure observers is typical but I guess if I had multiple of them, I likely want to have different sources or options. I see that "cpu" is the only source and sampleInterval is the only option for now, though, so maybe this will only matter if this API offers more sources and more options in the future.

Even more future proof:

await observer.observe({ sources: ["cpu"], sampleInterval: 2500 });

Indeed, the plan is to have more than one source in the future.
Also as you mentioned, in issue #213, the array of source was already proposed.
We will be adding sources and fixing #213 whenever we find reliable enough metrics to read for these sources.

I believe we used to have the options as part of the observe method, and I don't recall the reason why we moved it. Maybe for consistency with other API at that point.

I am not against moving it to the observe method. Potentially we could also do that later and have it overwrite what was set in the constructor.

Looks like this will be fixed by #261