Skip to content

Conversation

wo-o29
Copy link
Contributor

@wo-o29 wo-o29 commented Sep 15, 2025

Overview

This PR improves the mergeProps utility function by:

  1. Removing redundant checks for undefined in the pushProp function.
    Since the if (curr[key] === undefined) continue; statement is already handled at the start of the loop, the additional else if (curr[key] !== undefined) check inside the default case is unnecessary and removed for clarity.

  2. Simplifying the mergeStyle function by removing the null check for the first style argument.
    In practice, a is never null but can be undefined.
    Spreading potentially undefined values with { ...a, ...b } safely results in identical behavior without extra branching logic.

These changes clean up the code, improve maintainability, and have no effect on the existing behavior or output of the utility.

Checklist

  • Did you write or update tests to cover your changes?
  • Did you run yarn run fix to format and lint the code and docs?
  • Did you run yarn run test:coverage to confirm test coverage?
  • Did you update JSDoc comments if applicable?

@Copilot Copilot AI review requested due to automatic review settings September 15, 2025 12:54
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.

Pull Request Overview

Refactors the mergeProps utility function by simplifying conditional logic and removing redundant checks for better maintainability.

  • Removes redundant undefined check in the default case since it's already handled at the loop start
  • Simplifies style merging by removing null check and relying on spread operator behavior
  • Improves code clarity without changing functionality
Comments suppressed due to low confidence (1)

src/utils/mergeProps/mergeProps.ts:53

  • The break statement on line 50 creates unreachable code on line 53. When mergedFunction is truthy, the code will exit the switch case and never reach the assignment prev[key] = curr[key]. Consider restructuring this logic or removing the unreachable code.
        const mergedFunction = mergeFunction(prev[key], curr[key]);

        if (mergedFunction) {
          prev[key] = mergedFunction;
          break;
        }

        prev[key] = curr[key];

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5bdb4ff) to head (d62a553).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #279   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         1093      1092    -1     
  Branches       324       323    -1     
=========================================
- Hits          1093      1092    -1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants