w3c / aria-at-app

Test runner and results reporter for ARIA-AT
http://aria-at.w3.org/
Other
35 stars 15 forks source link

feat: Use standardized gql fragments in queries.js files #1147

Closed howard-e closed 1 month ago

howard-e commented 2 months ago

To supersede #876.

I've deviated from the original suggestion of:

I'm down with this. Most fields don't impact query performance so it's totally fine to just query all of them every time. For fields that do impact performance, like tests on TestPlanVersions and testResults on TestPlanRuns, I think it should be divided into two fragments to make accidentally tanking your performance less likely to happen. Something like TEST_PLAN_RUN_FIELDS and TEST_PLAN_RUN_TEST_RESULTS_FIELDS.

I think fields like renderableContent might be preferable to leave off, since they're extremely verbose and "specialty" for specific use cases. If it's there where it's not needed it will definitely make debugging responses harder.

We can manually add them where they're needed.

Mainly because having significantly populated queries per page even if the performance hit was marginal (significant in some cases) became somewhat noisy.

So I did my best to identify common enough existing attributes being requested per page and included them in their own respective client/components/common/fragments/TYPE.js file. Where applicable, I created 2 separate queries for the graphql type (simple and all).

Another oddity about this work is there is an inconsistency between the testPlanVersionResolver and testPlanVersionsResolver which makes using the ...TestPlanVersion fragment throw errors for some queries. It may not be worth resolving now due to the current size of the PR and could be follow up task if it becomes an issue.

This PR is still missing several tests for components to feel confidence in this change, but I would rather not balloon this PR more. In discussing this with @stalgiag, they noted that the missing tests can be done in a follow up PR. So this shouldn't be merged until that work is done (so leaving as draft for now).