Added a bunch of these eslint-disable-next-line rules in preparation for the exhaustive-deps rule (see #41166)

@sarayourfriend , WDYT about the current rtl.watch() approach? Asking because, with the new eslint rule, we may need to have to "ignore" the rule a lot of the times, which defeats the point a little bit

(cc @chad1008 )

It does defeat the purpose of adding the exhaustive-deps rule if all components should eventually support RTL. In the interim though probably better to catch a few oversights in the dependency arrays.

I agree. Ignoring the rule for now is a lesser evil compared to not adding the eslint rule at all. Will keep it as is for now

Want to confirm I'm following the logic here :)

rtlWatchResult isn't actually called in the useMemo's create function, so it's not technically a dependency from React's POV.

We do, however, want to recalculate classes whenever rtlWatchResult changes, so we add it to the dependency array to trigger the useMemo, which exhaustive-deps will hate for the reason described above?

Correct.

A couple of alternative approaches that come to mind:

• we could make these "dynamic style" functions accept, as an argument, a isRTL boolean (for example: return cx( styles.borderBoxControlSplitControls( rtlWatchResult ), className );). This would make the computation of RTL styles less "auto-magical", which isn't necessarily a bad thing since we've been missing a few of those!
• create a hook that forces a re-render of the component when the writing direction changes, and therefore remove it from the list of dependencies of useMemo

Thoughts?

The first option sounds, to me, like it might make things a little more intuitive, but I'd happily defer to others who've worked more with the current auto-magical approach 😄

I feel like we've discussed this before so sorry if I'm forgetting a crucial piece of the convo, but do we actually have a good reason to memoize these things at all? I searched around but couldn't really find anything that mentioned any performance issues that were observed. The Emotion folks also don't recommend blanket optimizations like this.

That is also a good question. From what I remember, the reason why we memoize Emotion classname computation is because of a test ran by Q before I joined the components squad, where he found out that computing these classnames can be quite expensive.

Since then, quite some time has passed and, with that, libraries (like Emotion and React) also got updated. Therefore, I'm not sure if this assumption still holds true. We should run some tests and take it from there

It does defeat the purpose of adding the exhaustive-deps rule if all components should eventually support RTL. In the interim though probably better to catch a few oversights in the dependency arrays.