refactor(sqllab): Remove tableOptions from redux state

[justinpark]

SUMMARY

Similar to #23257, this commit migrates the tableOptions state from redux to react-query to reduce the complexity of the queryEditor state.
(The tableOptions state only used in the editor for autocomplete so useQuery is a lighter way to share the state)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After:

Before:

TESTING INSTRUCTIONS

Go to sqllab and select a database to get the associated schemas
Type some part of the table name on the editor and check the autocomplete including the fetched schemas

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

[codecov]

Codecov Report

Merging #23488 (1346107) into master (976e333) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 1346107 differs from pull request most recent head eab204a. Consider uploading reports for the commit eab204a to get more accurate results

@@            Coverage Diff             @@
##           master   #23488      +/-   ##
==========================================
+ Coverage   68.08%   68.10%   +0.02%     
==========================================
  Files        1920     1920              
  Lines       73984    73978       -6     
  Branches     8092     8092              
==========================================
+ Hits        50374    50385      +11     
+ Misses      21539    21518      -21     
- Partials     2071     2075       +4     

Flag	Coverage Δ
javascript	54.14% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/SqlLab/actions/sqlLab.js	64.32% <ø> (-0.18%)	⬇️
...frontend/src/SqlLab/components/SaveQuery/index.tsx	75.55% <ø> (ø)
...d/src/SqlLab/components/SqlEditorLeftBar/index.tsx	53.73% <ø> (-0.56%)	⬇️
superset-frontend/src/SqlLab/fixtures.ts	100.00% <ø> (ø)
superset-frontend/src/SqlLab/reducers/sqlLab.js	37.28% <ø> (+0.20%)	⬆️
superset-frontend/src/SqlLab/types.ts	57.14% <ø> (ø)
...et-frontend/src/components/TableSelector/index.tsx	78.37% <ø> (-0.85%)	⬇️
...d/src/SqlLab/components/AceEditorWrapper/index.tsx	66.27% <100.00%> (+2.42%)	⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[justinpark]

@michael-s-molina could you help the code review?

[eschutho]

Hi @justinpark! What are plans long term for redux here? Are you looking to remove it completely and replace it with react-query or just for select places? I know for a while now we've had state in both redux and in hooks, and haven't ever come to a decision on where frontend state should reside.

[justinpark]

Hi justinpark! What are plans long term for redux here? Are you looking to remove it completely and replace it with react-query or just for select places? I know for a while now we've had state in both redux and in hooks, and haven't ever come to a decision on where frontend state should reside.

@eschutho As I state in #21240, the long term plan is replacing existing api related state (queries, tables, validationResult and functionNames in queryEditors) in redux store to a hook based state (using react-query was the first proposal and finally it will migrate to redux toolkit query (which is a similar solution of react-query) once #23460 landed.). Redux will be remained the editor's state(queryEditors, tabHistory, alerts, unsavedQueryEditor) that is only made by the user inputs.

The benefit of this redux cleanup work is

1. Can reduce many similar action/reducer logic for fetching state (as shown in this PR) and easier way to access the date by provided helpers
2. Implicitly avoid the duplicate api request (i.e. shown examples in refactor: introduce react-query on api resource hook #21240)
3. consolidate/manage the result data in a single place

Please let me know if you have any concerns with this redux cleanup work

[michael-s-molina]

@justinpark @eschutho I would like to add my 2 cents about the topic. The motivation for using react-query/RTK Query is the following (copied from RTK website):

Over the last couple years, the React community has come to realize that "data fetching and caching" is really a different set of concerns than "state management". While you can use a state management library like Redux to cache data, the use cases are different enough that it's worth using tools that are purpose-built for the data fetching use case.

Originally @justinpark was planning to use react-query to manage data fetching and caching. Given that RTK has a tool in the same domain called RTK Query, I advised to use that instead to preserve library compatibility.

Hi @justinpark! What are plans long term for redux here? Are you looking to remove it completely and replace it with react-query or just for select places? I know for a while now we've had state in both redux and in hooks, and haven't ever come to a decision on where frontend state should reside.

RTK Query is built on top of Redux as you can see in another quote from the same RTK website:

The data fetching and caching logic is built on top of Redux Toolkit's createSlice and createAsyncThunk APIs

What RTK Query does is to automatically generate the actions, reducers, slices, and hooks to manage data fetching and caching. So it's a complement to Redux, not a replacement.

@justinpark volunteered to convert existing react-query hooks to RTK Query hooks and remove the react-query dependency.

@justinpark I would advise to complete some steps before further modifying existing Redux state:

• Merge existing PR to introduce Redux Toolkit
• Go back to previous PRs that used react-query and replace them with RTK Query
• Remove react-query dependency
• Before removing any more state from the Redux store, I would model the endpoints based on the API itself (not its current use), to correctly define the parameters and return types, which would give us one of the best benefits of using RTK Query and Typescript: API type safety.

Lastly, thank you so much @justinpark for driving this effort. It's a really nice improvement, long overdue, that will really simplify our use of Redux and introduce many quality checks during development.

[michael-s-molina]

Given that @justinpark will remove the dependency of react-query in a follow-up, this PR LGTM as the API will remain pretty similar.

[michael-s-molina]

RTK Query has a (currently experimental) code-gen tool that will take an OpenAPI spec or GraphQL schema and give you a typed API client, as well as provide methods for enhancing the generated client after the fact.

@justinpark Maybe this tool can help us generate the API given that we use OpenAPI.

[justinpark]

justinpark Maybe this tool can help us generate the API given that we use OpenAPI.

I will take a look compatibility with fab API