Skip to content
This repository has been archived by the owner on Sep 26, 2024. It is now read-only.

refactor: Move queries to backend #774

Merged
merged 10 commits into from
Nov 19, 2021
Merged

Conversation

shelegdmitriy
Copy link
Contributor

@shelegdmitriy shelegdmitriy commented Nov 4, 2021

Resolves #773
We had a mess with queries as some of them was on backend but huge amount of them on frontend

@shelegdmitriy shelegdmitriy requested a review from frol November 4, 2021 16:31
@shelegdmitriy shelegdmitriy self-assigned this Nov 4, 2021
@render
Copy link

render bot commented Nov 4, 2021

@shelegdmitriy shelegdmitriy force-pushed the refactor/move_queries_to_backend branch from ef7310b to fbfad2d Compare November 5, 2021 09:16
@shelegdmitriy shelegdmitriy added the refactoring code, configuration clean ups label Nov 5, 2021
@shelegdmitriy shelegdmitriy marked this pull request as ready for review November 5, 2021 09:23
@shelegdmitriy
Copy link
Contributor Author

@frol some tests have failed. That because backend isn't deployable for tests. I've tested it manually and it worked fine

frontend/src/components/blocks/Blocks.tsx Outdated Show resolved Hide resolved
frontend/src/libraries/explorer-wamp/blocks.ts Outdated Show resolved Hide resolved
Comment on lines 27 to 30
return await this.call<BlocksListInfo[]>("blocks-list", [
limit,
paginationIndexer,
]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to bring it to your attention that we used to name fields going through WAMP-proto in snake_case and used this explorer-wamp module to actually map the values to the proper types (usually BN, Date) and naming convention.

I cannot say I have a very strong opinion about the naming convention (yet, I would vote for snake_case in JSON payloads), but I certainly believe we should convert the data into a proper type in explorer-wamp adapters.

Actually, I believe I did a poor job with call adapter, which makes a false impression that it will do something with the received data to match the specified type while in fact it just silently casts whatever received value to the specified type, which is naive. call should always return unknown type and thus force the user of the adapter to validate the received value and map it over to proper type. Something along these lines: https://github.com/near/near-explorer/blob/master/frontend/src/context/NetworkStatsProvider.tsx#L47-L61 (yet, we already broke the naming convention there, and it also leaked the abstraction from explorer-wamp, by subscribing to the events directly in the Context implementation)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does I need to move logic from /backend to /explorer-wamp and convert to proper types? Or I just need to pass variables in snake_case there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean in /backend/blocks.js for example I map through the values and convert values to proper types (mostly to number only) is it correct or I should do it in explorer-wamp/blocks.tsx file?

frontend/src/pages/accounts/[id].jsx Outdated Show resolved Hide resolved
backend/src/accounts.js Outdated Show resolved Hide resolved
backend/src/db-utils.js Outdated Show resolved Hide resolved
backend/src/db-utils.js Outdated Show resolved Hide resolved
backend/src/db-utils.js Outdated Show resolved Hide resolved
backend/src/db-utils.js Outdated Show resolved Hide resolved
backend/src/db-utils.js Outdated Show resolved Hide resolved
backend/src/chunks.js Outdated Show resolved Hide resolved
@frol frol merged commit e106fed into master Nov 19, 2021
@frol frol deleted the refactor/move_queries_to_backend branch November 19, 2021 18:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactoring code, configuration clean ups
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize the computation of total transaction count for each account
2 participants