Rule proposal: Help with memoization

[fregante]

Description

Memoized functions rely on their arguments to work properly. Using global state means you also have to remember using the same global state in cacheKey.

Thoughts on how this can be avoided?

Fail

const api = mem(() => fetch(`/api/${location.hash.replace('#','')}`))
api() // while on the home
api() // while on the contacts page

const api = mem((name = location.hash.replace('#','')) => fetch(`/api/${name}`))
api()
api('contacts')

const api = mem((name) => fetch(`/api/${name ?? location.hash.replace('#','')}`))
api()
api('contacts')

Pass

const api = mem((name) => fetch(`/api/${name}`))
api(location.hash.replace('#',''))
api('contacts')

Additional Info

This is probably impossible to actually achieve in a linter though 🥲

Related:

• Many features don't reset across different repositories refined-github/refined-github#5821 (comment)

[sindresorhus]

We could make a rule where you give it a list of method names where you want the function to be pure.

[fregante]

I don’t think that’s possible though. Can I use fetch in a pure function? I think it’s difficult to tell apart the inputs (location.hash) from the output (the server response via fetch). Both are technically accessing “global state”

[sindresorhus]

@fregante Then you would have to use an ignore comment for fetch. At least it would force you to make an explicit choice about it.

[fregante]

Makes sense, but also:

1 first commit

// eslint-disable-next-line pure-memoization -- fetch is ok
const api = mem((name) => fetch(`/api/${name}`))

2 add default

// eslint-disable-next-line pure-memoization -- fetch is ok
const api = mem((name = document.title) => fetch(`/api/${name}`))

[sindresorhus]

That is not specific to this rule though. You can accidentally disable more than you intended with any rule. It's also not a realistic example.

[fregante]

I agree with your last point regarding "making an explicit choice"

See these for a realistic example and the bugfix PR that such a rule would have prevented. Plenty of other such bugs in RG:

• https://github.com/refined-github/refined-github/blob/05e146aea409cec6d94828b0b9fc0339d202e70d/source/github-helpers/get-default-branch.ts#L18
• Fix feature cache across ajaxed page loads refined-github/refined-github#5841

Another example where global/local scope should be blocked: serialized functions.

• ESLint/TypeScript: Any way to mark the function as external/scope-less? developit/greenlet#56

There's a slight difference though

• help with memoization: block access to external state, but allow pure functions
• help with serialization: block access to local scope, but allow browser globals

[fregante]

Probably impossible or too difficult or too complex for unicorn:

• https://stackoverflow.com/a/69389079 ("impossible")
• https://github.com/purely-functional/eslint-plugin-pure (abandoned)
• https://github.com/jhusain/eslint-plugin-immutable (abandoned)

I thought about potentially enabling this rule on specific files, maybe enforcing only /*#__PURE__*/ items, recursively, sounds like fools errand.

Maybe a rule that blocks any globals (except stuff like Math.*, Object.*) would be more useful. But even then, you can't guarantee that a local value doesn't depend on a global, like Date.now() or myElement.ownerDocument.location.href

[fregante]

I think the only possible helper for memoized functions would be a rule to block default parameters, to be enabled on-demand:

// eslint-enable-next-line unicorn/no-defaults
const api = mem((name = location.hash.replace('#','')) => fetch(`/api/${name}`))
//                    ^ Error: Default value not allowed
api()
api('contacts')