apollographql / apollographql/fullstack-tutorial

Launches pagination implementation is a bad example for reference

Đang mở
#78 1 bình luận 1 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
TypeScript
Star
1.2k
Fork
808
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

The GraphQL server-side cursor pagination example should be discarded. While the intent of the `paginateResults` function in `final/server/src/utils.js` is good, there are a number of significant flaws with this implementation.

The lowest hanging fruit is that there is a line of code at the end of the function - `results.slice(cursorIndex >= 0 ? cursorIndex + 1 : 0, cursorIndex >= 0);` which is unreachable.

However, the most egregious error is how the `launches` function is defined in `final/server/src/resolvers.js` - which gives the impression that data is being paged responsibly. It is **not**

Every time pagination is invoked, the API request loads ALL THE DATA from the API and then passes it to the pagination function. For the example SpaceX data, there are approximately 76 results that get loaded and then processed to return the 20 results the user is expecting:

```
Query: {
launches: async (_, { pageSize = 20, after }, { dataSources }) => {
const allLaunches = await dataSources.launchAPI.getAllLaunches();
// we want these in reverse chronological order
allLaunches.reverse();

const launches = paginateResults({
after,
pageSize,
results: allLaunches,
});

return {
launches,
cursor: launches.length ? launches[launches.length - 1].cursor : null,
// if the cursor of the end of the paginated results is the same as the
// last item in _all_ results, then there are no more results after this
hasMore: launches.length
? launches[launches.length - 1].cursor !==
allLaunches[allLaunches.length - 1].cursor
: false,
};
},
...
```

If that API endpoint were to return a massive amount of data (imagine 40,000 results), this resolver would load all 40,000 results and then find 20 to return. When the user would click load more, all 40,000 results would be loaded into memory again and then the next 20 results would be returned...effectively processing 80,000 results to display 40 to the user. EEK!

This is definitely something that should be revisited. Imagine a scenario with 30 users following a simple workflow of viewing the first 20 results and then loading an additional 20 results.

Additionally, if the result set grew (imagine 40,001 records), the newest record would never be displayed to the user because it would have been in a spot the cursor presumably has already visited, right?

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Hướng nghiên cứu

Đọc final/server/src/utils.js và final/server/src/resolvers.js, tập trung vào paginateResults và Query.launches; theo dõi cách getAllLaunches cung cấp dữ liệu cho từng request. Xác định hành vi phân trang dự kiến và xác minh rằng trang đầu tiên và các trang tiếp theo vẫn giữ nguyên ngữ nghĩa của cursor, pageSize, ordering và hasMore mà không tải toàn bộ tập kết quả.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
graphql, javascript, node.js
Lĩnh vực
backend-api-design, performance
Loại issue
Tái cấu trúc
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Cần làm rõ
Mức phù hợp với người mới
30/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.