Skip to content

fix: avoid re-fetching the page of posts embedded in the initial page load#4703

Open
ekumanov wants to merge 2 commits into
flarum:2.xfrom
ekumanov:fix/discussionpage-seed-preloaded-posts
Open

fix: avoid re-fetching the page of posts embedded in the initial page load#4703
ekumanov wants to merge 2 commits into
flarum:2.xfrom
ekumanov:fix/discussionpage-seed-preloaded-posts

Conversation

@ekumanov

Copy link
Copy Markdown
Contributor

Fixes #4702

Changes proposed in this pull request:

Since #4156, DiscussionPage.show() constructs PostStreamState without seeding it, so on every cold discussion-page load the stream re-fetches GET /api/posts?filter[discussion]=N&page[near]=M — data that Content\Discussion already embedded in the preloaded document (it needs it for the noscript content anyway) and that preloadedApiDocument() 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() passes this.preloadedNearPage(preloadedDiscussion) to show() 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 navigation preloadedNearPage() returns [] and the stream fetches exactly as before.
  • preloadedNearPage() returns the longest run of store-loaded posts contiguous in postIds() order. Stray posts pulled in by other relationships (e.g. extension includes — real production payloads do contain them) therefore can't corrupt visibleStart/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.
  • When no near param 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:

Measured impact (production rc.3 forum, 41 extensions; details in #4702):

  • Docker mirror of the production site (same machine/DB, only the bundle changed): the posts request is eliminated on cold loads; no functional change.
  • Production, same host, same day: Lighthouse mobile (simulated Slow 4G) LCP on a discussion page 7.7 s → 6.2 s (median of 3); one fewer API request per cold discussion view.

QA performed (production mirror with real data):

  • Cold landing on a 4-post and a 749-post discussion: no posts request, correct window, scrubber total correct.
  • /d/{id}/{slug}/400 deep link: window around post 400 rendered with no posts request.
  • Non-contiguous extension include in the payload: correctly excluded by the contiguity rule.
  • In-app navigation from the index: still fetches (discussion show + posts near) and renders, preserving [2.0.0-beta.1] Opening a discussion not always loads all posts #4137 behavior.
  • Event posts (e.g. discussion renamed) inside the seeded window render fine; no console errors anywhere.

Necessity

Confirmed

  • Frontend changes: tested on a local Flarum installation (and a production mirror, see QA above).
  • Backend changes: n/a — no backend changes.

ekumanov and others added 2 commits June 10, 2026 21:23
… 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>
@ekumanov ekumanov requested a review from a team as a code owner June 10, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant