Notes: Stream note loading progressively on large posts#80018
Open
adamsilverstein wants to merge 6 commits into
Open
Notes: Stream note loading progressively on large posts#80018adamsilverstein wants to merge 6 commits into
adamsilverstein wants to merge 6 commits into
Conversation
The collab sidebar fetches every note for a post in a single `per_page: -1` query and blocks rendering until the full set has loaded. This does not scale to posts with hundreds or thousands of notes. Pair the query with the core-data RECEIVE_INTERMEDIATE_RESULTS flag so the resolver streams the pages (100 records at a time) into the store as they arrive, letting the sidebar render notes progressively instead of waiting for the whole collection. This mirrors the pattern already used by the hierarchical term selector. Addresses #71668.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +54 B (0%) Total Size: 7.73 MB 📦 View Changed
|
The notes query now streams pages via RECEIVE_INTERMEDIATE_RESULTS, so the core-data resolver paginates the request itself with an explicit `page` param instead of leaning on api-fetch's fetch-all middleware. The startup request the preload specs snapshot therefore gains `&page=1`. Update the expected route in the post-editor and site-editor preload specs to match.
|
Flaky tests detected in 758f234. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/29759857390
|
Seed a post with 105 notes, hold the second page of the notes query via route interception, and assert the first 100 threads render in the All notes sidebar before the full set has loaded, then release the request and assert all 105 render. Without RECEIVE_INTERMEDIATE_RESULTS the store only receives records once every page has resolved, so this test fails when the flag is removed (verified locally red/green).
adamsilverstein
added
the
[Type] Bug
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
1 participant
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.
What?
Adds the core-data
RECEIVE_INTERMEDIATE_RESULTSflag to the notes query inuseNoteThreads, so notes stream into the collab sidebar page-by-page as they load instead of the sidebar blocking until the entire collection has been fetched.Fixes #71668.
Why?
The collab sidebar fetches every note for a post in a single
per_page: -1query. Without the intermediate-results flag, the core-data resolver resolves that query in one shot and only populates the store once the full set has loaded, so the sidebar shows nothing until the last record arrives. On posts with hundreds or thousands of notes that is a long, blank wait.How?
per_page: -1combined withRECEIVE_INTERMEDIATE_RESULTS: trueswitches the resolver to its paginated branch: it fetches 100 records per page and dispatchesreceiveEntityRecordsafter each page. BecauseuseEntityRecordsis reactive, the sidebar re-renders as each page lands and notes appear progressively.This is the same pattern already used by the hierarchical term selector (
packages/editor/src/components/post-taxonomies/hierarchical-term-selector.js). The flag is aSymbol, so it does not leak into the request URL or affect the query cache key.This is the low-risk, incremental first step discussed in #71668. It does not attempt the larger "render top-level notes first, then replies" or lazy-load-on-expand redesigns, which remain worth pursuing for the full scaling story (see the issue discussion).
Testing Instructions
Setup: seed a post with many notes
The streaming only kicks in past 100 notes (the resolver pages at 100 per request), so the post needs 100+ notes. The easiest way to get there is @t-hamano's Notes Data Generator test plugin (dev sites only): install and activate it, open a post with a few blocks, and use its panel in the editor sidebar to generate notes from random users, content, and dates. Adding notes by hand across several blocks works too, just slower.
Verify progressive loading
comments: you should see sequential?...&type=note&...&per_page=100&page=1,page=2, ... requests instead of oneper_page=-1request.Automated test
test/e2e/specs/editor/various/block-notes-progressive-load.spec.jscovers the streaming behavior directly: it seeds a post with 105 notes, holds the second page of the notes query via route interception, and asserts the first 100 threads render in the sidebar before the full set has loaded (then releases the request and asserts all 105 render). Without the flag the store only receives records once every page has resolved, so the test fails on trunk.Alternatives considered
Discussed in #71668:
_embed=childrento fetch first-level replies in one query, and lazy-loading replies only when a thread is expanded. Both are stronger for the extreme "thousands of notes" case but require more work (and likely a small REST change for embedding). This PR ships the immediate win; those can follow.