fix: avoid re-fetching the page of posts embedded in the initial page load#4703
Open
ekumanov wants to merge 2 commits into
Open
fix: avoid re-fetching the page of posts embedded in the initial page load#4703ekumanov wants to merge 2 commits into
ekumanov wants to merge 2 commits into
Conversation
… load Since flarum#4156, DiscussionPage.show() constructs PostStreamState without seeding it, so every cold discussion-page load re-fetches /api/posts?filter[discussion]=N&page[near]=M — data the preloaded document already contains. The request is serialized between the JS boot and the first post paint, adding a full API round-trip to the LCP critical path of every cold discussion view. Restore the 1.x seeding, scoped so it cannot reintroduce flarum#4137: only the preloaded-document path seeds the stream, and only with the longest run of store-loaded posts contiguous in postIds() order, so stray posts included via other relationships fall through to the normal fetch. Fixes flarum#4702 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Includes transpiled JS/TS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Fixes #4702
Changes proposed in this pull request:
Since #4156,
DiscussionPage.show()constructsPostStreamStatewithout seeding it, so on every cold discussion-page load the stream re-fetchesGET /api/posts?filter[discussion]=N&page[near]=M— data thatContent\Discussionalready embedded in the preloaded document (it needs it for the noscript content anyway) and thatpreloadedApiDocument()has already pushed into the store. The request is serialized after the JS boot and before the first post can paint, so it adds a full API round-trip (typically 0.5–1.5 s for real visitors) to the LCP critical path of every cold discussion view. 1.x rendered straight from the payload.This PR restores the seeding, scoped so it cannot reintroduce #4137:
load()passesthis.preloadedNearPage(preloadedDiscussion)toshow()only on the preloaded-document path. API show responses (post-refactor: remove listing of posts in the show discussion endpoint #4067) embed no posts page, so for in-app navigationpreloadedNearPage()returns[]and the stream fetches exactly as before.preloadedNearPage()returns the longest run of store-loaded posts contiguous inpostIds()order. Stray posts pulled in by other relationships (e.g. extension includes — real production payloads do contain them) therefore can't corruptvisibleStart/visibleEnd, which was the [2.0.0-beta.1] Opening a discussion not always loads all posts #4137 failure mode that fix: discussion posts not always properly loaded #4156 fixed by removing the old extraction.show()accepts the seed as an optional second parameter (default[]), so existing callers and subclasses are unaffected.nearparam is present, the pop-in target falls back to the first preloaded post's number (matching 1.x), then to 1.Reviewers should focus on:
preloadedNearPage()as the guard against [2.0.0-beta.1] Opening a discussion not always loads all posts #4137.show()signature extension (optional param) for compatibility with extensions overridingshow().Measured impact (production rc.3 forum, 41 extensions; details in #4702):
QA performed (production mirror with real data):
/d/{id}/{slug}/400deep link: window around post 400 rendered with no posts request.Necessity
Confirmed