Skip to content

feat(gene-tools): allow inline arrow functions calling memoized functions #74

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.

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

Merged

Conversation

RaulVallespinLuque
Copy link
Contributor

Previously, the memoization validator would reject all inline arrow functions in props,
even when they were just passing parameters to a memoized function. This was too strict
and forced developers to create separate variables for simple parameter passing.

This change allows inline arrow functions specifically when they only call a memoized
function, like:
onClick={() => memoizedFn(id)}

where memoizedFn is created with useCallback. This pattern is safe since the called
function is properly memoized, and it improves code readability for simple parameter
passing cases.

Changes:

  • Added isCallToMemoizedFunction helper to detect valid cases
  • Modified arrow function validation to allow calls to memoized functions
  • Added test case and example component to verify the behavior

@RaulVallespinLuque RaulVallespinLuque added the enhancement New feature or request label Apr 14, 2025
@RaulVallespinLuque RaulVallespinLuque changed the title Feat(gene-tools): allow inline arrow functions calling memoized functions feat(gene-tools): allow inline arrow functions calling memoized functions Apr 14, 2025
@kopach kopach requested a review from Copilot April 14, 2025 16:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

packages/gene-tools/src/checkers/common/validators/memoizeValidator.js:140

  • The current check only supports inline arrow functions with a direct CallExpression body. Consider extending the logic to handle arrow functions with block bodies that return a call expression, if that usage is expected.
if (!isCallToMemoizedFunction(path, path.node.value.expression)) {

Copy link
Member

@kopach kopach left a comment

Choose a reason for hiding this comment

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

probably better to check with authors, but my guess is that original intent was to prevent re-rendering of memoized components. if we pass arrow function as a prop to such component - it will be treated as new reference and component will re-render
Therefore, if my assumption is correct, even though I like this simplification, IMO we can't merge it

@kopach kopach added type: feature and removed enhancement New feature or request labels Apr 14, 2025
@RaulVallespinLuque RaulVallespinLuque enabled auto-merge (squash) April 15, 2025 08:41
@RaulVallespinLuque RaulVallespinLuque merged commit da45d2d into master Apr 15, 2025
16 checks passed
@RaulVallespinLuque RaulVallespinLuque deleted the feat/allow-inline-arrow-with-memoized-calls branch April 15, 2025 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants