fix(workspace): stop table tabs from re-browsing on every switch - #2961
Merged
Merged
Conversation
A table data tab loads its first page from an effect that depended on the `viewTableParams` object. The workspace tab layer keeps every open tab mounted and rebuilds the tab bodies whenever unrelated workspace state changes (active tab, datasource list, tab list), so that object was new on most parent renders. React compares effect dependencies by reference, so every tab switch re-issued the browse request for every open table tab and pushed the visible page back to the first one. Load on the table identity (datasource, database, schema, table) and read the latest params from a ref instead. `workspaceTabItems` also no longer depends on the active tab id, which it never read, so switching tabs no longer rebuilds every tab body.
Follow-up to the tab-switch load fix, from the review of that change: - Loading is now keyed on the table identity, so a cancelled or failed first page no longer recovers on its own. Surface that state inside the tab and give it a retry button instead of leaving the tab blank until it is reopened. - Add the missing rejection handler for the initial browse request, which was an unhandled rejection before. - `changeTabDetails` is captured by the memoized tab bodies, which are no longer rebuilt on a tab switch, so let `setWorkspaceTabsState` read the current split layout from the store instead of writing back an older one. - Document the effect-order invariant and the paging exception on the target key, and cover the database type and empty identity fields in its test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
N/A — no tracking issue exists for this defect. Found while manually testing the Community dev desktop: switching between two open table data tabs re-issued the browse request every time and pushed the result back to page 1.
Summary
A table data tab (
ViewTable) started its first-page load from an effect whose dependency was the wholeviewTableParamsobject. The workspace tab layer keeps every open tab mounted and rebuilds the tab bodies whenever unrelated workspace state changes: the active tab id is a dependency ofworkspaceTabItems(WorkspaceTabs/index.tsx:1991before this change), the workspace store is subscribed through an object selector (WorkspaceTabs/index.tsx:787), and the tab body elements are recreated from that memo. React compares effect dependencies by reference, so the params object was new on most parent renders, and every tab switch re-issuedviewTablefor every open table tab and reset the visible page to the first one.The fix keys the load on the table identity (datasource, database, schema, table) and reads the latest params from a ref, so a rebuilt params object no longer looks like a new table.
workspaceTabItemsalso no longer depends onactiveConsoleId, whichgetWorkspaceTabItemsnever read, so a tab switch no longer rebuilds every tab body.Review of that change found two consequences, fixed in the second commit:
ViewTablenow catches the rejected browse (previously an unhandled rejection) and shows a retry affordance in the tab instead of staying blank until the tab is reopened.changeTabDetailsis captured by the memoized tab bodies, which are no longer rebuilt on a tab switch, so it could write back the split layout of an earlier render and revert a pane's active tab. It now letssetWorkspaceTabsStateread the current layout from the store.Affected surfaces
The changed client is shared by Web, the JCEF desktop renderer, and the Pro/Studio composition; no packaging or bridge behavior changed. Two user-facing strings were added to all five locale catalogs.
Verification
NODE_ENVunset because the local shell exportsproduction):yarn run test:i18n→Validated es-ES and ko-KR against 19 frontend modules, 2 properties bundles, and 5 READMEs.(source hashes regenerated with--write-source-hashesfor the two new keys)yarn run test:view-table-target(new script, also appended toprebuild:web:community) →view table target tests passedyarn run lint(eslint src/**+stylelint src/**,--max-warnings=0) → exit 0yarn run build:web:community --app_version=0.0.0→ full prebuild test chain,Webpack: Compiled successfully,verify-production-bundles.cjspassedgit diff --check→ clean; CI on this PR is green for Frontend, Backend, updater, repository checks, dependency review, license summary, and SBOM.127.0.0.1:8889), switching betweenapp.public.chat2db_ordersandapp.public.chat2db_geo_testno longer re-issues the browse request and no longer resets the page.ViewTablepulls in the full result-set stack, so the regression test pins the table-identity key contract instead of asserting request counts on a rendered tab; the no-refetch behavior and the retry affordance were confirmed manually.Risk and compatibility
viewTablerequest is sent, just not spuriously repeated.blocks/SearchResult/components/ResultSet/index.tsx:295, paging, the import-target refresh event, and reopening the tab). A cancelled or failed first page now shows an in-tab retry affordance where it previously recovered silently on the next tab switch.Reviewer map
src/components/ViewTable/index.tsx— the load effect depends ongetViewTableTargetKey(viewTableParams), params come fromviewTableParamsRef, and the rejected browse is caught into the new retry state; the key contract lives insrc/components/ViewTable/viewTableTarget.ts.src/pages/main/workspace/components/WorkspaceTabs/index.tsx—workspaceTabItemslost the unusedactiveConsoleIddependency, andchangeTabDetailsnow reads the current layout from the store.src/i18n/*/common.ts(common.button.retry,common.text.tableDataNotLoaded) and the regeneratedscripts/i18n-source-hashes.json.getWorkspaceTabItems, or an in-tab paging caller drivingviewTableParams.pageNowould reintroduce stale or repeated loads; the unit test pins the key contract.Contributor declaration
AI assistance: the root-cause analysis, patch, review follow-ups, and tests were produced with an AI coding agent, then reviewed and manually verified in the Community dev desktop by the maintainer before this PR.