Skip to content

Extend rx.memo configuration and function memoization - #7084

Draft
masenf wants to merge 1 commit into
mainfrom
codex/extend-rx-memo
Draft

Extend rx.memo configuration and function memoization#7084
masenf wants to merge 1 commit into
mainfrom
codex/extend-rx-memo

Conversation

@masenf

@masenf masenf commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add by_value, recursive, and JavaScript-safe name options to @rx.memo
  • compose value comparison with custom wrappers and make Var-returning memos cache results by default
  • preserve nested auto-memoization inside recursive explicit component boundaries
  • document the new behavior and add changelog fragments

Testing

  • uv run --no-sync pytest -q tests/units/components/test_memo.py tests/units/compiler/test_memoize_plugin.py (246 passed)
  • focused Pyright checks (0 errors)
  • focused Ruff check and format checks
  • commit-time Ruff, codespell, stub generation, Pyright, ty, and Biome hooks

Notes

  • by_value uses JSON.stringify, including its cyclic-value and serialization limitations.
  • function memo caches persist for the module lifetime and grow with unique argument tuples.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

New Feature Submission

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features

  • Have you added an explanation of what your changes do and why you would like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

Review in cubic

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

Fix recursive parameter capture and packed-argument identity caching before merging.

Findings

  1. P1 Recursive extraction loses parameter scope
  2. P1 Packed arguments always miss cache
  3. P2 Valid dollar-sign names are rejected

Summary

  • Wrapper composition and compiler templates support both component and function memos.
  • Documentation, changelog fragments, and compiler tests cover the new options.
  • Recursive extraction loses enclosing parameter bindings, and default caching misses every call for packed children/rest signatures.

Reviews (1) · Last reviewed commit: "feat: extend rx.memo configuration"

Comment on lines +516 to +520
transformed = hooks.compile_component(
memo.component,
page_context=page_context,
compile_context=compile_context,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Recursive extraction loses parameter scope

A recursive memo returning rx.text(label + State.count.to(str)) contains a reactive expression that references its labelRxMemo parameter. This pass extracts that expression into an auto-memo definition, but generated wrappers accept only children and receive no captured parameters. The extracted component therefore references an unbound labelRxMemo and raises a ReferenceError during rendering. Forward the enclosing memo parameters into generated wrappers, or prevent extraction of expressions that depend on those locals.

Comment on lines +87 to +93
_DEFAULT_FUNCTION_MEMO_WRAPPER: FunctionVar = FunctionStringVar.create(
"(fn) => { const resultKey = Symbol(); const cache = new Map(); "
"return (...args) => { let node = cache; for (const arg of args) { "
"if (!node.has(arg)) node.set(arg, new Map()); node = node.get(arg); } "
"if (!node.has(resultKey)) node.set(resultKey, fn(...args)); "
"return node.get(resultKey); }; }"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Packed arguments always miss cache

Function memos declaring children or rx.RestProp pack their arguments into a fresh JavaScript object literal. Repeated calls such as merge_styles(base=base, color="red") therefore use a different Map key on every render, even when the inputs are unchanged. The default cache never reuses the result and permanently retains every temporary props object and result. Key these signatures by their logical argument values rather than the fresh transport object, and add a repeated-call regression test.

Comment on lines +112 to +119
if name.isidentifier():
return True
if not name or name[0] not in "_$":
return False
return all(
char in "_$" or "a" <= char <= "z" or "A" <= char <= "Z" or "0" <= char <= "9"
for char in name[1:]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Valid dollar-sign names are rejected

A valid JavaScript name such as name="format$total" fails Python’s isidentifier() check, then fails this fallback because its first character is neither _ nor $. This unnecessarily prevents users from choosing names allowed by the documented JavaScript-identifier contract. Allow ASCII letters as initial characters in the fallback and test a name containing an interior $.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@codspeed-hq

codspeed-hq Bot commented Sep 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 40 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing codex/extend-rx-memo (2bdccb4) with main (074a818)

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.

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.

1 participant