fix(widgets): avoid duplicate queries and repeated archive date formatting - #3068
Open
logelog wants to merge 2 commits into
Open
fix(widgets): avoid duplicate queries and repeated archive date formatting#3068logelog wants to merge 2 commits into
logelog wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: bdafeee The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The PR's three changes are well-scoped and fit EmDash's patterns: a real duplicate-query bug during concurrent widget-area reads, wasted date-formatting work in the Archives widget, and a perf-harness bug that failed to exercise the HTML prefetch path. I traced each change against the relevant request-cache, middleware, and test code.
What I checked:
- Read the full diff and the changed source files (
packages/core/src/widgets/index.ts,archives.ts,astro/prefetch.ts,infra/perf-monitor/probe/src/measure.ts,scripts/query-counts.mjs) pluspackages/core/src/request-cache.tsand the middleware. - Verified
prefetchLayoutDatais called only for HTML navigations and is wrapped viaafter(), so the newrequestCached("widget-areas", getWidgetAreas)sharing pluspeekRequestCacheingetWidgetArea(name)correctly collapses concurrent named reads onto the in-flight bulk load. - Confirmed
groupEntriesByPublishedAtstill groups/counts over the full input but only callstoLocaleDateStringafter slicing to the display limit. - Checked the D1 query-count snapshot increase against the harness code: the old harness sent no
Acceptheader, so EmDash skipped its HTML prefetch; the newtext/htmlheader makes the snapshot reflect what browser requests already do. The SQLite snapshot is unchanged, which is consistent with prefetch running only on remote adapters. - Reviewed the two changesets; both are present-tense, name the user-facing surface, and explain the observable behavior.
- Inspected the new tests. The widget-request-cache and archive formatting tests reproduce the claimed regressions; the perf-monitor test verifies the probe now requests HTML.
I found no logic bugs, security issues, convention violations, or missing tests. The D1 snapshot change is a measurement correction, not new production queries, and the code reduces real logged-out work on remote adapters.
One minor note: the perf-monitor probe test lives under packages/core/tests/unit/ because the infra/perf-monitor package has no test script. That is pragmatic, but if the infra package later grows its own vitest config this test should move there. Not a blocker.
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
github-actions
Bot
added
the
query-count changed
Contributor
Query-count snapshot changes17 routes changed, total Δ +67 queries. D1
Comparing snapshot files between base and head. Updated automatically on each push. |
github-actions
Bot
added
review/needs-rereview
19 tasks
Contributor
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
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 does this PR do?
Reduces duplicate database reads and unnecessary content processing during public page renders that use widget areas or the Archives widget.
Accept: text/html, so they exercise the same layout prefetch as browser requests. RSS uses its own content type.The snapshot change includes a measurement correction: the fixture's warm homepage count changes from 6 to 10 because the harness includes prefetch work that already runs for browser requests. Old and corrected snapshot totals are not a like-for-like performance comparison.
Type of change
Checklist
pnpm typecheckpasses.pnpm lintpasses.AI-generated code disclosure
Screenshots / test output
Screenshots: not applicable; the rendered Archives HTML is unchanged.
The same local D1/workerd benchmark was run against the reference implementation, the previous PR head, and this update. Each route used five warmup requests followed by five batches of 50 requests. Values below are the median of batch mean active CPU per response, including garbage collection and excluding idle time.
The reference uses the benchmark branch runtime; the previous PR head is
db122778. All three runs use the same seeded D1 data within each dataset. The larger fixture has 500 synthetic posts across 24 months. In the 500-post fixture, all three implementations render byte-identical Archives HTML with the same 12 displayed groups, links, labels, and counts. The extra date projection reduces the large-fixture post cost by about 53% against the previous PR head; the small-fixture differences are modest. These are local profiler measurements, separate from live Workers invocation CPU.Validation also covers publication visibility, locale filtering, request deduplication, content cache invalidation, preview isolation, failure recovery, SQL parameter binding, and widget prefetch regressions. The standard fixture's query counts stay unchanged with the date projection on both SQLite and D1; the SQL snapshots now show the narrower Archives query.