fix: cache location rules responses client-side (#29)#35
Conversation
The `/interact/v1/get_location_rules/{type}` endpoint runs unbounded
`get_posts()` queries and was fetched per LocationRule, on mount and on
every param change, with no caching or dedup. This memoizes responses in
a module-level cache keyed by param and shares in-flight requests across
all LocationRule instances, avoiding redundant parallel queries and large
repeated payloads.
Responses are cloned before adding the "Current Post/Page" option so the
shared cache is never mutated, failed requests are not cached (so they can
retry), and the effect now guards against setState after unmount.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughLocation rule fetching now caches shared API promises by parameter, removes failed requests for retry, clones responses before modification, and prevents asynchronous state updates after unmount. ChangesLocation rule fetching
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/editor/components/location-rules/index.js (1)
176-178: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffOptional: scope the clone to avoid deep-copying large payloads.
cloneDeep(response)runs unconditionally, but only thepost/pagebranch mutatesoptions(theunshiftat line 196) — and those are precisely the unbounded payloads called out in issue#29. Non-post/pageparams never mutate, and thepost/pagecase only needs the matched group'soptionsarray copied. A full deep clone perLocationRuleinstance duplicates the whole (potentially huge) post list each render.Consider cloning only when needed, and shallowly at the mutation site, keeping the shared cache immutable while avoiding the O(n) deep copy of every option object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/editor/components/location-rules/index.js` around lines 176 - 178, Limit cloning in the location-rule option handling to the post/page branch that performs the unshift in the relevant rule-building logic. Avoid unconditional cloneDeep(response); preserve cached data by shallow-copying only the matched group and its options array immediately before adding “Current Post/Page,” while leaving non-post/page responses unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/editor/components/location-rules/index.js`:
- Around line 176-178: Limit cloning in the location-rule option handling to the
post/page branch that performs the unshift in the relevant rule-building logic.
Avoid unconditional cloneDeep(response); preserve cached data by shallow-copying
only the matched group and its options array immediately before adding “Current
Post/Page,” while leaving non-post/page responses unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c8658d5-7539-4256-8005-5a2468f1bb0f
📒 Files selected for processing (1)
src/editor/components/location-rules/index.js
1 participant
Summary
Fixes #29 — the location rules fetch can be a PHP performance bottleneck.
The
/interact/v1/get_location_rules/{type}endpoint runs unboundedget_posts( [ 'posts_per_page' => -1 ] )queries forpost/pagerules. On the client it was fired perLocationRuleinstance, on mount and on everyparamchange, with no caching or dedup — so multiple rules meant multiple parallel unbounded queries and large repeated payloads that scale badly with site size.This applies the recommended client-side fix:
paramthat memoizes each response.LocationRuleinstances (dedup), so simultaneous mounts trigger a single request perparam.setStateafter unmount and resets the busy state on error.No server-side or API changes; behavior is identical from the user's perspective, just without the redundant repeated queries.
Test plan
postrules) — verify only one network request per param in the Network tab.post->page->post) — verify the second visit to a param serves from cache with no new request.Made with Cursor
Summary by CodeRabbit