Skip to content

fix: make Moment a MemoizationLeaf - #6784

Merged
masenf merged 1 commit into
reflex-dev:mainfrom
benedikt-bartscher:fix-moment-memo
Jul 16, 2026
Merged

fix: make Moment a MemoizationLeaf#6784
masenf merged 1 commit into
reflex-dev:mainfrom
benedikt-bartscher:fix-moment-memo

Conversation

@benedikt-bartscher

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where passing a stateful Reflex Var as a child to rx.moment caused the date to be silently parsed as today at midnight. The root cause was that the memoization compiler was wrapping the child in an independent Bare_* snapshot component, and react-moment received a React element object rather than the raw date string — feeding that object into moment({}).

  • Moment now inherits from both NoSSRComponent and MemoizationLeaf, mirroring the existing pattern in reflex-components-recharts. MemoizationLeaf sets _memoization_mode = MemoizationMode(recursive=False), which keeps the child variable's interpolation inside the Moment snapshot rather than lifting it to a separate memo wrapper.
  • Two regression tests are added that mirror the analogous Title/Meta tests, asserting the stateful hook stays inside the memo body and no Bare_* wrapper tag appears in the generated output.

Confidence Score: 5/5

Safe to merge — single-line inheritance change with strong test coverage and an established codebase precedent in reflex-components-recharts.

The change is minimal and targeted: adding MemoizationLeaf to Moment's base classes mirrors an identical pattern already used in recharts. The MRO is correct (NoSSRComponent and MemoizationLeaf both ultimately extend Component, and neither NoSSRComponent nor Component shadows _memoization_mode in a way that would override MemoizationLeaf). Two dedicated regression tests verify both the page-level and memo-body-level behavior. No side effects on other components are possible since the change is scoped to the Moment class alone.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-components-moment/src/reflex_components_moment/moment.py Adds MemoizationLeaf to Moment's MRO — a one-line, well-precedented fix that prevents child memoization wrapping
tests/units/compiler/test_memoize_plugin.py Adds two regression tests (page-level and memo-body-level) asserting the fix prevents Bare_ wrapping of stateful Moment children
pyi_hashes.json Updated moment.pyi hash to reflect the new MemoizationLeaf base class in the generated stub
packages/reflex-components-moment/news/+moment-memoization-leaf.bugfix.md Changelog entry accurately describing the bug and fix

Reviews (1): Last reviewed commit: "fix: make Moment a MemoizationLeaf" | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing benedikt-bartscher:fix-moment-memo (c95ce22) with main (34ee005)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@benedikt-bartscher
benedikt-bartscher marked this pull request as ready for review July 16, 2026 18:03
@benedikt-bartscher
benedikt-bartscher requested a review from a team as a code owner July 16, 2026 18:03

@masenf masenf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good catch

@masenf
masenf merged commit 7e80931 into reflex-dev:main Jul 16, 2026
107 checks passed
@masenf

masenf commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

i was trying to figure out why i wasn't seeing this in my apps, and everywhere i was using moment was either in a foreach or explicitly memo'd outer component, both of which would mask the bug.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants