Components: refactor NavigatorScreen to pass exhaustive-deps

[chad1008]

What?

Updates the NavigatorScreen component pass the exhaustive-deps eslint rule.

Why?

Part of the effort in #41166 to apply exhuastive-deps to the Components package

How?

Adding location.isBack and previousLocation?.focusTargetSelector to the useEffect deps array.

This does cause some additional renders on navigation between screens, but they don't appear problematic in my tests. I think they're happening because each time you navigate the locationHistory changes. This triggers an update to the goTo and goBack values that are being drawn from context. This, in turn, causes an update to the context value as a whole (goTo and goBack both appear in the useMemo dep array for navigatorContextValue). Updating the navigatorContextValue means that even if it didn't change on this particular render, all location values are being redeclared, which will trigger a re-render thanks to our new dependencies.

All of that said, I know Navigator is your brainchild @ciampo so please let me know if that logic sounds flawed! 🙂

Testing Instructions

1. From your local Gutenberg directory, run npx eslint --rule 'react-hooks/exhaustive-deps: warn' packages/components/src/navigatior/navigator-screen
2. Confirm that the linter returns no errors or warnings
3. Confirm navigator unit tests still pass
4. Run Storybook locally, confirm the Navigator component stories and/or docs still work as expected

[ciampo]

• ✅ Code changes make sense
• ✅ Unit tests are passing
• ✅ Storybook example behaves as expected
• ✅ Global Styles sidebar in the Site editor works as expected

This does cause some additional renders on navigation between screens, but they don't appear problematic in my tests.

Does it cause more component re-renders, or just that useEffect to run more often?

Also, I wonder if we should add an if statement around elementToFocus.focus(); and make sure that we only focus if document.activeElement !== elementToFocus ?

[chad1008]

Does it cause more component re-renders, or just that useEffect to run more often?

Good point. I was conflating the two. Upon closer inspection, before this change there were four re-renders and two useEffect runs per navigation. With this change there are still four re-renders, but now there are four useEffect runs. So it isn't re-rendering more, it's just running the effect every time because it's catching the changes in location.

Also, I wonder if we should add an if statement around elementToFocus.focus(); and make sure that we only focus if document.activeElement !== elementToFocus ?

Makes sense! No need to focus if the desired element is already active.

I'll put a followup PR together 👍