[core] Replace indexOf with includes

[k-rajat19]

• I have followed (at least) the PR section of the contributing guide.

Fixes #42826

[aarongarciah]

Tests are failing. Some conditions must be negated depending on whether the searched term should be present or not:

• arr.indexOf(term) === -1 -> !array.includes(term)
• arr.indexOf(term) !== -1 -> array.includes(term)

The same applies when using .indexOf on strings.

[mui-bot]

Netlify deploy preview

https://deploy-preview-42883--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 07e2911

[aarongarciah]

Looks good after the last changes, but I'd like an additional review from @mui/code-infra since this PR touches many different areas.

[Janpot]

I'm not combing through each and every individual line change, but I'm good with the approach. Two things to note:

• Are we aware of any microbenchmarks that compare both approaches?
• In a similar fashion we could look into replacing someString.indexOf(someOtherString) === 0 with someString.startsWith(someOtherString). Perhaps we're also using some other pattern of indexOf that can be replaced with endsWith?

edit: Just doing a regex search (.indexOf\([^)]) and it seems like I can still find occurences not covered by this PR

[romgrk]

Looking through the changes here I don't think any of them is important enough to worry about the performance of those methods.

[Janpot]

Looking through the changes here I don't think any of them is important enough to worry about the performance of those methods.

Absolutely, and my expectation is that there won't be any significant impact, It's not a blocker, I'm just curious.
A while ago I saw some benchmarks for the startsWith case that suggested it's slower than indexOf(...) === 0 which is really counterintuitive imo. Haven't dug into this yet to find out what's going on there though, broken benchmark or something I misunderstand about js?

[aarongarciah]

@Janpot

Are we aware of any microbenchmarks that compare both approaches?

The general discourse seems that .includes might be a bit slower, but the difference is so tiny that it's not worth caring about. These are old links but probably still apply:

• lib: prefer using Array#includes instead of Array#indexOf nodejs/node#26568 (comment)
• https://stackoverflow.com/a/45264728
• https://ryanpeden.com/javascript-indexof-vs-includes-performance

In a similar fashion we could look into replacing someString.indexOf(someOtherString) === 0 with someString.startsWith(someOtherString).

Great idea. @k-rajat19 could you update the PR converting someString.indexOf(someOtherString) === 0 to someString.startsWith(someOtherString). You can do a regex search to find all occurrences. I found 20 of them.

Perhaps we're also using some other pattern of indexOf that can be replaced with endsWith?

I did a quick search and couldn't find this pattern.

[aarongarciah]

Hey @k-rajat19, are you available to make the requested changes?

[k-rajat19]

Sorry I forgot about this, I will update it soon

[aarongarciah]

Thanks @k-rajat19! As @Janpot pointed out there are lots of .indexOf occurrences not covered by this PR. We can merge this one and if you are up for it, feel free to open an additional PRs to cover all occurrences.