Skip to content

Conversation

@D-Sketon
Copy link
Contributor

@D-Sketon D-Sketon commented Jan 2, 2026

Use the spread operator to append promises returned by processNodeWithOnAttrs to the promises array instead of the incorrect promises.push.apply(...) call.

Summary by CodeRabbit

  • Refactor
    • Improved code efficiency in the JavaScript minification process with streamlined array handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@netlify
Copy link

netlify bot commented Jan 2, 2026

👷 Deploy request for htmlnano pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 074485e

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

Walkthrough

Replaced Function.prototype.apply with the spread operator syntax for extending a promises array in the minifyJs module. The control flow and behavior remain unchanged; only the method of adding array elements is updated.

Changes

Cohort / File(s) Summary
Promise array handling
src/_modules/minifyJs.ts
Refactored promises.push.apply(processNodeWithOnAttrs(...)) to use spread operator: promises.push(...processNodeWithOnAttrs(...)). No behavioral changes; purely modernizing array extension syntax.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing an incorrect push.apply call with the spread operator for pushing event-handler minification promises in minifyJs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 737e964 and 074485e.

📒 Files selected for processing (1)
  • src/_modules/minifyJs.ts
🔇 Additional comments (1)
src/_modules/minifyJs.ts (1)

49-49: Excellent bug fix!

The previous code promises.push.apply(processNodeWithOnAttrs(...)) was incorrect because apply requires two arguments: the context (this) and an array of arguments. The correct syntax would have been promises.push.apply(promises, processNodeWithOnAttrs(...)).

The spread operator promises.push(...processNodeWithOnAttrs(...)) is the modern, idiomatic approach and correctly extends the array with all returned promises.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@maltsev
Copy link
Owner

maltsev commented Jan 3, 2026

Thanks!

@maltsev maltsev merged commit a3284da into maltsev:master Jan 3, 2026
6 checks passed
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