Problem/Motivation

This issue similar to #3416898: Use String.prototype.includes() instead of String.prototype.indexOf() where necessary.
Some of Array.prototype.indexOf() is used to check a specific item is or isn't included within a array.

Proposed resolution

Use Array.prototype.includes().
This method is suitable for array item existence check.

Remaining tasks

User interface changes

No.

API changes

No.

Data model changes

No.

Release notes snippet

Issue fork drupal-3454079

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Tom Konda created an issue. See original summary.

Tom Konda opened merge request !8381

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Coming from #3416898: Use String.prototype.includes() instead of String.prototype.indexOf() where necessary this change already seemed to be accepted so this seems fine.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Needs work

small detail

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Needs work » Needs review

I changed to simple else clause in my local environment, it seems to work correctly.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback fron @nod_ appears to be addressed.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Needs work

Thanks for the update. When reviewing with the full context, it makes sense to have the else if, can you add it back please?

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Needs work » Needs review

I reverted else to else if.
Please review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Revert was fine.

nod_ closed merge request !8381

  • nod_ committed 6f618569 on 10.4.x 
    Issue #3454079 by Tom Konda, smustgrave: Prefer to use Array.prototype....

  • nod_ committed 530cf173 on 11.0.x 
    Issue #3454079 by Tom Konda, smustgrave: Prefer to use Array.prototype....

  • nod_ committed 0819b7aa on 11.x 
    Issue #3454079 by Tom Konda, smustgrave: Prefer to use Array.prototype....
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 0819b7aaf4 to 11.x and 530cf173e4 to 11.0.x and 6f61856980 to 10.4.x. Thanks!

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Fixed » Needs work

10.4.x branch has tracker module.
In this module still have Array.prototype.indexOf() so need to fix it.

Tom Konda opened merge request !8642

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Needs work » Needs review

Fix remaining Array.prototype.indexOf() on the 10.4.x branch.
Please review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

LGTM!

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the update, since it's pretty minor I'd rather not reopen this one, especially since the tracker module is out of 11.x

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 10.4.x-dev » 10.3.x-dev
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 10.3.x-dev » 10.4.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.