feat(compiler): mirror memo output paths to Python source modules#6457
feat(compiler): mirror memo output paths to Python source modules#6457FarhanAliRaza wants to merge 13 commits into
Conversation
Memos now compile into a single JSX file per user module at a path that mirrors the module's dotted name, instead of one file per memo under . The page-side import surface matches the source layout, which makes debugging easier and lets Vite group co-defined memos in the same chunk. Memos without a captured source module keep the legacy per-name files and index. A manifest in records emitted paths so stale files from previous compiles get pruned.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR changes how
Confidence Score: 4/5The core mirrored-path compilation logic is correct and well-tested; the stale-file cleanup is the only broken path, and failure there is silent — orphaned .jsx files accumulate rather than causing build errors. The manifest-based stale file pruning never actually deletes files when get_web_dir() returns the default relative Path('.web'). The is-absolute check prepends .web/ to paths that already start with .web/, so every deletion target path is wrong in a standard Reflex project. The cleanup tests all mock get_web_dir() to an absolute path, so they pass without catching this. reflex/compiler/utils.py — specifically prune_stale_memo_files and its relative-path arithmetic. Important Files Changed
Reviews (2): Last reviewed commit: "remove weird importlib import of reflex...." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Identical memoizable subtrees on pages from different user modules produce the same wrapper tag. The auto-memo registry was keyed by tag alone, so the second registration overwrote the first — only one of the source modules got a mirrored memo file emitted, and the other page imported the tag from a JSX file that never declared it. Vite failed the prod build with MISSING_EXPORT. Key the registry by (tag, source_module) so each module's mirrored file gets its own definition, and add an integration test that builds two pages from distinct user modules sharing a memoizable subtree.
was d, so once a module had been resolved its mirrored path was frozen for the process. A user toggling a module between a regular and a package () during dev reload kept the original origin and emitted memo files to the stale path. Drop the cache and read from first, falling back to only when the module isn't loaded — is rebound on reload, while a cached spec wouldn't be. Also tighten to close the mkstemp fd up front so it can't leak if reopening raises, and re-export from for parity with the rest of the surface.
Memo components now mirror to their source module's combined file, so the self-referencing memo test can no longer find a per-name RecursiveBox.jsx path. Join all emitted code and assert on content.
Auto-memoized (rx.memo) components now compile to .web paths mirroring their defining Python module instead of a shared components.jsx, scoping the memo registry per module so same-named components no longer collide. Adds reflex_base.utils.memo_paths to translate source modules into the mirrored JSX path and $/... specifier.
This is net-new functionality, we don't need to provide compat shims
use more durable module/package names that are less likely to change in the future
1. the import was weird, why not just import directly? 2. the canonical location for _get_memo_component_class has moved to reflex_base
| def prune_stale_memo_files(emitted_paths: Iterable[str | Path]) -> None: | ||
| """Delete memo files written previously that this compile no longer emits. | ||
|
|
||
| Only paths that appear in the previous manifest are considered for | ||
| deletion — never a fresh filesystem walk — so files this code did not | ||
| emit are never touched. Empty parent directories created by mirrored | ||
| output are removed up to (but not including) the ``.web`` root. | ||
|
|
||
| Args: | ||
| emitted_paths: Absolute (or ``.web``-relative) paths the current | ||
| compile produced for the memo pipeline. | ||
| """ | ||
| web_dir = get_web_dir() | ||
|
|
||
| emitted_relative: set[str] = set() | ||
| for path in emitted_paths: | ||
| absolute = Path(path) | ||
| if not absolute.is_absolute(): | ||
| absolute = web_dir / absolute | ||
| try: | ||
| relative = absolute.relative_to(web_dir) | ||
| except ValueError: | ||
| continue | ||
| emitted_relative.add(str(relative).replace(os.sep, "/")) |
There was a problem hiding this comment.
When get_web_dir() returns the default Path(".web") (a relative path), the if not absolute.is_absolute() branch prepends .web/ to a path that already starts with .web/. For example, path = ".web/myapp/module.jsx" becomes absolute = Path(".web/.web/myapp/module.jsx"). The subsequent relative_to(Path(".web")) strips only the leading .web, recording ".web/myapp/module.jsx" in the manifest. On the next prune pass, target = Path(".web") / ".web/myapp/module.jsx" resolves to .web/.web/myapp/module.jsx, which never exists on disk, so is_file() returns False and the stale file is never deleted.
All tests in test_stale_cleanup.py mock get_web_dir() to an absolute tmp_path / ".web", so they pass without catching this. The simplest fix is to drop the is-absolute guard and call relative_to directly — it works for both absolute and relative paths as long as both sides are consistent: Path(".web/myapp/module.jsx").relative_to(Path(".web")) correctly yields PosixPath("myapp/module.jsx").
There was a problem hiding this comment.
yep, noticed this in local testing as well. the pruned paths are not cleaned up when the module is deleted/renamed
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
we need to put the modules under a subdir of .web that is not already used, maybe .web/app_components.
The problem arises when I have an app called app and a module called app.root which exports a memo component... this ends up getting overwritten by the app/root.jsx that the framework emits and then the page breaks when you try to load it.
The user's file tree hierarchy really shouldn't be able to break the app, hence keeping it under a subdirectory that is only used for emitting memo components would protect the rest of the app from unexpected changes.
2 participants
Memos now compile into a single JSX file per user module at a path that mirrors the module's dotted name, instead of one file per memo under . The page-side import surface matches the source
layout, which makes debugging easier and lets Vite group co-defined memos in the same chunk.
Memos without a captured source module keep the legacy per-name files and index. A manifest in records emitted
paths so stale files from previous compiles get pruned.
All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features:
closes #6218
fixes ENG-9150