diff --git a/config/oxlint-plugins/app-store-performance.mjs b/config/oxlint-plugins/app-store-performance.mjs index 9d5ac41eac1..2fc37e07ddb 100644 --- a/config/oxlint-plugins/app-store-performance.mjs +++ b/config/oxlint-plugins/app-store-performance.mjs @@ -173,6 +173,45 @@ function resolveSelector(argument, state) { return name ? (state.namedSelectors.get(name) ?? null) : null } +/** + * Allocation that happens on EVERY call, used when following a selector into a + * helper. Deliberately stricter than isAllocatingExpression: a helper that returns + * a cached reference on one branch and builds a fresh one on another is the normal + * identity-caching shape, and flagging it would be a false positive. + */ +function alwaysAllocates(expression) { + if (expression?.type === 'ConditionalExpression') { + return alwaysAllocates(expression.consequent) && alwaysAllocates(expression.alternate) + } + if (expression?.type === 'LogicalExpression') { + return alwaysAllocates(expression.left) && alwaysAllocates(expression.right) + } + return ( + expression?.type === 'ArrayExpression' || + expression?.type === 'ObjectExpression' || + expression?.type === 'NewExpression' || + (expression?.type === 'CallExpression' && + ALLOCATING_METHODS.has(propertyName(expression.callee) ?? '')) + ) +} + +/** + * One hop: a selector that delegates to a module-scope helper is the idiomatic + * shape here, and neither the inline-body check nor a reviewer reading the call + * site can see what that helper returns. + */ +function expandThroughNamedHelper(expression, state) { + if (expression?.type !== 'CallExpression') { + return [expression] + } + const helper = state.namedSelectors.get(identifierName(expression.callee) ?? '') + if (!helper) { + return [expression] + } + const returned = returnedExpressions(helper) + return returned.length > 0 && returned.every(alwaysAllocates) ? returned : [expression] +} + function createRuleState() { return { appStoreHooks: new Set(), @@ -270,7 +309,8 @@ function deferredSelectorRule(inspect) { selector: resolveSelector(argument, state), argument, shallow, - node + node, + state }) if (report) { this.report(report) @@ -293,11 +333,13 @@ function noIdentitySelectorRule() { } function noFreshSelectorResultRule() { - return deferredSelectorRule(({ selector, shallow }) => { + return deferredSelectorRule(({ selector, shallow, state }) => { if (shallow || !selector) { return null } - const freshResult = returnedExpressions(selector).find(isAllocatingExpression) + const freshResult = returnedExpressions(selector) + .flatMap((expression) => expandThroughNamedHelper(expression, state)) + .find(isAllocatingExpression) return freshResult ? { node: freshResult, @@ -322,12 +364,14 @@ function nestedFreshValues(expression) { } function noNestedFreshUnderShallowRule() { - return deferredSelectorRule(({ selector, shallow }) => { + return deferredSelectorRule(({ selector, shallow, state }) => { if (!shallow || !selector) { return null } const nestedFresh = returnedExpressions(selector) + .flatMap((expression) => expandThroughNamedHelper(expression, state)) .flatMap(nestedFreshValues) + .flatMap((expression) => expandThroughNamedHelper(expression, state)) .find(isAllocatingExpression) return nestedFresh ? { diff --git a/config/scripts/app-store-performance-plugin.test.mjs b/config/scripts/app-store-performance-plugin.test.mjs index 1af760482a8..649b246e4e7 100644 --- a/config/scripts/app-store-performance-plugin.test.mjs +++ b/config/scripts/app-store-performance-plugin.test.mjs @@ -97,4 +97,34 @@ describe('app store performance Oxlint plugin', () => { 'app-store-performance(no-nested-fresh-under-shallow)' ]) }) + + it('follows a selector one hop into a module-scope helper', () => { + const diagnostics = lintSource(` + import { useAppStore } from '@/store' + import { useShallow } from 'zustand/react/shallow' + const buildRows = (state) => state.rows.map((row) => row.id) + const Delegating = () => useAppStore((state) => buildRows(state)) + const NestedDelegating = () => useAppStore(useShallow((state) => ({ ids: buildRows(state) }))) + `) + + expect(diagnostics.map((diagnostic) => diagnostic.code)).toEqual([ + 'app-store-performance(no-fresh-selector-result)', + 'app-store-performance(no-nested-fresh-under-shallow)' + ]) + }) + + it('does not flag a helper that returns a cached reference on some branch', () => { + const diagnostics = lintSource(` + import { useAppStore } from '@/store' + import { useShallow } from 'zustand/react/shallow' + // The identity-caching shape: fresh only on a miss, cached otherwise. + const selectCachedRows = (state) => cache.get(state.key) ?? state.rows.filter(Boolean) + const Cached = () => useAppStore((state) => selectCachedRows(state)) + const CachedNested = () => useAppStore(useShallow((state) => ({ rows: selectCachedRows(state) }))) + // An unknown helper cannot be resolved, so it must not be guessed at. + const External = () => useAppStore((state) => externalBuild(state)) + `) + + expect(diagnostics).toEqual([]) + }) })