Skip to content

refactor: use Array.includes() instead of indexOf() > -1#8529

Open
dashitongzhi wants to merge 1 commit into
DevExpress:masterfrom
dashitongzhi:code-quality-improvements
Open

refactor: use Array.includes() instead of indexOf() > -1#8529
dashitongzhi wants to merge 1 commit into
DevExpress:masterfrom
dashitongzhi:code-quality-improvements

Conversation

@dashitongzhi
Copy link
Copy Markdown

Description

Replace legacy indexOf() > -1 pattern with more readable Array.includes() method across multiple files. Also replace map() with forEach() when the return value is not used (side-effect only).

Changes Made

  • src/utils/handle-errors.js: Changed map() to forEach() for console.log side effects
  • src/client-functions/selectors/create-snapshot-methods.js: hasClass method now uses includes()
  • src/client-functions/selectors/add-api.js: filterNodes and expandSelectorResults now use includes()
  • src/utils/string.js: splitQuotedText and getConcatenatedValuesString now use includes()
  • src/browser/provider/built-in/locally-installed.js: isValidBrowserName now uses includes()
  • src/client/ui/select-element.js: onWindowClick disabled class check now uses includes()
  • src/compiler/test-file/formats/typescript/get-test-list.js: replaceComments and getStringValue now use includes()
  • src/compiler/test-file/formats/es-next/get-test-list.js: getStringValue now uses includes()
  • src/compiler/test-file/test-file-parser-base.js: checkExpDefineTargetName now uses includes()

Benefits

  1. Improved readability: includes() is more semantic and self-documenting than indexOf() > -1
  2. Modern JavaScript: Follows ES6+ best practices
  3. Correctness: Using forEach() instead of map() when return value is not used makes intent clearer and avoids confusion

Total: 9 files changed, 12 insertions(+), 12 deletions(-)

Replace legacy indexOf() > -1 pattern with more readable Array.includes() method across multiple files. Also replace map() with forEach() when the return value is not used (side-effect only).

Changes:
- src/utils/handle-errors.js: map -> forEach for console.log side effects
- src/client-functions/selectors/create-snapshot-methods.js: hasClass method
- src/client-functions/selectors/add-api.js: filterNodes and expandSelectorResults
- src/utils/string.js: splitQuotedText and getConcatenatedValuesString
- src/browser/provider/built-in/locally-installed.js: isValidBrowserName
- src/client/ui/select-element.js: onWindowClick disabled class check
- src/compiler/test-file/formats/typescript/get-test-list.js: replaceComments and getStringValue
- src/compiler/test-file/formats/es-next/get-test-list.js: getStringValue
- src/compiler/test-file/test-file-parser-base.js: checkExpDefineTargetName

This improves code readability and follows modern JavaScript best practices.
Copilot AI review requested due to automatic review settings May 20, 2026 01:28
@testcafe-need-response-bot testcafe-need-response-bot Bot added the STATE: Need response An issue that requires a response or attention from the team. label May 20, 2026
Copy link
Copy Markdown
Contributor

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 several membership checks to use includes() instead of the legacy indexOf(...) > -1 / < 0 patterns, and replaces a side-effect-only map() with forEach() to better match intent. This aligns with existing codebase usage of includes() and improves readability without changing behavior.

Changes:

  • Replaced indexOf(...) > -1 / < 0 checks with includes() / !includes() across client, compiler, browser-provider, and utility code.
  • Replaced messages.map(...) with messages.forEach(...) where the return value was unused.
  • Minor readability improvements in string/newline checks via String.prototype.includes.

Reviewed changes

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

Show a summary per file
File Description
src/utils/string.js Uses includes() for quote-char and newline detection.
src/utils/handle-errors.js Replaces side-effect-only map() with forEach() for logging.
src/compiler/test-file/test-file-parser-base.js Uses includes() for method-name membership checks.
src/compiler/test-file/formats/typescript/get-test-list.js Uses includes() for newline detection and token-type membership.
src/compiler/test-file/formats/es-next/get-test-list.js Uses includes() for token-type membership.
src/client/ui/select-element.js Uses includes() for disabled-class detection.
src/client-functions/selectors/create-snapshot-methods.js Uses includes() for hasClass on snapshots.
src/client-functions/selectors/add-api.js Uses includes() for node membership and deduplication in selector results.
src/browser/provider/built-in/locally-installed.js Uses includes() to validate local browser names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

STATE: Need response An issue that requires a response or attention from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants