Skip to content

Fix DashWrapper hydration cache eviction guard on latest-dash#31

Open
chgiesse wants to merge 1 commit into
mainfrom
codex/investigate-dash-renderer-performance-issues-bfar9k
Open

Fix DashWrapper hydration cache eviction guard on latest-dash#31
chgiesse wants to merge 1 commit into
mainfrom
codex/investigate-dash-renderer-performance-issues-bfar9k

Conversation

@chgiesse
Copy link
Copy Markdown
Owner

Motivation

  • Reduce repeated hydration and global layout scans that amplify provider-triggered re-renders by introducing path-scoped caching and indexed layout-hash lookups.
  • Address a TypeScript runtime issue encountered when running the renderer test build after integrating recent dash core updates.

Description

  • Add a path-scoped hydration cache in dash/dash-renderer/src/wrapper/DashWrapper.tsx with a simple LRU eviction, persistent memoizedKeys, pathKey memoization, and targeted invalidation on forced renders.
  • Restructure the layout-hash reducer in dash/dash-renderer/src/reducers/reducer.js to store entries and an index mapping so lookups for affected subpaths are O(1) instead of scanning the whole map.
  • Update selector logic in dash/dash-renderer/src/wrapper/selectors.ts to consume the new layoutHashes shape and limit checks to relevant indexed paths only.
  • Add a small safety guard around LRU eviction in DashWrapper to avoid calling Map.delete with an undefined key, which fixed the TypeScript compile error surfaced during the test build.

Testing

  • Ran npm run private::lint.renderer, which completed successfully.
  • Ran cd dash/dash-renderer && npm test -- --runInBand, and the webpack/TypeScript build phase completed (TypeScript error resolved); the Karma browser tests could not execute in this environment because no Chrome binary was available (Please, set "CHROME_BIN" env variable).

Codex Task

@chgiesse chgiesse marked this pull request as ready for review February 24, 2026 14:53
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ffc4e2dc7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +102 to 106
useEffect(() => {
const cache = hydrationCache.current;
if (_newRender) {
newRender.current = true;
renderH = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Set new-render flag before hydration cache lookup

_newRender is now processed in useEffect, but the render path reads newRender.current before effects run (for example, the cache reuse branch guarded by if (!newRender.current)). When a parent sends _newRender to force a redraw while renderH is unchanged, this render can still return cached hydration, and because mutating a ref in the effect does not schedule another render, the forced redraw may be skipped entirely.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant