Preact: [preact/debug] false positive complain about memoization

[o-alexandrov]

Reproduction

Libraries:

• preact 10.4.8

Steps to reproduce

Press here to see an example.
Dynamic dependencies of memoized calculations through useMemo/useCallback result in a false positive complain about memoization.

Expected Behavior

Dynamic dependencies shouldn't result in a console's complain.

Actual Behavior

False positive complain.

[marvinhagemeister]

I'm not sure I understand. Looking at the linked code, it sets the dependency array to undefined. That's the same as not setting it in the first place in JavaScript. So the code is essentially doing this:

useMemo(() => ...)
// vs
useMemo(() => ..., [])

Without a dependency array the memoization will not work as the value is always recomputed on each render.

[o-alexandrov]

@marvinhagemeister based on the linked line in the description of this issue, seems like framer-motion's devs intent was exactly to disable memoization conditionally.

Based on your comment, I understand that your proposal is to:

• not to set undefined, but instead explicitly mark all dependencies

The problem is:

• they don't need memoization in certain conditions
• it's much more efficient to disable memoization by passing undefined, rather than specifying dependencies, that you would know would always be triggered as changed in that certain condition
  
  
  
  • hence there is a conditional memoization
  
  

[o-alexandrov]

@InventingWithMonster could you please share your view?
Hopefully, I didn't misinterpret the intention in the mentioned file in the description of this Issue above.
This issue is about DX when working with preact and framer-motion

[mattgperry]

@o-alexandrov That's correct. In some circumstances we want to disable memoisation, so we pass undefined. We can't count on this being true for the duration of the lifecycle of the component so we can't conditionally use the hook, and the alternative is goofy stuff like [isPresent] : [Math.random()]

IMO I don't think it's helpful to have these warnings. They will run for external libraries and they haven't been written in the same development env as the one they're consumed in. Most likely these will be written for React, which doesn't have this warning, so it isn't easy for library authors to avoid. I also don't think library internals should be exposed in this way.

Linting feels more appropriate.

[afzalsayed96]

Is there a way to turn this off in dev? Can I contribute a PR for resolving this?

[marvinhagemeister]

Is there a way to turn this off in dev?

No.

Can I contribute a PR for resolving this?

Yes 👍

[afzalsayed96]

@marvinhagemeister Okay what would be the right approach here according to you?

[marvinhagemeister]

@afzalsayed96 remove that check in preact/debug. My understanding is that linters can/should take care of that already and linter rules can be easily ignored on a case-by-case basis.