fix(sqllab): excessive API calls for schemas

[justinpark]

SUMMARY

Currently, the refresh logic for schemas is being triggered excessively due to unnecessary dependencies, causing it to be called every time a tab is switched.

This commit addresses the issue by modifying the logic to execute only when a trigger is needed.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

before--too-many-schemas.mov

After:

after--too-many-schemas.mov

TESTING INSTRUCTIONS

Switch the tabs between the same db selected tabs
Check the network tab and then only schemas called once

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[michael-s-molina]

Thanks for the PR @justinpark!

[michael-s-molina]

There's a lot of duplicated code. Maybe something like this?

export function useCatalogs(options: Params) {
  const { dbId, onSuccess, onError } = options || {};
  const [trigger] = useLazyCatalogsQuery();
  const result = useCatalogsQuery(
    { dbId, forceRefresh: false },
    {
      skip: !dbId,
    },
  );

  const fetchData = useCallback(
    (forceRefresh: boolean) => {
      trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => {
        if (isSuccess) {
          onSuccess?.(data || EMPTY_CATALOGS, forceRefresh);
        }
        if (isError) {
          onError?.();
        }
      });
    },
    [dbId, onError, onSuccess, trigger],
  );

  const refetch = useCallback(() => {
    if (dbId) {
      fetchData(true);
    }
  }, [dbId, fetchData]);

  useEffect(() => {
    if (dbId && !result.currentData) {
      fetchData(false);
    }
  }, [dbId, fetchData, result.currentData]);

  return {
    ...result,
    refetch,
  };
}

[justinpark]

That's a good suggestion. One thing that including result.currentData in the dependencies array of useEffect can cause unintended fetchData calls. I'll change it to handle this within the fetchData function.

[michael-s-molina]

Same as before.