Skip to content

fix: early return in toolbar button to avoid unneeded calls to useInt… #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Arukuen wants to merge 2 commits into
base: develop
Choose a base branch
from
+5 −2
Open

fix: early return in toolbar button to avoid unneeded calls to useInt… #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbd7e8e
fix: early return in toolbar button to avoid unneeded calls to useInt…
Arukuen May 5, 2026
8ca2974
fix: isolate interactionsFiltered into separate useSelect to reduce e…
Arukuen May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/editor/hooks/use-interactions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,19 +244,22 @@ const useInteractions = () => {
}

const interactions = select( 'interact/interactions' ).getInteractions()
const interactionsFiltered = interactions.filter( interaction => isInteractionShown( interaction, select ) )

return {
interactions,
interactionsFiltered,
setInteractions,
updateInteraction,
deleteInteraction,
}
}, [] )

const interactionsFiltered = useSelect( select => {
return data.interactions.filter( interaction => isInteractionShown( interaction, select ) )
}, [ data ] )
Comment on lines +256 to +258
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In @wordpress/datauseSelect, does putting an object like[data]in deps cause recalculation when object identity changes, and is[data.interactions] the recommended dependency when only that field is used?

💡 Result:

Yes, putting an object like [data] in the useSelect dependencies array causes recalculation (i.e., the mapSelect callback is rememoized via useCallback) whenever the object's reference changes, even if its contents are shallowly equal. This is because the dependencies array uses React's standard dependency comparison, which for objects is reference equality (!==), not shallow content equality.[1][2][3] The deps array memoizes the mapSelect function with useCallback(mapSelect, deps), so a new object reference [data] (if data is a new object instance) will trigger recreation of the callback function, causing useSelect to re-evaluate selectors even if store state hasn't changed.[2][4] [data.interactions] is the recommended approach when only that field is used inside the mapSelect callback. Official guidance emphasizes including in deps exactly the values referenced inside the callback (like primitive IDs, search terms, or specific fields), to ensure correctness (callback uses current values) and performance (avoids unnecessary recomputations).[5][1][6][7][3] Using a primitive or specific field avoids unnecessary triggers from unrelated object reference changes. ESLint react-hooks/exhaustive-deps is configured to validate useSelect deps.[4]

Citations:

🏁 Script executed:

# Read the file around the mentioned lines to understand the context
sed -n '240,275p' src/editor/hooks/use-interactions.js

Repository: gambitph/Interactions

Length of output: 1220

🏁 Script executed:

# Check the full function to see the first useSelect that creates `data`
sed -n '200,260p' src/editor/hooks/use-interactions.js | head -70

Repository: gambitph/Interactions

Length of output: 2027

Narrow the selector dependency to data.interactions to avoid unnecessary re-evaluations.

At line 258, using [ data ] as a dependency makes this selector re-evaluate whenever the entire data object reference changes. Since this callback only accesses data.interactions, the dependency array should be narrowed to [ data.interactions ]. This follows the WordPress @wordpress/data guidance and ensures the selector runs only when the interactions data actually changes.

Suggested fix 
	const interactionsFiltered = useSelect( select => {
		return data.interactions.filter( interaction => isInteractionShown( interaction, select ) )
-	}, [ data ] )
+	}, [ data.interactions ] )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const interactionsFiltered = useSelect( select => {
return data.interactions.filter( interaction => isInteractionShown( interaction, select ) )
}, [ data ] )
const interactionsFiltered = useSelect( select => {
return data.interactions.filter( interaction => isInteractionShown( interaction, select ) )
}, [ data.interactions ] )
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/editor/hooks/use-interactions.js` around lines 256 - 258, The selector
passed to useSelect (used to compute interactionsFiltered) currently depends on
the whole data object ([data]) causing unnecessary re-runs; change the
dependency array to [data.interactions] so the selector only re-evaluates when
interactions actually change. Locate the useSelect call that defines
interactionsFiltered and update its dependency array accordingly (the callback
uses isInteractionShown and data.interactions).


return {
loadingError: useSelect( select => select( 'interact/interactions' ).getLoadingError(), [] ),
interactionsFiltered,
...data,
}
}
Expand Down
Loading