Thank you for your PR 🙏🏽!

Can you briefly explain why we should handle ref(null) if the types only accepts MaybeRef<number>?

Can you briefly explain why we should handle ref(null) if the types only accepts MaybeRef<number>?

Oh, I confirmed this as a bug in #5273. I hadn't noticed the type restriction before.🤦‍♂️

But if someone turns off strictNullChecks in their tsconfig.json, TypeScript won't report an error. For example:

const fpsLimit = ref<number>(null)

useRafFn(() => {

}, { fpsLimit })

You're right that the types only accept MaybeRefOrGetter<number>, but I think that's actually the root of the problem. The option defaults to undefined (no limit), so the API already supports the concept of "no limit." But if a user wants to reactively toggle between limited and unlimited at runtime, they can't express that with the current type.

As @9romise mentioned, this also surfaces with strictNullChecks off, but beyond that I think the type should be widened to MaybeRefOrGetter<number | null | undefined> to officially support the reactive toggle use case. I'll update the PR with that change.

As @9romise mentioned, this also surfaces with strictNullChecks off, but beyond that I think the type should be widened to MaybeRefOrGetter<number | null | undefined> to officially support the reactive toggle use case. I'll update the PR with that change.

Honestly, I don't think we should allow MaybeRefOrGetter<null | undefined>. You should always handle what you expect null to be in your code. I think @43081j might have a stronger opinion on that 😅

That's a fair point.

That said, I think the runtime fix itself still stands on its own. It's not really about supporting null, it's about fixing the toValue() ordering. When fpsLimit was made reactive in #4409, the truthiness check stayed on the raw fpsLimit parameter instead of the unwrapped value. Since a ref object is always truthy, toValue() then resolves to whatever the ref holds, and 1000 / null gives Infinity (silently breaking the loop with no errors).

The fix just moves toValue(), before the truthiness check.

From my perspective, the core need here seems to be the ability to control the limit reactively (enable/disable).
See: https://github.com/vueuse/vueuse/blob/main/packages%2Fcore%2FuseRafFn%2Findex.ts#L27-L33

However, while we treat undefined as no limit, the type is currently restricted to MaybeRefOrGetter<number>, which makes it impossible to support this use case.

change makes sense to me but i agree, i would be explicit. is null "off", or is undefined "off"? we should pick one and only accept that. in this case i'd lean towards null since it implies someone had to actively set the value, rather than just mistakenly passing an undefined variable.

note that this is just a typescript decision, the underlying code outside of TS will handle null and undefined still

change makes sense to me but i agree, i would be explicit. is null "off", or is undefined "off"? we should pick one and only accept that. in this case i'd lean towards null since it implies someone had to actively set the value, rather than just mistakenly passing an undefined variable.

note that this is just a typescript decision, the underlying code outside of TS will handle null and undefined still

I'll cast my vote for null, for the same reason.

Open in StackBlitz

npm i https://pkg.pr.new/@vueuse/components@5284

npm i https://pkg.pr.new/@vueuse/core@5284

npm i https://pkg.pr.new/@vueuse/electron@5284

npm i https://pkg.pr.new/@vueuse/firebase@5284

npm i https://pkg.pr.new/@vueuse/integrations@5284

npm i https://pkg.pr.new/@vueuse/math@5284

npm i https://pkg.pr.new/@vueuse/metadata@5284

npm i https://pkg.pr.new/@vueuse/nuxt@5284

npm i https://pkg.pr.new/@vueuse/router@5284

npm i https://pkg.pr.new/@vueuse/rxjs@5284

npm i https://pkg.pr.new/@vueuse/shared@5284

commit: 198eb95

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.46%. Comparing base (39afd85) to head (198eb95).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #5284      +/-   ##
==========================================
- Coverage   65.46%   65.46%   -0.01%     
==========================================
  Files         344      344              
  Lines        8039     8040       +1     
  Branches     2483     2471      -12     
==========================================
  Hits         5263     5263              
- Misses       2257     2258       +1     
  Partials      519      519              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.