Allows the force update to be infinitely called

[zeachco]

This use case is far-fetched but in some contexts (gaming for example) the number of iterations can go beyond the maximum value supported by javascript and end up being Infinity which would cause the force update to stop working
because Infinity + 1 === Infinity is true in javascript

I didn't added tests as I suppose we don't want to run a tests with a loop of 9007199254740991+ iterations and had some trouble mocking the tick value of useState

Code change checklist

• Added/updated unit tests
• Updated /docs. For new functionality, at least API.md should be updated
• Verified that there is no significant performance drop (npm run perf)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 65adb55

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mweststrate]

Nice catch! Random idea with integer unsafety, an alternative might be to use (tick => ++tick % 1000)?

[zeachco]

Nice catch! Random idea with integer unsafety, an alternative might be to use (tick => ++tick % 1000)?

Modulo is expensive from last time I checked (2005'ish 😅 ) and we can't really assume how many renders React could batch, or can we?

[mweststrate]

Fair points :)

…

On Thu, Jun 10, 2021 at 8:55 PM Olivier Rousseau ***@***.***> wrote: Nice catch! Random idea with integer unsafety, an alternative might be to use (tick => ++tick % 1000)? Modulo is expensive from last time I checked (2005'ish 😅 ) and we can't really assume how many renders React could batch, or can we? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2982 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBBLVMOZZSDUIYQT3GTTSEKEXANCNFSM46PC2PVA> .

[zeachco]

Also to mention, it's the first time I contribute to this project, I couldn't find the npm run perf as it seems to be only for mobx itself but I used fastest way known to me to "reset" the loop without using bit-shifting 😅

[urugator]

I think useState uses strict comparison, can't we just setState(NaN)?

[urugator]

Btw should we worry about

mobx/packages/mobx/src/core/globalstate.ts

Line 54 in eb4fb5d

runId = 0

?

mobxGuid seems to be used only for debug names, therefore harmless I suppose?

[zeachco]

I think useState uses strict comparison, can't we just setState(NaN)?

I think that would even be better, I'll give it a try when I have a chance

[mweststrate]

Hope that they will never use Object.is in that case, as that would make everything fall apart :')

…

On Fri, Jun 11, 2021 at 1:07 PM Olivier Rousseau ***@***.***> wrote: I think useState uses strict comparison, can't we just setState(NaN)? I think that would even be better, I'll give it a try when I have a chance — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2982 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBABMR3IXILLGZQCD6LTSH4AZANCNFSM46PC2PVA> .

[zeachco]

So I tried using NaN but it doesn't work
for experiments, I played around with https://codesandbox.io/s/useforceupdate-cu3nh?file=/src/index.js:943-960

I think using an object reference would work better since {} !== {}

function useForceUpdate() {
  const [, setState] = useState({});
  return () => setState(() => ({}));
}

alternatively, this works too but side-effects, if any, would be triggered in a different order because we don't rely on the setState callback

function useForceUpdate() {
  const [value, setState] = useState(true);
  return () => setState(!value);
}

[zeachco]

Hope that they will never use Object.is in that case, as that would make everything fall apart :')

Spotted on! They do use a copy of the polyfill in their shallowEqual 🤷
https://github.com/facebook/fbjs/blob/c69904a511b900266935168223063dd8772dfc40/packages/fbjs/src/core/shallowEqual.js#L29

[urugator]

Ah it's noted here, could't find it before (it's not mentioned in useState docs...).
Here is explained how comparison/bailout works (it states ===, but whatever).

I think using an object reference would work better since {} !== {}

👍 (without the extra closure)

[zeachco]

Here is explained how comparison/bailout works (it states ===, but whatever).

Maybe I missed something, I understand what's being said but it doesn't seems to be the actual behaviour as of react 16.8.0

function useForceUpdateNoValue() {
  const [, setState] = useState();
  return () => setState();
}

it doesn't update, I'd like to blame the shallowEqual mentioned above

👍 (without the extra closure)

no sure what you mean by extra closures, talking about the () => ({}) ? It's an object construction and not a closure since it's wrapped in parentheses

Or was it the initial value of useState ?
It's true we don't need useState({}) and we could just have useState() but if react changes their compare algo, it would be easier to debug something that happens 100% rather than only on the second re-render 🤔
but that's a personal opinion and I'm very open changing it to with or without 🤷

[urugator]

it doesn't seems to be the actual behaviour

Hm weird indeed

no sure what you mean by extra closures

setRef(() => ({})) => setRef({})

[zeachco]

setRef(() => ({})) => setRef({})

I though there was a reason at first mobx did this, when calling the setState's dispatcher
I though it could change the order in which useEffect are called like this

setRef({}) // executes on render once at the end of the batch
setRef(() => ({})) // executes on render as many times as it's called during the batch

so useEffects based off values derived by the mobx object would trigger a different amount of times

but after testing, it seems it does not 🤔
meaning we could simplify all this and just to

function useForceUpdate() {
  const [value, setRef] = useState(false);
  return () => setRef(!value);
}

I have updated my example

[zeachco]

Ok, my example was weak, we can see this mobx test failing about it while not using the react dispatcher instead of a direct value set

[urugator]

I though there was a reason at first mobx did this

I think the only reason was the need for previous value to perform increment :)

meaning we could simplify all this and just to

Toggle would rely on comparison happening immediately (rather than sometimes later), dunno if it's a safe choice...

[urugator]

Pushed a PR #2983 as it was faster than explaining.

[zeachco]

thanks, I was wondering about the useCallback as well 👍
you solution looks better so I'll close this PR