fix(caching): invalidate list caches on entity update events#15337
fix(caching): invalidate list caches on entity update events#15337hunnyboy1217 wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 0e5e5bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 78 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 |
|
@hunnyboy1217 is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for your contribution, @hunnyboy1217! After reviewing this PR, we need one thing addressed before we can move forward: Required changes:
Everything else looks solid: ✅ PR template is thorough and complete — root cause, repro steps, tradeoff, and run instructions are all there Notes: The Once the changeset is added, this will be ready for team review. Triggered by: new PR opened |
hunnyboy1217
force-pushed
the
fix/caching-invalidate-list-on-update
branch
from
5626c2e to
4e555b2
Compare
|
Thank you for your contribution, @hunnyboy1217! 🎉 The previously requested changeset has been added — the ✅ PR template is complete — clear root cause, repro steps, tradeoff, and run instructions Notes: Two fields are now unused in
Neither affects correctness — both could be cleaned up in a follow-up if the team wants to keep the dead code surface small. A team member will do a final review before this is merged. We appreciate your patience! Triggered by: new commit pushed |
|
Re-reviewed after the latest commit, @hunnyboy1217 — no new concerns. The implementation, tests, and changeset are all in order exactly as noted in the previous review. The ✅ Changeset present and correctly formatted Notes (carry-forward, non-blocking):
Neither affects correctness. Both are candidates for a follow-up cleanup if the team wants to reduce dead-code surface. A team member will do the final review before merging. Thanks for your patience! Triggered by: new commit pushed |
|
Re-reviewed after the latest commit, @hunnyboy1217 — good progress on the cleanup. Both non-blocking notes from the previous review have been addressed: ✅ Notes (non-blocking): The Everything else remains solid — changeset is present and correctly formatted, the parser fix is correct, unit tests reflect the new expectations with the named regression test for #14903, and the integration test covers the draft-to-published scenario from the issue.
Triggered by: new commit pushed |
|
Re-reviewed after the latest commit, @hunnyboy1217. The one outstanding note from the previous review — the ✅ Notes (non-blocking): One orphan remains in // strategy.ts — handleEvent (dead computation + dead argument)
const operation = eventName.split(".").pop() as "created" | "updated" | "deleted"
...
const tags_ = await this.computeTags(item, {
entities: [entityReference],
operation, // no longer consumed inside computeTags
})Not a correctness issue — purely cosmetic. A follow-up PR could drop the
Triggered by: new commit pushed |
|
Re-reviewed after the latest commit, @hunnyboy1217 — the last outstanding note is now resolved. The dead ✅ The full clean-up trail is now complete — Everything else remains solid: changeset is present and correctly formatted, the parser fix is correct and well-commented, unit tests reflect the new expectations with the named regression test for #14903, and the integration test covers the draft-to-published scenario from the issue.
Triggered by: new commit pushed |
| expect(cachedList).toBeNull() | ||
| }) | ||
|
|
||
| it("should invalidate filtered lists when an entity not in the cached list is updated (regression for #14903)", async () => { |
There was a problem hiding this comment.
While I think your approach makes sense, I think we should keep the restriction on the operations to avoid unpredictable results. So adding support for the update operation instead of allowing changes for all operations.
2 participants
Summary
What — What changes are introduced in this PR?
buildAffectedCacheKeysin@medusajs/cachingnow always emits the${EntityType}:list:*invalidation tag for every entity reference, instead of emitting it only when the entity was found in an array context or when the operation wascreated/deleted.Why — Why are these changes relevant or necessary?
Fixes #14903. With the
cachingfeature flag enabled, list endpoints (e.g.GET /store/products) returned stale data after entity updates that affected filter‑relevant fields (status, sales channel, category, region, etc.).Repro from the issue:
GET /store/products— response is cached. The draft is excluded by the status filter, so the cached array does not contain it.product.updatedevent fires.GET /store/productsagain — the previously cached response is returned; the newly published product is missing until TTL expires.Root cause: when
product.updatedfired, only theProduct:<id>invalidation tag was generated. Because the draft product was filtered out of the cached list, no cache entry was tagged with that ID, so nothing matched and the stale list survived. The${type}:list:*wildcard tag — which is applied to cached list responses — was never produced forupdatedevents when the entity was not already part of a cached array.The cache layer has no visibility into which entity fields are filter‑relevant, so the only correct default is to invalidate list caches on every mutation.
How — How have these changes been implemented?
entity.isInArray || ["created", "deleted"].includes(operation)gate aroundkeys.add(\${entity.type}:list:*`)so the wildcard list tag is added for every entity reference. The entity‑level tag (Product:`) is unchanged.The tradeoff is slightly more aggressive list‑cache invalidation on
updatedevents. Single‑entity caches (Product:<id>) are still preserved for lookups that are unaffected by lists.Testing — How have these changes been tested, or how can the reviewer test the feature?
:list:*tag on non‑arrayupdatedoperations) and renamed the test that previously asserted the bug into an explicit regression test for [Bug]: Stale cache served for list responses with caching feature flag enabled #14903.status: "published") product list, emitsproduct.updatedfor a product not in the cached list, and asserts the cached list entry is cleared.To run locally:
Examples
Checklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
Closes #14903.
The reporter (@radoslawroszkowiak) identified the exact line and proposed this fix in the issue body; the maintainer triage bot confirmed the analysis on 2026‑04‑15. This PR implements that proposal and adds regression coverage at both the unit and integration level.
The change is contained to
@medusajs/cachingand only affects behavior when thecachingfeature flag is enabled (off by default).