Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: Network Request Refactor #18741

Closed
mrtracy opened this issue Sep 25, 2017 · 1 comment
Closed

ui: Network Request Refactor #18741

mrtracy opened this issue Sep 25, 2017 · 1 comment
Assignees
Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. no-issue-activity X-stale

Comments

@mrtracy
Copy link
Contributor

mrtracy commented Sep 25, 2017

The network request API in the Admin UI is not particularly intuitive and is not scaling well. An overview of current issues:

CachedDataReducer API

CachedDataReducer has an unintuitive API for refreshing data: React components that need routinely-refreshed data (e.g. refresh some data every 10 seconds) must dispatch a "refresh" thunk action inside of both componentDidUpdate and componentWillReceiveProps. The intention is that periodically fetched data will only be fetched only if there is a currently rendered component which needs it. However, this strategy is often unclear to new developers, requires a hack to work correctly (requiring the valid flag to be a connected property), is difficult to debug, and is providing confusion in use cases which do not require data to be periodically refreshed.

Additionally, the use of CachedDataReducer as a "reducer factory", while reducing boilerplate, does seem to introduce some conceptual complexity; it is also only one possible solution to re-using the cached data pattern across multiple endpoints. We might investigate the use of type-safe accessors (the pattern currently used for local settings) instead of directly typing the repository.

Project Organization

Currently, the endpoint definitions for our API are under /util/api.ts, while the actual cached data reducers are defined under /redux/apiReducers. Things are basically all over the place; even if we don't change the underlying pattern much, we should reorganize this structure to be more straightforward.

@mrtracy mrtracy added this to the 1.2 milestone Sep 25, 2017
@mrtracy mrtracy self-assigned this Sep 25, 2017
@mrtracy mrtracy modified the milestones: 2.0, 2.1 Feb 12, 2018
@knz knz added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. and removed refactoring labels Apr 24, 2018
@couchand couchand added the A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category. label Apr 24, 2018
@petermattis petermattis removed this from the 2.1 milestone Oct 5, 2018
@knz knz assigned piyush-singh and unassigned mrtracy Jan 13, 2019
@github-actions
Copy link

github-actions bot commented Jun 8, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. no-issue-activity X-stale
Projects
None yet
Development

No branches or pull requests

5 participants